Skip to content

Commit

Permalink
Merge pull request #4886 from Shopify/bump/theme-tools-2024-11-19
Browse files Browse the repository at this point in the history
Bump Shopify/theme-tools packages
  • Loading branch information
gonzaloriestra authored Nov 28, 2024
2 parents 3704940 + 33f713e commit 4254118
Show file tree
Hide file tree
Showing 10 changed files with 103 additions and 78 deletions.
6 changes: 6 additions & 0 deletions .changeset/witty-dodos-juggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/theme': patch
'@shopify/app': patch
---

Bump @shopify/theme-check-node & @shopify/theme-language-server
2 changes: 1 addition & 1 deletion packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"@shopify/polaris": "12.10.0",
"@shopify/polaris-icons": "8.0.0",
"@shopify/theme": "3.70.0",
"@shopify/theme-check-node": "2.9.2",
"@shopify/theme-check-node": "3.2.2",
"body-parser": "1.20.2",
"camelcase-keys": "9.1.3",
"chokidar": "3.5.3",
Expand Down
9 changes: 5 additions & 4 deletions packages/app/src/cli/services/build/theme-check.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {readFileSync} from '@shopify/cli-kit/node/fs'
import {itemToString} from '@shopify/cli-kit/node/output'
import {TokenItem} from '@shopify/cli-kit/node/ui'
import {Severity, type Offense, check} from '@shopify/theme-check-node'
import {Severity, type Offense, check, path as pathUtils} from '@shopify/theme-check-node'

/**
* Returns a code snippet from a file. All line numbers given MUST be zero indexed
*/
function getSnippet(absolutePath: string, startLine: number, endLine: number) {
function getSnippet(uri: string, startLine: number, endLine: number) {
const absolutePath = pathUtils.fsPath(uri)
const fileContent = readFileSync(absolutePath).toString()
const lines = fileContent.split('\n')
const snippetLines = lines.slice(startLine, endLine + 1)
Expand Down Expand Up @@ -45,9 +46,9 @@ function severityToToken(severity: Severity) {
*/
function formatOffenses(offenses: Offense[]): TokenItem {
const offenseBodies = offenses.map((offense, index) => {
const {message, absolutePath, start, end, check, severity} = offense
const {message, uri, start, end, check, severity} = offense
// Theme check line numbers are zero indexed, but intuitively 1-indexed
const codeSnippet = getSnippet(absolutePath, start.line, end.line)
const codeSnippet = getSnippet(uri, start.line, end.line)

// Ensure enough padding between offenses
const offensePadding = index === offenses.length - 1 ? '' : '\n\n'
Expand Down
4 changes: 2 additions & 2 deletions packages/theme/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
"dependencies": {
"@oclif/core": "3.26.5",
"@shopify/cli-kit": "3.70.0",
"@shopify/theme-check-node": "2.9.2",
"@shopify/theme-language-server-node": "1.14.0",
"@shopify/theme-check-node": "3.2.2",
"@shopify/theme-language-server-node": "2.2.2",
"chokidar": "3.5.3",
"h3": "1.12.0",
"yaml": "2.3.2"
Expand Down
6 changes: 3 additions & 3 deletions packages/theme/src/cli/commands/theme/check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('Check', () => {
context: 'theme',
settings: {},
checks: [],
root: '',
rootUri: '',
}
const mockOffenses: Offense[] = []

Expand All @@ -53,7 +53,7 @@ describe('Check', () => {
context: 'app',
settings: {},
checks: [],
root: '',
rootUri: '',
}
const mockOffenses: Offense[] = []

Expand All @@ -72,7 +72,7 @@ describe('Check', () => {
context: 'theme',
settings: {},
checks: [],
root: '',
rootUri: '',
}
const mockOffenses: Offense[] = []

Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/cli/commands/theme/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export default class Check extends ThemeCommand {

let version = 'unknown'
if (pkgJsonPath) {
version = (await getPackageVersion(pkgJsonPath)) || 'unknown'
version = (await getPackageVersion(pkgJsonPath)) ?? 'unknown'
}

outputInfo(version)
Expand Down
48 changes: 29 additions & 19 deletions packages/theme/src/cli/services/check.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@ import {
import {fileExists, readFileSync, writeFile} from '@shopify/cli-kit/node/fs'
import {outputInfo, outputSuccess} from '@shopify/cli-kit/node/output'
import {renderInfo} from '@shopify/cli-kit/node/ui'
import {Severity, SourceCodeType, loadConfig, type Offense, type Theme} from '@shopify/theme-check-node'
import {
Severity,
SourceCodeType,
loadConfig,
path as pathUtils,
type Offense,
type Theme,
} from '@shopify/theme-check-node'
import {Mock, MockInstance, afterAll, beforeEach, describe, expect, test, vi} from 'vitest'

vi.mock('@shopify/cli-kit/node/fs', async () => ({
Expand Down Expand Up @@ -52,7 +59,7 @@ describe('formatOffenses', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand Down Expand Up @@ -80,7 +87,7 @@ describe('formatOffenses', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -89,7 +96,7 @@ describe('formatOffenses', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.WARNING,
start: {index: 0, line: 2, character: 0},
end: {index: 10, line: 2, character: 10},
Expand All @@ -115,12 +122,14 @@ describe('formatOffenses', () => {

describe('sortOffenses', () => {
test('should sort offenses by file path', () => {
const uri1 = pathUtils.normalize('file:///path/to/file1')
const uri2 = pathUtils.normalize('file:///path/to/file2')
const offenses: Offense[] = [
{
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file2',
uri: uri2,
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -129,7 +138,7 @@ describe('sortOffenses', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file1',
uri: uri1,
severity: Severity.WARNING,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -139,18 +148,19 @@ describe('sortOffenses', () => {
const result = sortOffenses(offenses)

expect(result).toEqual({
'/path/to/file1': [offenses[1]],
'/path/to/file2': [offenses[0]],
[pathUtils.fsPath(uri1)]: [offenses[1]],
[pathUtils.fsPath(uri2)]: [offenses[0]],
})
})

test('should sort offenses by severity within each file', () => {
const uri = pathUtils.normalize('file:///path/to/file')
const offenses: Offense[] = [
{
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri,
severity: Severity.WARNING,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -159,7 +169,7 @@ describe('sortOffenses', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri,
severity: Severity.ERROR,
start: {index: 0, line: 2, character: 0},
end: {index: 10, line: 2, character: 10},
Expand All @@ -169,7 +179,7 @@ describe('sortOffenses', () => {
const result = sortOffenses(offenses)

expect(result).toEqual({
'/path/to/file': [offenses[1], offenses[0]],
[pathUtils.fsPath(uri)]: [offenses[1], offenses[0]],
})
})
})
Expand All @@ -190,7 +200,7 @@ describe('formatSummary', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -199,7 +209,7 @@ describe('formatSummary', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.WARNING,
start: {index: 0, line: 2, character: 0},
end: {index: 10, line: 2, character: 10},
Expand Down Expand Up @@ -235,7 +245,7 @@ describe('renderOffensesText', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -258,7 +268,7 @@ describe('formatOffensesJson', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -267,7 +277,7 @@ describe('formatOffensesJson', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.WARNING,
start: {index: 0, line: 2, character: 0},
end: {index: 10, line: 2, character: 10},
Expand Down Expand Up @@ -333,7 +343,7 @@ describe('handleExit', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -349,7 +359,7 @@ describe('handleExit', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.ERROR,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand All @@ -365,7 +375,7 @@ describe('handleExit', () => {
type: SourceCodeType.LiquidHtml,
check: 'LiquidHTMLSyntaxError',
message: 'Attempting to close HtmlElement',
absolutePath: '/path/to/file',
uri: 'file:///path/to/file',
severity: Severity.INFO,
start: {index: 0, line: 1, character: 0},
end: {index: 10, line: 1, character: 10},
Expand Down
38 changes: 22 additions & 16 deletions packages/theme/src/cli/services/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
type FixApplicator,
type Offense,
type Theme,
path as pathUtils,
} from '@shopify/theme-check-node'
import YAML from 'yaml'

Expand All @@ -37,7 +38,9 @@ interface TransformedOffenseMap {
}

type SeverityCounts = Partial<{
[K in keyof typeof Severity]: number
[Severity.ERROR]: number
[Severity.WARNING]: number
[Severity.INFO]: number
}>

export type FailLevel = 'error' | 'suggestion' | 'style' | 'warning' | 'info' | 'crash'
Expand Down Expand Up @@ -71,8 +74,9 @@ function severityToLabel(severity: Severity) {
/**
* Returns a code snippet from a file. All line numbers given MUST be zero indexed
*/
function getSnippet(absolutePath: string, startLine: number, endLine: number) {
const fileContent = readFileSync(absolutePath).toString()
function getSnippet(uri: string, startLine: number, endLine: number) {
const fsPath = pathUtils.fsPath(uri)
const fileContent = readFileSync(fsPath).toString()
const lines = fileContent.split('\n')
const snippetLines = lines.slice(startLine, endLine + 1)
const isSingleLine = snippetLines.length === 1
Expand Down Expand Up @@ -110,9 +114,9 @@ function severityToToken(severity: Severity) {
*/
export function formatOffenses(offenses: Offense[]) {
const offenseBodies = offenses.map((offense, index) => {
const {message, absolutePath, start, end, check, severity} = offense
const {message, uri, start, end, check, severity} = offense
// Theme check line numbers are zero indexed, but intuitively 1-indexed
const codeSnippet = getSnippet(absolutePath, start.line, end.line)
const codeSnippet = getSnippet(uri, start.line, end.line)

// Ensure enough padding between offenses
const offensePadding = index === offenses.length - 1 ? '' : '\n\n'
Expand All @@ -132,12 +136,13 @@ const offenseSeverityAscending = (offenseA: Offense, offenseB: Offense) => offen
export function sortOffenses(offenses: Offense[]): OffenseMap {
// Bucket offenses by filename
const offensesByFile = offenses.reduce((acc: OffenseMap, offense: Offense) => {
const {absolutePath} = offense
if (!acc[absolutePath]) {
acc[absolutePath] = []
const {uri} = offense
const filePath = pathUtils.fsPath(uri)
if (!acc[filePath]) {
acc[filePath] = []
}

acc[absolutePath]!.push(offense)
acc[filePath]!.push(offense)
return acc
}, {})

Expand Down Expand Up @@ -222,9 +227,9 @@ export function formatOffensesJson(offensesByFile: OffenseMap): TransformedOffen
return {
path,
offenses: transformedOffenses,
errorCount: counts[Severity.ERROR] || 0,
warningCount: counts[Severity.WARNING] || 0,
infoCount: counts[Severity.INFO] || 0,
errorCount: counts[Severity.ERROR] ?? 0,
warningCount: counts[Severity.WARNING] ?? 0,
infoCount: counts[Severity.INFO] ?? 0,
}
})
}
Expand Down Expand Up @@ -274,15 +279,16 @@ export async function initConfig(root: string) {

const saveToDiskFixApplicator: FixApplicator = async (sourceCode, fix) => {
const updatedSource = applyFixToString(sourceCode.source, fix)
await writeFile(sourceCode.absolutePath, updatedSource)
const absolutePath = pathUtils.fsPath(sourceCode.uri)
await writeFile(absolutePath, updatedSource)
}

export async function performAutoFixes(sourceCodes: Theme, offenses: Offense[]) {
await autofix(sourceCodes, offenses, saveToDiskFixApplicator)
}

export async function outputActiveConfig(themeRoot: string, configPath?: string) {
const {ignore, settings, root} = await loadConfig(configPath, themeRoot)
const {ignore, settings, rootUri} = await loadConfig(configPath, themeRoot)

const config = {
// loadConfig flattens all configs, it doesn't extend anything
Expand All @@ -292,7 +298,7 @@ export async function outputActiveConfig(themeRoot: string, configPath?: string)
// duplicate patterns to ignore. We can clean them before outputting.
ignore: [...new Set(ignore)],

root,
rootUri,

// Dump out the active settings for all checks.
...settings,
Expand All @@ -314,7 +320,7 @@ export async function outputActiveChecks(root: string, configPath?: string) {
return acc
}

const severityLabel = severityToLabel(severity === undefined ? Severity.INFO : severity)
const severityLabel = severityToLabel(severity ?? Severity.INFO)

// Map metafields from the check into desired output format
const meta = checks.find((check) => check.meta.code === checkCode)
Expand Down
Loading

0 comments on commit 4254118

Please sign in to comment.