diff --git a/.changeset/fix-theme-push-fresh-dynamic-source-defaults.md b/.changeset/fix-theme-push-fresh-dynamic-source-defaults.md new file mode 100644 index 0000000000..4283b2e8d1 --- /dev/null +++ b/.changeset/fix-theme-push-fresh-dynamic-source-defaults.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Upload `config/settings_schema.json` before any other theme file. Fixes `theme push` failing on the first push when blocks or sections reference a `color_palette` theme setting. diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts index fc540bd80f..ce4e50169c 100644 --- a/packages/theme/src/cli/utilities/theme-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-fs.test.ts @@ -473,7 +473,8 @@ describe('theme-fs', () => { otherLiquidFiles, templateJsonFiles, otherJsonFiles, - configFiles, + configSchemaFile, + configDataFile, staticAssetFiles, contextualizedJsonFiles, blockLiquidFiles, @@ -489,10 +490,8 @@ describe('theme-fs', () => { ]) expect(otherJsonFiles).toEqual([{key: 'locales/en.default.json', checksum: '6'}]) expect(templateJsonFiles).toEqual([{key: 'templates/404.json', checksum: '7'}]) - expect(configFiles).toEqual([ - {key: 'config/settings_schema.json', checksum: '8'}, - {key: 'config/settings_data.json', checksum: '9'}, - ]) + expect(configSchemaFile).toEqual([{key: 'config/settings_schema.json', checksum: '8'}]) + expect(configDataFile).toEqual([{key: 'config/settings_data.json', checksum: '9'}]) expect(staticAssetFiles).toEqual([ {key: 'assets/base.css', checksum: '1'}, {key: 'assets/sparkle.gif', checksum: '3'}, @@ -511,15 +510,23 @@ describe('theme-fs', () => { const files: Checksum[] = [] // When - const {sectionLiquidFiles, otherLiquidFiles, templateJsonFiles, otherJsonFiles, configFiles, staticAssetFiles} = - partitionThemeFiles(files) + const { + sectionLiquidFiles, + otherLiquidFiles, + templateJsonFiles, + otherJsonFiles, + configSchemaFile, + configDataFile, + staticAssetFiles, + } = partitionThemeFiles(files) // Then expect(sectionLiquidFiles).toEqual([]) expect(otherLiquidFiles).toEqual([]) expect(templateJsonFiles).toEqual([]) expect(otherJsonFiles).toEqual([]) - expect(configFiles).toEqual([]) + expect(configSchemaFile).toEqual([]) + expect(configDataFile).toEqual([]) expect(staticAssetFiles).toEqual([]) }) }) diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts index cf8928b41b..68f6f6ac22 100644 --- a/packages/theme/src/cli/utilities/theme-fs.ts +++ b/packages/theme/src/cli/utilities/theme-fs.ts @@ -44,7 +44,8 @@ const THEME_PARTITION_REGEX = { layoutLiquidRegex: /^layout\/.+\.liquid$/, sectionLiquidRegex: /^sections\/.+\.liquid$/, blockLiquidRegex: /^blocks\/.+\.liquid$/, - configRegex: /^config\/(settings_schema|settings_data)\.json$/, + configSchemaRegex: /^config\/settings_schema\.json$/, + configDataRegex: /^config\/settings_data\.json$/, sectionJsonRegex: /^sections\/.+\.json$/, templateJsonRegex: /^templates\/.+\.json$/, jsonRegex: /^(?!config\/).*\.json$/, @@ -465,7 +466,8 @@ export function partitionThemeFiles(files: T[]) { const templateJsonFiles: T[] = [] const otherJsonFiles: T[] = [] const contextualizedJsonFiles: T[] = [] - const configFiles: T[] = [] + const configSchemaFile: T[] = [] + const configDataFile: T[] = [] const staticAssetFiles: T[] = [] const blockLiquidFiles: T[] = [] const layoutFiles: T[] = [] @@ -482,8 +484,10 @@ export function partitionThemeFiles(files: T[]) { } else { otherLiquidFiles.push(file) } - } else if (THEME_PARTITION_REGEX.configRegex.test(fileKey)) { - configFiles.push(file) + } else if (THEME_PARTITION_REGEX.configSchemaRegex.test(fileKey)) { + configSchemaFile.push(file) + } else if (THEME_PARTITION_REGEX.configDataRegex.test(fileKey)) { + configDataFile.push(file) } else if (THEME_PARTITION_REGEX.jsonRegex.test(fileKey)) { if (THEME_PARTITION_REGEX.contextualizedJsonRegex.test(fileKey)) { contextualizedJsonFiles.push(file) @@ -506,7 +510,8 @@ export function partitionThemeFiles(files: T[]) { templateJsonFiles, contextualizedJsonFiles, otherJsonFiles, - configFiles, + configSchemaFile, + configDataFile, staticAssetFiles, blockLiquidFiles, layoutFiles, diff --git a/packages/theme/src/cli/utilities/theme-uploader.test.ts b/packages/theme/src/cli/utilities/theme-uploader.test.ts index 41f92a3525..b97e7c68cc 100644 --- a/packages/theme/src/cli/utilities/theme-uploader.test.ts +++ b/packages/theme/src/cli/utilities/theme-uploader.test.ts @@ -319,17 +319,19 @@ describe('theme-uploader', () => { await renderThemeSyncProgress() // Then - expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(9) + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(10) // Minimum theme files start first expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith(1, remoteTheme.id, MINIMUM_THEME_ASSETS, adminSession) - // Dependent assets start second + // settings_schema.json is uploaded first among dependent files so block / section / + // section-group / template validators can resolve dynamic-source defaults that + // reference theme-level settings declared in the schema. expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( 2, remoteTheme.id, - [{key: 'layout/theme.liquid'}], + [{key: 'config/settings_schema.json'}], adminSession, ) - // Independent assets start right after dependent assets start + // Independent assets fan out concurrently with the dependent chain. expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( 3, remoteTheme.id, @@ -346,16 +348,22 @@ describe('theme-uploader', () => { ], adminSession, ) - // Dependent assets continue after the first batch of dependent assets ends + // Layout files come after the schema is in place. expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( 4, remoteTheme.id, - [{key: 'blocks/block.liquid'}], + [{key: 'layout/theme.liquid'}], adminSession, ) expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( 5, remoteTheme.id, + [{key: 'blocks/block.liquid'}], + adminSession, + ) + expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( + 6, + remoteTheme.id, [ { key: 'sections/header.liquid', @@ -365,7 +373,7 @@ describe('theme-uploader', () => { ) expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( - 6, + 7, remoteTheme.id, [ { @@ -376,7 +384,7 @@ describe('theme-uploader', () => { ) expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( - 7, + 8, remoteTheme.id, [ { @@ -387,7 +395,7 @@ describe('theme-uploader', () => { ) expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( - 8, + 9, remoteTheme.id, [ { @@ -397,17 +405,11 @@ describe('theme-uploader', () => { adminSession, ) + // settings_data.json must be last expect(bulkUploadThemeAssets).toHaveBeenNthCalledWith( - 9, + 10, remoteTheme.id, - [ - { - key: 'config/settings_data.json', - }, - { - key: 'config/settings_schema.json', - }, - ], + [{key: 'config/settings_data.json'}], adminSession, ) }) diff --git a/packages/theme/src/cli/utilities/theme-uploader.ts b/packages/theme/src/cli/utilities/theme-uploader.ts index 2b56cb0435..5d3a477a59 100644 --- a/packages/theme/src/cli/utilities/theme-uploader.ts +++ b/packages/theme/src/cli/utilities/theme-uploader.ts @@ -202,6 +202,7 @@ function getRemoteFilesToBeDeleted(remoteChecksums: Checksum[], themeFileSystem: } // Contextual Json Files -> Json Files -> Liquid Files -> Config Files -> Static Asset Files +// Config file consideration: Inverse of the upload order: data first (consumes schema), then schema. function orderFilesToBeDeleted(files: Checksum[]): Checksum[] { const fileSets = partitionThemeFiles(files) return [ @@ -213,7 +214,8 @@ function orderFilesToBeDeleted(files: Checksum[]): Checksum[] { ...fileSets.blockLiquidFiles, ...fileSets.layoutFiles, ...fileSets.otherLiquidFiles, - ...fileSets.configFiles, + ...fileSets.configDataFile, + ...fileSets.configSchemaFile, ...fileSets.staticAssetFiles, ] } @@ -311,13 +313,23 @@ function selectUploadableFiles(themeFileSystem: ThemeFileSystem, remoteChecksums * We use this 2d array to batch files of the same type together * while maintaining the order between file types. The files with * dependencies we have are: - * 1. Layout files don't necessarily need to be the first, but they must uploaded before templates. - * 2. Liquid blocks need to be uploaded before sections - * 3. Liquid sections need to be uploaded afterwards - * 4. JSON sections need to be uploaded after sections - * 5. JSON templates need to be uploaded after all sections and layouts - * 6. Contextualized templates should be uploaded after as they are variations of templates - * 7. Config files must be the last ones, but we need to upload config/settings_schema.json first, followed by config/settings_data.json + * + * 1. config/settings_schema.json must be uploaded FIRST. It declares the + * theme-level settings that block / section / section-group / template + * validators resolve dynamic-source defaults against (e.g. defaults of + * the form {{ settings.. }}). + * 2. Layout files don't necessarily need to be the first, but they must be + * uploaded before templates. + * 3. Liquid blocks need to be uploaded before sections + * 4. Liquid sections need to be uploaded afterwards + * 5. JSON sections need to be uploaded after sections + * 6. JSON templates need to be uploaded after all sections and layouts + * 7. Contextualized templates should be uploaded after as they are + * variations of templates + * 8. config/settings_data.json must be uploaded LAST. Its current and + * presets are validated against the freshly-uploaded + * settings_schema.json, and presets can reference sections and + * templates uploaded in earlier steps. * * The files with no dependencies we have are: * - The other Liquid files (for example, snippets, and liquid templates) @@ -336,13 +348,14 @@ function orderFilesToBeUploaded(files: ChecksumWithSize[]): { independentFiles: [fileSets.otherLiquidFiles, fileSets.otherJsonFiles, fileSets.staticAssetFiles], // Follow order of dependencies: dependentFiles: [ + fileSets.configSchemaFile, fileSets.layoutFiles, fileSets.blockLiquidFiles, fileSets.sectionLiquidFiles, fileSets.sectionJsonFiles, fileSets.templateJsonFiles, fileSets.contextualizedJsonFiles, - fileSets.configFiles, + fileSets.configDataFile, ], } } @@ -379,7 +392,7 @@ function calculateLocalChecksums(localThemeFileSystem: ThemeFileSystem): Checksu localThemeFileSystem.files.forEach((file, key) => { // Text files: use UTF-8 byte count // Binary files: use base64 length - const size = file.value ? Buffer.byteLength(file.value, 'utf8') : (file.attachment?.length ?? 0) + const size = file.value ? Buffer.byteLength(file.value, 'utf8') : file.attachment?.length ?? 0 checksums.push({ key,