Skip to content

Commit

Permalink
Update checksum calculation to no longer minify assets before calcula…
Browse files Browse the repository at this point in the history
…ting
  • Loading branch information
karreiro committed Sep 30, 2024
1 parent 200fc0d commit 729faaf
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/tiny-poems-tie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme': patch
---

Update checksum calculation to no longer minify assets before calculating
10 changes: 5 additions & 5 deletions packages/theme/src/cli/utilities/asset-checksum.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,27 @@ import {readThemeFile} from './theme-fs.js'
import {describe, expect, test} from 'vitest'

describe('asset-checksum', () => {
describe('normalizeJson', async () => {
describe('calculateChecksum', async () => {
const testCases = [
{file: 'assets/base.css', expectedChecksum: 'b7fbe0ecff2a6c1d6e697a13096e2b17'},
{file: 'assets/sparkle.gif', expectedChecksum: '7adcd48a3cc215a81fabd9dafb919507'},
{file: 'config/settings_data.json', expectedChecksum: '22e69af13b7953914563c60035a831bc'},
{file: 'config/settings_schema.json', expectedChecksum: 'cbe979d3fd3b7cdf2041ada9fdb3af57'},
{file: 'layout/password.liquid', expectedChecksum: '7a92d18f1f58b2396c46f98f9e502c6a'},
{file: 'layout/theme.liquid', expectedChecksum: '2374357fdadd3b4636405e80e21e87fc'},
{file: 'locales/en.default.json', expectedChecksum: '94d575574a070397f297a2e9bb32ce7d'},
{file: 'locales/en.default.json', expectedChecksum: '0b2f0aa705a4eb2b4740e2ed68bc043f'},
{file: 'sections/announcement-bar.liquid', expectedChecksum: '3e8fecc3fb5e886f082e12357beb5d56'},
{file: 'snippets/language-localization.liquid', expectedChecksum: 'aa0c697b712b22753f73c84ba8a2e35a'},
{file: 'templates/404.json', expectedChecksum: 'f14a0bd594f4fee47b13fc09543098ff'},
{file: 'templates/404.json', expectedChecksum: '64caf742bd427adcf497bffab63df30c'},
]

testCases.forEach(({file, expectedChecksum}) => {
test(`returns the expected checksum for "${file}"`, async () => {
// Given
const root = 'src/cli/utilities/fixtures/theme'
const content = await readThemeFile(root, file)

// When
const actualChecksum = await calculateChecksum(file, await readThemeFile(root, file))
const actualChecksum = await calculateChecksum(file, content)

// Then
expect(actualChecksum).toEqual(expectedChecksum)
Expand Down
50 changes: 32 additions & 18 deletions packages/theme/src/cli/utilities/asset-checksum.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,42 @@
import {isThemeAsset, isJson, isTextFile} from './theme-fs.js'
import {isTextFile} from './theme-fs.js'
import {Checksum} from '@shopify/cli-kit/node/themes/types'
import {fileHash} from '@shopify/cli-kit/node/crypto'

export function calculateChecksum(fileKey: string, fileContent: string | Buffer | undefined) {
let content = fileContent
if (!fileContent) {
return ''
}

if (Buffer.isBuffer(fileContent)) {
return md5(fileContent)
}

/**
* Settings data files are always minified before persistence, so their
* checksum calculation accounts for that minification.
*/
if (isSettingsData(fileKey)) {
return minifiedJSONFileChecksum(fileContent)
}

if (!content) return ''
return regularFileChecksum(fileKey, fileContent)
}

function minifiedJSONFileChecksum(fileContent: string) {
let content = fileContent

if (Buffer.isBuffer(content)) return md5(content)
content = content.replace(/\r\n/g, '\n')
content = content.replace(/\/\*[\s\S]*?\*\//, '')
content = normalizeJson(content)

if (isTextFile(fileKey)) content = content.replace(/\r\n/g, '\n')
return md5(content)
}

if (isJson(fileKey) && !isThemeAsset(fileKey) && !isSettingsSchema(fileKey)) {
content = normalizeJson(content)
function regularFileChecksum(fileKey: string, fileContent: string) {
let content = fileContent

/**
* The backend (Assets API) escapes forward slashes for internal JSON
* assets such as templates/*.json, sections/*.json, config/*.json.
*
* To maintain consistency in checksum calculation, we follow the same
* approach here (note that already escaped forward slashes are not
* re-escaped).
*/
content = content.replace(/(?<!\\)\//g, '\\/')
if (isTextFile(fileKey)) {
content = content.replace(/\r\n/g, '\n')
}

return md5(content)
Expand Down Expand Up @@ -92,6 +106,6 @@ function hasLiquidSource(checksums: Checksum[], key: string) {
return checksums.some((checksum) => checksum.key === `${key}.liquid`)
}

function isSettingsSchema(path: string) {
return path.endsWith('/settings_schema.json')
function isSettingsData(path: string) {
return path.endsWith('/settings_data.json')
}

0 comments on commit 729faaf

Please sign in to comment.