Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Shopify/theme-tools packages #4886

Merged
merged 1 commit into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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