From d0f596fe08e5d1d120890af81c85b86fed3862ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 2 Jan 2025 13:47:10 +0100 Subject: [PATCH 1/3] Execute addUidToToml in app-context for all commands --- .../app/src/cli/models/app/identifiers.ts | 25 +++++++++++-------- packages/app/src/cli/services/app-context.ts | 4 +++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/app/src/cli/models/app/identifiers.ts b/packages/app/src/cli/models/app/identifiers.ts index e594cc7b334..93974c88948 100644 --- a/packages/app/src/cli/models/app/identifiers.ts +++ b/packages/app/src/cli/models/app/identifiers.ts @@ -48,17 +48,9 @@ interface UpdateAppIdentifiersOptions { * @returns An copy of the app with the environment updated to reflect the updated identifiers. */ export async function updateAppIdentifiers( - {app, identifiers, command, developerPlatformClient}: UpdateAppIdentifiersOptions, + {app, identifiers, command}: UpdateAppIdentifiersOptions, systemEnvironment = process.env, ): Promise { - if (developerPlatformClient.supportsAtomicDeployments) { - // We can't update the TOML files in parallel because some extensions might share the same file - for (const extension of app.allExtensions) { - // eslint-disable-next-line no-await-in-loop - await addUidToToml(extension) - } - } - let dotenvFile = app.dotenv if (!dotenvFile) { @@ -95,8 +87,21 @@ export async function updateAppIdentifiers( return app } +export async function addUidToTomlsIfNecessary( + extensions: ExtensionInstance[], + developerPlatformClient: DeveloperPlatformClient, +) { + if (!developerPlatformClient.supportsAtomicDeployments) return + + // We can't update the TOML files in parallel because some extensions might share the same file + for (const extension of extensions) { + // eslint-disable-next-line no-await-in-loop + await addUidToToml(extension) + } +} + async function addUidToToml(extension: ExtensionInstance) { - if (!extension.isUUIDStrategyExtension) return + if (!extension.isUUIDStrategyExtension || extension.configuration.uid) return const tomlContents = await readFile(extension.configurationPath) const extensionConfig = decodeToml(tomlContents) diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index db2888d3f58..a9129176e61 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -9,6 +9,7 @@ import {getAppConfigurationState, loadAppUsingConfigurationState} from '../model import {RemoteAwareExtensionSpecification} from '../models/extensions/specification.js' import {AppLinkedInterface} from '../models/app/app.js' import metadata from '../metadata.js' +import {addUidToTomlsIfNecessary} from '../models/app/identifiers.js' import {tryParseInt} from '@shopify/cli-kit/common/string' export interface LoadedAppContextOutput { @@ -99,6 +100,9 @@ export async function linkedAppContext({ await logMetadata(remoteApp, forceRelink) + // Add UIDs to extension TOML files if using app-management. + await addUidToTomlsIfNecessary(localApp.allExtensions, developerPlatformClient) + return {app: localApp, remoteApp, developerPlatformClient, specifications, organization} } From 5b3a2c5fee0e917f252cab20c1da9d0585255bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 2 Jan 2025 16:18:03 +0100 Subject: [PATCH 2/3] Extract logic to a new file --- .../src/cli/models/app/identifiers.test.ts | 254 ------------------ .../app/src/cli/models/app/identifiers.ts | 43 +-- .../app/add-uid-to-extension-toml.test.ts | 183 +++++++++++++ .../services/app/add-uid-to-extension-toml.ts | 45 ++++ 4 files changed, 229 insertions(+), 296 deletions(-) create mode 100644 packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts create mode 100644 packages/app/src/cli/services/app/add-uid-to-extension-toml.ts diff --git a/packages/app/src/cli/models/app/identifiers.test.ts b/packages/app/src/cli/models/app/identifiers.test.ts index 6286d3d64c2..0b6b967db16 100644 --- a/packages/app/src/cli/models/app/identifiers.test.ts +++ b/packages/app/src/cli/models/app/identifiers.test.ts @@ -114,260 +114,6 @@ describe('updateAppIdentifiers', () => { } }) }) - - test('adds the missing uid to a simple TOML for atomic deployments', async () => { - await inTemporaryDirectory(async (tmpDir: string) => { - // Given - const uiExtension = await testUIExtension({directory: tmpDir}) - const app = testApp({ - directory: tmpDir, - allExtensions: [uiExtension], - }) - await writeFile( - uiExtension.configurationPath, - `name = "tae" -type = "theme"`, - ) - - // When - await updateAppIdentifiers( - { - app, - identifiers: { - app: 'FOO', - extensions: { - my_extension: 'BAR', - }, - }, - command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient({supportsAtomicDeployments: true}), - }, - {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, - ) - - // Then - const fileContent = await readFile(uiExtension.configurationPath) - expect(fileContent).toEqual(`name = "tae" -uid = "${uiExtension.uid}" -type = "theme"`) - }) - }) -}) - -test('does not change a simple TOML when the uid is already present for atomic deployments', async () => { - await inTemporaryDirectory(async (tmpDir: string) => { - // Given - const uiExtension = await testUIExtension({directory: tmpDir}) - const app = testApp({ - directory: tmpDir, - allExtensions: [uiExtension], - }) - await writeFile( - uiExtension.configurationPath, - `name = "tae" -uid = "${uiExtension.uid}" -type = "theme"`, - ) - - // When - await updateAppIdentifiers( - { - app, - identifiers: { - app: 'FOO', - extensions: { - my_extension: 'BAR', - }, - }, - command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient({supportsAtomicDeployments: true}), - }, - {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, - ) - - // Then - const fileContent = await readFile(uiExtension.configurationPath) - expect(fileContent).toEqual(`name = "tae" -uid = "${uiExtension.uid}" -type = "theme"`) - }) -}) - -test('adds the missing uid to a unified config TOML for atomic deployments', async () => { - await inTemporaryDirectory(async (tmpDir: string) => { - // Given - const uiExtension = await testUIExtension({ - directory: tmpDir, - configuration: { - name: 'Extension 1', - handle: 'ext1', - type: 'ui_extension', - metafields: [], - }, - }) - const app = testApp({ - directory: tmpDir, - allExtensions: [uiExtension], - }) - await writeFile( - uiExtension.configurationPath, - `api_version = "2024-04" -[[extensions]] -name = "Extension 1" -handle = "ext1" -type = "ui_extension"`, - ) - - // When - await updateAppIdentifiers( - { - app, - identifiers: { - app: 'FOO', - extensions: { - my_extension: 'BAR', - }, - }, - command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient({supportsAtomicDeployments: true}), - }, - {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, - ) - - // Then - const fileContent = await readFile(uiExtension.configurationPath) - expect(fileContent).toEqual(`api_version = "2024-04" -[[extensions]] -name = "Extension 1" -handle = "ext1" -uid = "${uiExtension.uid}" -type = "ui_extension"`) - }) -}) - -test('does not change a unified config TOML when the uid is already present for atomic deployments', async () => { - await inTemporaryDirectory(async (tmpDir: string) => { - // Given - const uiExtension = await testUIExtension({ - directory: tmpDir, - configuration: { - name: 'Extension 1', - handle: 'ext1', - type: 'ui_extension', - metafields: [], - }, - }) - const app = testApp({ - directory: tmpDir, - allExtensions: [uiExtension], - }) - await writeFile( - uiExtension.configurationPath, - `api_version = "2024-04" -[[extensions]] -name = "Extension 1" -handle = "ext1" -uid = "${uiExtension.uid}" -type = "ui_extension"`, - ) - - // When - await updateAppIdentifiers( - { - app, - identifiers: { - app: 'FOO', - extensions: { - my_extension: 'BAR', - }, - }, - command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient({supportsAtomicDeployments: true}), - }, - {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, - ) - - // Then - const fileContent = await readFile(uiExtension.configurationPath) - expect(fileContent).toEqual(`api_version = "2024-04" -[[extensions]] -name = "Extension 1" -handle = "ext1" -uid = "${uiExtension.uid}" -type = "ui_extension"`) - }) -}) - -test('adds the missing uids to a unified config TOML with multiple extensions for atomic deployments', async () => { - await inTemporaryDirectory(async (tmpDir: string) => { - // Given - const uiExtension1 = await testUIExtension({ - directory: tmpDir, - configuration: { - name: 'Extension 1', - handle: 'ext1', - type: 'ui_extension', - metafields: [], - }, - }) - const uiExtension2 = await testUIExtension({ - directory: tmpDir, - configuration: { - name: 'Extension 2', - handle: 'ext2', - type: 'ui_extension', - metafields: [], - }, - }) - const app = testApp({ - directory: tmpDir, - allExtensions: [uiExtension1, uiExtension2], - }) - await writeFile( - uiExtension1.configurationPath, - `api_version = "2024-04" -[[extensions]] -name = "t:name" -handle = "ext2" -type = "ui_extension" - -[[extensions]] -name = "t:name" -handle = "ext1" -type = "ui_extension"`, - ) - - // When - await updateAppIdentifiers( - { - app, - identifiers: { - app: 'FOO', - extensions: { - my_extension: 'BAR', - }, - }, - command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient({supportsAtomicDeployments: true}), - }, - {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, - ) - - // Then - const fileContent = await readFile(uiExtension1.configurationPath) - expect(fileContent).toEqual(`api_version = "2024-04" -[[extensions]] -name = "t:name" -handle = "ext2" -uid = "${uiExtension2.uid}" -type = "ui_extension" - -[[extensions]] -name = "t:name" -handle = "ext1" -uid = "${uiExtension1.uid}" -type = "ui_extension"`) - }) }) test('does not change a unified config TOML with multiple when the uid is already present for atomic deployments', async () => { diff --git a/packages/app/src/cli/models/app/identifiers.ts b/packages/app/src/cli/models/app/identifiers.ts index 93974c88948..b5397635093 100644 --- a/packages/app/src/cli/models/app/identifiers.ts +++ b/packages/app/src/cli/models/app/identifiers.ts @@ -5,8 +5,7 @@ import {patchEnvFile} from '@shopify/cli-kit/node/dot-env' import {constantize} from '@shopify/cli-kit/common/string' import {joinPath} from '@shopify/cli-kit/node/path' import {fileExists, readFile, writeFile} from '@shopify/cli-kit/node/fs' -import {deepCompare, getPathValue} from '@shopify/cli-kit/common/object' -import {decodeToml} from '@shopify/cli-kit/node/toml' +import {deepCompare} from '@shopify/cli-kit/common/object' import type {AppInterface} from './app.js' export interface IdentifiersExtensions { @@ -87,46 +86,6 @@ export async function updateAppIdentifiers( return app } -export async function addUidToTomlsIfNecessary( - extensions: ExtensionInstance[], - developerPlatformClient: DeveloperPlatformClient, -) { - if (!developerPlatformClient.supportsAtomicDeployments) return - - // We can't update the TOML files in parallel because some extensions might share the same file - for (const extension of extensions) { - // eslint-disable-next-line no-await-in-loop - await addUidToToml(extension) - } -} - -async function addUidToToml(extension: ExtensionInstance) { - if (!extension.isUUIDStrategyExtension || extension.configuration.uid) return - - const tomlContents = await readFile(extension.configurationPath) - const extensionConfig = decodeToml(tomlContents) - const extensions = getPathValue(extensionConfig, 'extensions') as ExtensionInstance[] - - if ('uid' in extensionConfig) return - if (extensions) { - const currentExtension = extensions.find((ext) => ext.handle === extension.handle) - if (currentExtension && 'uid' in currentExtension) return - } - - let updatedTomlContents = tomlContents - if (extensions?.length > 1) { - // If the TOML has multiple extensions, we look for the correct handle to add the uid below - const regex = new RegExp(`(\\n?(\\s*)handle\\s*=\\s*"${extension.handle}")`) - updatedTomlContents = tomlContents.replace(regex, `$1\n$2uid = "${extension.uid}"`) - } else { - // If the TOML has only one extension, we add the uid before the type, which is always present - if ('uid' in extensionConfig) return - const regex = /\n?((\s*)type\s*=\s*"\S*")/ - updatedTomlContents = tomlContents.replace(regex, `$2\nuid = "${extension.uid}"\n$1`) - } - await writeFile(extension.configurationPath, updatedTomlContents) -} - interface GetAppIdentifiersOptions { app: AppInterface } diff --git a/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts b/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts new file mode 100644 index 00000000000..ded45e32c73 --- /dev/null +++ b/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts @@ -0,0 +1,183 @@ +import {addUidToTomlsIfNecessary} from './add-uid-to-extension-toml.js' +import {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import {testDeveloperPlatformClient, testUIExtension} from '../../models/app/app.test-data.js' +import {describe, test, expect} from 'vitest' +import {writeFile, readFile, inTemporaryDirectory} from '@shopify/cli-kit/node/fs' +import {joinPath} from '@shopify/cli-kit/node/path' + +describe('addUidToTomlsIfNecessary', () => { + test('skips if platform does not support atomic deployments', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const tomlPath = joinPath(tmpDir, 'extension.toml') + const tomlContent = ` + type = "checkout_ui_extension" + handle = "my-extension" + ` + await writeFile(tomlPath, tomlContent) + + const extension = await testUIExtension({ + directory: tmpDir, + configuration: { + handle: 'my-extension', + type: 'checkout_ui_extension', + name: 'test', + }, + configurationPath: tomlPath, + uid: '123', + }) + + const client = testDeveloperPlatformClient({supportsAtomicDeployments: false}) + + // When + await addUidToTomlsIfNecessary([extension], client) + + // Then + const updatedContent = await readFile(tomlPath) + expect(updatedContent).toBe(tomlContent) + }) + }) + + test('adds uid to single extension TOML', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const tomlPath = joinPath(tmpDir, 'extension.toml') + const tomlContent = ` + type = "checkout_ui_extension" + handle = "my-extension" + ` + await writeFile(tomlPath, tomlContent) + + const extension = { + configurationPath: tomlPath, + handle: 'my-extension', + isUUIDStrategyExtension: true, + uid: '123', + configuration: {}, + } as ExtensionInstance + + const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + + // When + await addUidToTomlsIfNecessary([extension], client) + + // Then + const updatedContent = await readFile(tomlPath) + expect(updatedContent).toContain('uid = "123"') + expect(updatedContent).toMatch(/uid.*type/s) + }) + }) + + test('adds uid to multi-extension TOML', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const tomlPath = joinPath(tmpDir, 'extension.toml') + const tomlContent = ` + [[extensions]] + type = "checkout_ui_extension" + handle = "extension-1" + + [[extensions]] + type = "checkout_ui_extension" + handle = "extension-2" + ` + await writeFile(tomlPath, tomlContent) + + const extension = { + configurationPath: tomlPath, + handle: 'extension-2', + isUUIDStrategyExtension: true, + uid: '456', + configuration: {}, + } as ExtensionInstance + + const extension1 = { + configurationPath: tomlPath, + handle: 'extension-1', + isUUIDStrategyExtension: true, + uid: '123', + configuration: {}, + } as ExtensionInstance + + const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + + // When + await addUidToTomlsIfNecessary([extension, extension1], client) + + // Then + const updatedContent = await readFile(tomlPath) + expect(updatedContent).toBe(` + [[extensions]] + type = "checkout_ui_extension" + handle = "extension-1" + uid = "123" + + [[extensions]] + type = "checkout_ui_extension" + handle = "extension-2" + uid = "456" + `) + }) + }) + + test('skips if extension already has uid in configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const tomlPath = joinPath(tmpDir, 'extension.toml') + const tomlContent = ` + type = "checkout_ui_extension" + handle = "my-extension" + uid = "existing-uid" + ` + await writeFile(tomlPath, tomlContent) + + const extension = { + configurationPath: tomlPath, + handle: 'my-extension', + isUUIDStrategyExtension: true, + uid: '123', + configuration: { + uid: 'existing-uid', + }, + } as ExtensionInstance + + const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + + // When + await addUidToTomlsIfNecessary([extension], client) + + // Then + const updatedContent = await readFile(tomlPath) + expect(updatedContent).toBe(tomlContent) + }) + }) + + test('skips if extension is not UUID strategy extension', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const tomlPath = joinPath(tmpDir, 'extension.toml') + const tomlContent = ` + type = "checkout_ui_extension" + handle = "my-extension" + ` + await writeFile(tomlPath, tomlContent) + + const extension = { + configurationPath: tomlPath, + handle: 'my-extension', + isUUIDStrategyExtension: false, + uid: '123', + configuration: {}, + } as ExtensionInstance + + const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + + // When + await addUidToTomlsIfNecessary([extension], client) + + // Then + const updatedContent = await readFile(tomlPath) + expect(updatedContent).toBe(tomlContent) + }) + }) +}) diff --git a/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts b/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts new file mode 100644 index 00000000000..102727a1a98 --- /dev/null +++ b/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts @@ -0,0 +1,45 @@ +import {ExtensionInstance} from '../../models/extensions/extension-instance.js' +import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' +import {decodeToml} from '@shopify/cli-kit/node/toml' +import {readFile, writeFile} from '@shopify/cli-kit/node/fs' +import {getPathValue} from '@shopify/cli-kit/common/object' + +export async function addUidToTomlsIfNecessary( + extensions: ExtensionInstance[], + developerPlatformClient: DeveloperPlatformClient, +) { + if (!developerPlatformClient.supportsAtomicDeployments) return + + // We can't update the TOML files in parallel because some extensions might share the same file + for (const extension of extensions) { + // eslint-disable-next-line no-await-in-loop + await addUidToToml(extension) + } +} + +async function addUidToToml(extension: ExtensionInstance) { + if (!extension.isUUIDStrategyExtension || extension.configuration.uid) return + + const tomlContents = await readFile(extension.configurationPath) + const extensionConfig = decodeToml(tomlContents) + const extensions = getPathValue(extensionConfig, 'extensions') as ExtensionInstance[] + + if ('uid' in extensionConfig) return + if (extensions) { + const currentExtension = extensions.find((ext) => ext.handle === extension.handle) + if (currentExtension && 'uid' in currentExtension) return + } + + let updatedTomlContents = tomlContents + if (extensions?.length > 1) { + // If the TOML has multiple extensions, we look for the correct handle to add the uid below + const regex = new RegExp(`(\\n?(\\s*)handle\\s*=\\s*"${extension.handle}")`) + updatedTomlContents = tomlContents.replace(regex, `$1\n$2uid = "${extension.uid}"`) + } else { + // If the TOML has only one extension, we add the uid before the type, which is always present + if ('uid' in extensionConfig) return + const regex = /\n?((\s*)type\s*=\s*"\S*")/ + updatedTomlContents = tomlContents.replace(regex, `$2\nuid = "${extension.uid}"\n$1`) + } + await writeFile(extension.configurationPath, updatedTomlContents) +} From c431e4fc5de33fd143257b4b2350013f1ff40b41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 2 Jan 2025 16:43:26 +0100 Subject: [PATCH 3/3] Fix import --- packages/app/src/cli/services/app-context.test.ts | 1 + packages/app/src/cli/services/app-context.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index c6bc55bbcd5..12dc90d410f 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -18,6 +18,7 @@ vi.mock('./generate/fetch-extension-specifications.js') vi.mock('./app/config/link.js') vi.mock('./context.js') vi.mock('./dev/fetch.js') +vi.mock('./app/add-uid-to-extension-toml.js') async function writeAppConfig(tmp: string, content: string) { const appConfigPath = joinPath(tmp, 'shopify.app.toml') diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index a9129176e61..5b7a2216cd1 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -3,13 +3,13 @@ import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' import {fetchSpecifications} from './generate/fetch-extension-specifications.js' import link from './app/config/link.js' import {fetchOrgFromId} from './dev/fetch.js' +import {addUidToTomlsIfNecessary} from './app/add-uid-to-extension-toml.js' import {Organization, OrganizationApp} from '../models/organization.js' import {DeveloperPlatformClient, selectDeveloperPlatformClient} from '../utilities/developer-platform-client.js' import {getAppConfigurationState, loadAppUsingConfigurationState} from '../models/app/loader.js' import {RemoteAwareExtensionSpecification} from '../models/extensions/specification.js' import {AppLinkedInterface} from '../models/app/app.js' import metadata from '../metadata.js' -import {addUidToTomlsIfNecessary} from '../models/app/identifiers.js' import {tryParseInt} from '@shopify/cli-kit/common/string' export interface LoadedAppContextOutput {