diff --git a/packages/app/src/cli/commands/app/config/link.ts b/packages/app/src/cli/commands/app/config/link.ts index 91bd549aff0..1783c844eba 100644 --- a/packages/app/src/cli/commands/app/config/link.ts +++ b/packages/app/src/cli/commands/app/config/link.ts @@ -34,7 +34,7 @@ export default class ConfigLink extends AppLinkedCommand { directory: flags.path, clientId: undefined, forceRelink: false, - userProvidedConfigName: result.state.configurationFileName, + userProvidedConfigName: result.configFileName, }) return {app} diff --git a/packages/app/src/cli/models/app/config-file-naming.test.ts b/packages/app/src/cli/models/app/config-file-naming.test.ts new file mode 100644 index 00000000000..ac8a24be6ab --- /dev/null +++ b/packages/app/src/cli/models/app/config-file-naming.test.ts @@ -0,0 +1,58 @@ +import { + getAppConfigurationFileName, + getAppConfigurationShorthand, + isValidFormatAppConfigurationFileName, +} from './config-file-naming.js' +import {describe, expect, test} from 'vitest' + +describe('getAppConfigurationFileName', () => { + test('returns default filename when no config name is provided', () => { + expect(getAppConfigurationFileName()).toBe('shopify.app.toml') + expect(getAppConfigurationFileName(undefined)).toBe('shopify.app.toml') + }) + + test('returns the config name as-is when it matches the valid format', () => { + expect(getAppConfigurationFileName('shopify.app.production.toml')).toBe('shopify.app.production.toml') + expect(getAppConfigurationFileName('shopify.app.staging.toml')).toBe('shopify.app.staging.toml') + expect(getAppConfigurationFileName('shopify.app.toml')).toBe('shopify.app.toml') + }) + + test('slugifies arbitrary strings into the filename pattern', () => { + expect(getAppConfigurationFileName('production')).toBe('shopify.app.production.toml') + expect(getAppConfigurationFileName('My Store')).toBe('shopify.app.my-store.toml') + }) +}) + +describe('getAppConfigurationShorthand', () => { + test('returns undefined for the default config filename', () => { + expect(getAppConfigurationShorthand('shopify.app.toml')).toBeUndefined() + expect(getAppConfigurationShorthand('/some/path/shopify.app.toml')).toBeUndefined() + }) + + test('extracts the shorthand from a named config', () => { + expect(getAppConfigurationShorthand('shopify.app.production.toml')).toBe('production') + expect(getAppConfigurationShorthand('/path/to/shopify.app.staging.toml')).toBe('staging') + expect(getAppConfigurationShorthand('shopify.app.my-store.toml')).toBe('my-store') + }) + + test('returns undefined for non-matching filenames', () => { + expect(getAppConfigurationShorthand('random.toml')).toBeUndefined() + expect(getAppConfigurationShorthand('shopify.web.toml')).toBeUndefined() + }) +}) + +describe('isValidFormatAppConfigurationFileName', () => { + test('returns true for valid app configuration filenames', () => { + expect(isValidFormatAppConfigurationFileName('shopify.app.toml')).toBe(true) + expect(isValidFormatAppConfigurationFileName('shopify.app.production.toml')).toBe(true) + expect(isValidFormatAppConfigurationFileName('shopify.app.my-store.toml')).toBe(true) + expect(isValidFormatAppConfigurationFileName('shopify.app.test_env.toml')).toBe(true) + }) + + test('returns false for invalid filenames', () => { + expect(isValidFormatAppConfigurationFileName('production')).toBe(false) + expect(isValidFormatAppConfigurationFileName('shopify.web.toml')).toBe(false) + expect(isValidFormatAppConfigurationFileName('shopify.app..toml')).toBe(false) + expect(isValidFormatAppConfigurationFileName('')).toBe(false) + }) +}) diff --git a/packages/app/src/cli/models/app/config-file-naming.ts b/packages/app/src/cli/models/app/config-file-naming.ts new file mode 100644 index 00000000000..8d8823631e0 --- /dev/null +++ b/packages/app/src/cli/models/app/config-file-naming.ts @@ -0,0 +1,42 @@ +import {configurationFileNames} from '../../constants.js' +import {slugify} from '@shopify/cli-kit/common/string' +import {basename} from '@shopify/cli-kit/node/path' + +const appConfigurationFileNameRegex = /^shopify\.app(\.[-\w]+)?\.toml$/ +export type AppConfigurationFileName = 'shopify.app.toml' | `shopify.app.${string}.toml` + +/** + * Gets the name of the app configuration file (e.g. `shopify.app.production.toml`) based on a provided config name. + * + * @param configName - Optional config name to base the file name upon + * @returns Either the default app configuration file name (`shopify.app.toml`), the given config name (if it matched the valid format), or `shopify.app..toml` if it was an arbitrary string + */ +export function getAppConfigurationFileName(configName?: string): AppConfigurationFileName { + if (!configName) { + return configurationFileNames.app + } + + if (isValidFormatAppConfigurationFileName(configName)) { + return configName + } else { + return `shopify.app.${slugify(configName)}.toml` + } +} + +/** + * Given a path to an app configuration file, extract the shorthand section from the file name. + * + * This is undefined for `shopify.app.toml` files, or returns e.g. `production` for `shopify.app.production.toml`. + */ +export function getAppConfigurationShorthand(path: string) { + const match = basename(path).match(appConfigurationFileNameRegex) + return match?.[1]?.slice(1) +} + +/** Checks if configName is a valid one (`shopify.app.toml`, or `shopify.app..toml`) */ +export function isValidFormatAppConfigurationFileName(configName: string): configName is AppConfigurationFileName { + if (appConfigurationFileNameRegex.test(configName)) { + return true + } + return false +} diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index 7b44163ed3f..d5c3c321930 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -3,17 +3,15 @@ import { getAppConfigurationFileName, loadApp, loadOpaqueApp, - loadDotEnv, parseConfigurationObject, checkFolderIsValidApp, AppLoaderMode, - getAppConfigurationState, + getAppConfigurationContext, loadConfigForAppCreation, reloadApp, - loadHiddenConfig, } from './loader.js' import {parseHumanReadableError} from './error-parsing.js' -import {App, AppConfiguration, AppInterface, AppLinkedInterface, AppSchema, WebConfigurationSchema} from './app.js' +import {App, AppInterface, AppLinkedInterface, AppSchema, WebConfigurationSchema} from './app.js' import {DEFAULT_CONFIG, buildVersionedAppSchema, getWebhookConfig} from './app.test-data.js' import {ExtensionInstance} from '../extensions/extension-instance.js' import {configurationFileNames, blocks} from '../../constants.js' @@ -33,7 +31,7 @@ import { PackageJson, pnpmWorkspaceFile, } from '@shopify/cli-kit/node/node-package-manager' -import {inTemporaryDirectory, moveFile, mkdir, mkTmpDir, rmdir, writeFile, readFile} from '@shopify/cli-kit/node/fs' +import {inTemporaryDirectory, moveFile, mkdir, mkTmpDir, rmdir, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath, dirname, cwd, normalizePath} from '@shopify/cli-kit/node/path' import {platformAndArch} from '@shopify/cli-kit/node/os' import {outputContent, outputToken} from '@shopify/cli-kit/node/output' @@ -256,7 +254,7 @@ describe('load', () => { // When/Then await expect(loadApp({directory: tmp, specifications, userProvidedConfigName: undefined})).rejects.toThrow( - `Couldn't find directory ${tmp}`, + /Could not find a Shopify app configuration file/, ) }) }) @@ -267,7 +265,7 @@ describe('load', () => { // When/Then await expect(loadApp({directory: currentDir, specifications, userProvidedConfigName: undefined})).rejects.toThrow( - `Couldn't find an app toml file at ${currentDir}`, + /Could not find a Shopify app configuration file/, ) }) @@ -485,7 +483,7 @@ describe('load', () => { await makeBlockDir({name: 'my-extension'}) // When - await expect(loadTestingApp()).rejects.toThrow(/Couldn't find an app toml file at/) + await expect(loadTestingApp()).rejects.toThrow(/Could not find a Shopify app configuration file/) }) test('throws an error if the extension configuration file is invalid', async () => { @@ -1058,7 +1056,7 @@ describe('load', () => { await makeBlockDir({name: 'my-functions'}) // When - await expect(loadTestingApp()).rejects.toThrow(/Couldn't find an app toml file at/) + await expect(loadTestingApp()).rejects.toThrow(/Could not find a Shopify app configuration file/) }) test('throws an error if the function configuration file is invalid', async () => { @@ -2813,46 +2811,6 @@ describe('getAppConfigurationShorthand', () => { }) }) -describe('loadDotEnv', () => { - test('it returns undefined if the env is missing', async () => { - await inTemporaryDirectory(async (tmp) => { - // When - const got = await loadDotEnv(tmp, joinPath(tmp, 'shopify.app.toml')) - - // Then - expect(got).toBeUndefined() - }) - }) - - test('it loads from the default env file', async () => { - await inTemporaryDirectory(async (tmp) => { - // Given - await writeFile(joinPath(tmp, '.env'), 'FOO="bar"') - - // When - const got = await loadDotEnv(tmp, joinPath(tmp, 'shopify.app.toml')) - - // Then - expect(got).toBeDefined() - expect(got!.variables.FOO).toEqual('bar') - }) - }) - - test('it loads from the config specific env file', async () => { - await inTemporaryDirectory(async (tmp) => { - // Given - await writeFile(joinPath(tmp, '.env.staging'), 'FOO="bar"') - - // When - const got = await loadDotEnv(tmp, joinPath(tmp, 'shopify.app.staging.toml')) - - // Then - expect(got).toBeDefined() - expect(got!.variables.FOO).toEqual('bar') - }) - }) -}) - describe('checkFolderIsValidApp', () => { test('throws an error if the folder does not contain a shopify.app.toml file', async () => { await inTemporaryDirectory(async (tmp) => { @@ -3484,46 +3442,26 @@ describe('WebhooksSchema', () => { } }) -describe('getAppConfigurationState', () => { +describe('getAppConfigurationContext', () => { test.each([ - [ - `client_id="abcdef"`, - { - basicConfiguration: { - client_id: 'abcdef', - }, - isLinked: true, - }, - ], + [`client_id="abcdef"`, {client_id: 'abcdef'}, true], [ `client_id="abcdef" something_extra="keep"`, - { - basicConfiguration: { - client_id: 'abcdef', - something_extra: 'keep', - }, - isLinked: true, - }, + {client_id: 'abcdef', something_extra: 'keep'}, + true, ], - [ - `client_id=""`, - { - basicConfiguration: { - client_id: '', - }, - isLinked: false, - }, - ], - ])('loads from %s', async (content, resultShouldContain) => { + [`client_id=""`, {client_id: ''}, false], + ])('loads from %s', async (content, expectedContent, expectedIsLinked) => { await inTemporaryDirectory(async (tmpDir) => { const appConfigPath = joinPath(tmpDir, 'shopify.app.toml') const packageJsonPath = joinPath(tmpDir, 'package.json') await writeFile(appConfigPath, content) await writeFile(packageJsonPath, '{}') - const state = await getAppConfigurationState(tmpDir, undefined) - expect(state).toMatchObject(resultShouldContain) + const {activeConfig} = await getAppConfigurationContext(tmpDir, undefined) + expect(activeConfig.file.content).toMatchObject(expectedContent) + expect(activeConfig.isLinked).toBe(expectedIsLinked) }) }) @@ -3535,10 +3473,10 @@ describe('getAppConfigurationState', () => { await writeFile(appConfigPath, content) await writeFile(packageJsonPath, '{}') - const result = await getAppConfigurationState(tmpDir, undefined) + const {activeConfig} = await getAppConfigurationContext(tmpDir, undefined) - expect(result.basicConfiguration.client_id).toBe('') - expect(result.isLinked).toBe(false) + expect(activeConfig.file.content.client_id).toBe('') + expect(activeConfig.isLinked).toBe(false) }) }) }) @@ -3683,117 +3621,6 @@ value = true }) }) -describe('loadHiddenConfig', () => { - test('returns empty object if hidden config file does not exist', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - client_id: '12345', - } as AppConfiguration - await writeFile(joinPath(tmpDir, '.gitignore'), '') - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({}) - - // Verify empty config file was created - const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json') - const fileContent = await readFile(hiddenConfigPath) - expect(JSON.parse(fileContent)).toEqual({}) - }) - }) - - test('returns config for client_id if hidden config file exists', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - client_id: '12345', - } as AppConfiguration - const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json') - await mkdir(dirname(hiddenConfigPath)) - await writeFile( - hiddenConfigPath, - JSON.stringify({ - '12345': {someKey: 'someValue'}, - 'other-id': {otherKey: 'otherValue'}, - }), - ) - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({someKey: 'someValue'}) - }) - }) - - test('returns empty object if client_id not found in existing hidden config', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - client_id: 'not-found', - } as AppConfiguration - const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json') - await mkdir(dirname(hiddenConfigPath)) - await writeFile( - hiddenConfigPath, - JSON.stringify({ - 'other-id': {someKey: 'someValue'}, - }), - ) - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({}) - }) - }) - - test('returns config if hidden config has an old format with just a dev_store_url', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - client_id: 'not-found', - } as AppConfiguration - const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json') - await mkdir(dirname(hiddenConfigPath)) - await writeFile( - hiddenConfigPath, - JSON.stringify({ - dev_store_url: 'https://dev-store.myshopify.com', - }), - ) - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({dev_store_url: 'https://dev-store.myshopify.com'}) - }) - }) - - test('returns empty object if hidden config file is invalid JSON', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - client_id: '12345', - } as AppConfiguration - const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json') - await mkdir(dirname(hiddenConfigPath)) - await writeFile(hiddenConfigPath, 'invalid json') - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({}) - }) - }) -}) - describe('loadOpaqueApp', () => { let specifications: ExtensionSpecification[] diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 2521101367c..bc413b4eaca 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -6,58 +6,52 @@ import { WebType, getAppScopesArray, AppConfigurationInterface, - AppConfiguration, CurrentAppConfiguration, getAppVersionedSchema, AppSchema, - BasicAppConfigurationWithoutModules, SchemaForConfig, AppLinkedInterface, - AppHiddenConfig, } from './app.js' import {parseHumanReadableError} from './error-parsing.js' +import { + getAppConfigurationFileName, + getAppConfigurationShorthand, + type AppConfigurationFileName, +} from './config-file-naming.js' import {configurationFileNames, dotEnvFileNames} from '../../constants.js' import metadata from '../../metadata.js' import {ExtensionInstance} from '../extensions/extension-instance.js' import {ExtensionsArraySchema, UnifiedSchema} from '../extensions/schemas.js' -import { - ExtensionSpecification, - RemoteAwareExtensionSpecification, - isAppConfigSpecification, -} from '../extensions/specification.js' -import {getCachedAppInfo} from '../../services/local-storage.js' -import use from '../../services/app/config/use.js' +import {ExtensionSpecification, isAppConfigSpecification} from '../extensions/specification.js' import {CreateAppOptions, Flag} from '../../utilities/developer-platform-client.js' import {findConfigFiles} from '../../prompts/config.js' import {WebhookSubscriptionSpecIdentifier} from '../extensions/specifications/app_config_webhook_subscription.js' import {WebhooksSchema} from '../extensions/specifications/app_config_webhook_schemas/webhooks_schema.js' -import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js' -import {patchAppHiddenConfigFile} from '../../services/app/patch-app-configuration-file.js' -import {getOrCreateAppConfigHiddenPath} from '../../utilities/app/config/hidden-app-config.js' import {ApplicationURLs, generateApplicationURLs} from '../../services/dev/urls.js' +import {Project} from '../project/project.js' +import {selectActiveConfig} from '../project/active-config.js' +import { + resolveDotEnv, + resolveHiddenConfig, + extensionFilesForConfig, + webFilesForConfig, +} from '../project/config-selection.js' import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning' -import {fileExists, readFile, glob, findPathUp, fileExistsSync} from '@shopify/cli-kit/node/fs' +import {fileExists, readFile, fileExistsSync} from '@shopify/cli-kit/node/fs' import {TomlFile, TomlParseError} from '@shopify/cli-kit/node/toml/toml-file' import {zod} from '@shopify/cli-kit/node/schema' -import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env' -import { - getDependencies, - getPackageManager, - PackageManager, - usesWorkspaces as appUsesWorkspaces, -} from '@shopify/cli-kit/node/node-package-manager' +import {PackageManager} from '@shopify/cli-kit/node/node-package-manager' import {resolveFramework} from '@shopify/cli-kit/node/framework' import {hashString} from '@shopify/cli-kit/node/crypto' import {JsonMapType} from '@shopify/cli-kit/node/toml' import {joinPath, dirname, basename, relativePath, relativizePath} from '@shopify/cli-kit/node/path' import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent, outputDebug, OutputMessage, outputToken} from '@shopify/cli-kit/node/output' -import {joinWithAnd, slugify} from '@shopify/cli-kit/common/string' +import {joinWithAnd} from '@shopify/cli-kit/common/string' import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array' import {showNotificationsIfNeeded} from '@shopify/cli-kit/node/notifications-system' import ignore from 'ignore' - -const defaultExtensionDirectory = 'extensions/*' +import type {ActiveConfig} from '../project/active-config.js' /** * The mode in which the app is loaded, this affects how errors are handled: @@ -67,14 +61,25 @@ const defaultExtensionDirectory = 'extensions/*' */ export type AppLoaderMode = 'strict' | 'report' | 'local' +/** + * Narrow runtime state carried forward across app reloads. + * + * Replaces passing the entire previous AppInterface — only genuine runtime + * state (devUUIDs and tunnel URLs) needs to survive a reload. + */ +export interface ReloadState { + /** Extension handle → devUUID, preserved for dev-console stability across reloads */ + extensionDevUUIDs: Map + /** Previous dev tunnel URL, kept stable across reloads */ + previousDevURLs?: ApplicationURLs +} + type AbortOrReport = (errorMessage: OutputMessage, fallback: T, configurationPath: string) => T const abort: AbortOrReport = (errorMessage) => { throw new AbortError(errorMessage) } -const noopAbortOrReport: AbortOrReport = (_errorMessage, fallback, _configurationPath) => fallback - /** * Loads a configuration file, and returns its content as an unvalidated object. */ @@ -196,8 +201,10 @@ interface AppLoaderConstructorArgs< > { mode?: AppLoaderMode loadedConfiguration: ConfigurationLoaderResult - // Used when reloading an app, to avoid some expensive steps during loading. - previousApp?: AppLinkedInterface + // Pre-discovered project data — avoids re-scanning the filesystem for dependencies, package manager, etc. + project: Project + // Narrow runtime state from a previous app load, used during reloads + reloadState?: ReloadState } export async function checkFolderIsValidApp(directory: string) { @@ -209,39 +216,30 @@ export async function checkFolderIsValidApp(directory: string) { } export async function loadConfigForAppCreation(directory: string, name: string): Promise { - const state = await getAppConfigurationState(directory) - const config = state.basicConfiguration - const webs = await loadWebsForAppCreation(state.appDirectory, config.web_directories) + const {project, activeConfig} = await getAppConfigurationContext(directory) + const rawConfig = activeConfig.file.content + const webFiles = webFilesForConfig(project, activeConfig.file) + const webs = await Promise.all(webFiles.map((wf) => loadSingleWeb(wf.path, abort, wf.content))) const isLaunchable = webs.some((web) => isWebType(web, WebType.Frontend) || isWebType(web, WebType.Backend)) - const scopesArray = getAppScopesArray(config as CurrentAppConfiguration) + const scopesArray = getAppScopesArray(rawConfig as CurrentAppConfiguration) return { isLaunchable, scopesArray, name, - directory: state.appDirectory, + directory: project.directory, // By default, and ONLY for `app init`, we consider the app as embedded if it is launchable. isEmbedded: isLaunchable, } } -async function loadWebsForAppCreation(appDirectory: string, webDirectories?: string[]): Promise { - const webTomlPaths = await findWebConfigPaths(appDirectory, webDirectories) - return Promise.all(webTomlPaths.map((path) => loadSingleWeb(path))) -} - -async function findWebConfigPaths(appDirectory: string, webDirectories?: string[]): Promise { - const defaultWebDirectory = '**' - const webConfigGlobs = [...(webDirectories ?? [defaultWebDirectory])].map((webGlob) => { - return joinPath(appDirectory, webGlob, configurationFileNames.web) - }) - webConfigGlobs.push(`!${joinPath(appDirectory, '**/node_modules/**')}`) - return glob(webConfigGlobs) -} - -async function loadSingleWeb(webConfigPath: string, abortOrReport: AbortOrReport = abort): Promise { - const config = await parseConfigurationFile(WebConfigurationSchema, webConfigPath, abortOrReport) +async function loadSingleWeb( + webConfigPath: string, + abortOrReport: AbortOrReport = abort, + preloadedContent?: JsonMapType, +): Promise { + const config = await parseConfigurationFile(WebConfigurationSchema, webConfigPath, abortOrReport, preloadedContent) const roles = new Set('roles' in config ? config.roles : []) if ('type' in config) roles.add(config.type) const {type, ...processedWebConfiguration} = {...config, roles: Array.from(roles), type: undefined} @@ -256,22 +254,88 @@ async function loadSingleWeb(webConfigPath: string, abortOrReport: AbortOrReport * Load the local app from the given directory and using the provided extensions/functions specifications. * If the App contains extensions not supported by the current specs and mode is strict, it will throw an error. */ -export async function loadApp( - options: Omit, 'loadedConfiguration'> & { - directory: string - userProvidedConfigName: string | undefined - specifications: TModuleSpec[] - remoteFlags?: Flag[] - }, -): Promise> { - const specifications = options.specifications +export async function loadApp(options: { + directory: string + userProvidedConfigName: string | undefined + specifications: TModuleSpec[] + remoteFlags?: Flag[] + mode?: AppLoaderMode +}): Promise> { + const {project, activeConfig} = await getAppConfigurationContext(options.directory, options.userProvidedConfigName) + return loadAppFromContext({ + project, + activeConfig, + specifications: options.specifications, + remoteFlags: options.remoteFlags, + mode: options.mode, + }) +} + +/** + * Load an app from a pre-resolved Project and ActiveConfig. + * + * Use this when you already have a Project (e.g. from getAppConfigurationContext) + * instead of re-discovering from directory + configName. + */ +export async function loadAppFromContext(options: { + project: Project + activeConfig: ActiveConfig + specifications: TModuleSpec[] + remoteFlags?: Flag[] + mode?: AppLoaderMode + reloadState?: ReloadState + clientIdOverride?: string +}): Promise> { + const {project, activeConfig, specifications, remoteFlags = [], mode, reloadState, clientIdOverride} = options + + const rawConfig: JsonMapType = {...activeConfig.file.content} + if (clientIdOverride) { + rawConfig.client_id = clientIdOverride + } + + const appVersionedSchema = getAppVersionedSchema(specifications) + const configSchema = appVersionedSchema as SchemaForConfig + const configurationPath = activeConfig.file.path + const configurationFileName = basename(configurationPath) as AppConfigurationFileName + + const configuration = await parseConfigurationFile(configSchema, configurationPath, abort, rawConfig) + + const allClientIdsByConfigName = getAllLinkedConfigClientIds(project.appConfigFiles, { + [configurationFileName]: configuration.client_id, + }) - const state = await getAppConfigurationState(options.directory, options.userProvidedConfigName) - const loadedConfiguration = await loadAppConfigurationFromState(state, specifications, options.remoteFlags ?? []) + let gitTracked = false + try { + gitTracked = await checkIfGitTracked(project.directory, configurationPath) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // leave as false + } + + const configurationLoadResultMetadata: ConfigurationLoadResultMetadata = { + allClientIdsByConfigName, + usesLinkedConfig: true, + name: configurationFileName, + gitTracked, + source: activeConfig.source, + usesCliManagedUrls: configuration.build?.automatically_update_urls_on_dev, + } + + const loadedConfiguration: ConfigurationLoaderResult = { + directory: project.directory, + configPath: configurationPath, + configuration, + configurationLoadResultMetadata, + configSchema, + specifications, + remoteFlags, + } const loader = new AppLoader({ - mode: options.mode, + mode, loadedConfiguration, + project, + reloadState, }) return loader.loaded() } @@ -340,17 +404,16 @@ export async function loadOpaqueApp(options: { } catch { // loadApp failed - try loading as raw template config try { - const appDirectory = await getAppDirectory(options.directory) - const {configurationPath} = await getConfigurationPath(appDirectory, options.configName) + const project = await Project.load(options.directory) + const {configurationPath} = await getConfigurationPath(project.directory, options.configName) const rawConfig = await loadConfigurationFileContent(configurationPath) - const packageManager = await getPackageManager(appDirectory) return { state: 'loaded-template', rawConfig, scopes: extractScopesFromRawConfig(rawConfig), - appDirectory, - packageManager, + appDirectory: project.directory, + packageManager: project.packageManager, } // eslint-disable-next-line no-catch-all/no-catch-all } catch { @@ -361,68 +424,46 @@ export async function loadOpaqueApp(options: { } export async function reloadApp(app: AppLinkedInterface): Promise { - const state = await getAppConfigurationState(app.directory, basename(app.configPath)) - const loadedConfiguration = await loadAppConfigurationFromState(state, app.specifications, app.remoteFlags ?? []) - - const loader = new AppLoader({ - loadedConfiguration, - previousApp: app, - }) - - return loader.loaded() -} - -export async function loadAppUsingConfigurationState( - configState: AppConfigurationState, - { - specifications, - remoteFlags, - mode, - }: { - specifications: RemoteAwareExtensionSpecification[] - remoteFlags?: Flag[] - mode: AppLoaderMode - }, -): Promise> { - const loadedConfiguration = await loadAppConfigurationFromState(configState, specifications, remoteFlags ?? []) - - const loader = new AppLoader({ - mode, - loadedConfiguration, + const {project, activeConfig} = await getAppConfigurationContext(app.directory, basename(app.configPath)) + const reloadState: ReloadState = { + extensionDevUUIDs: new Map(app.allExtensions.map((ext) => [ext.handle, ext.devUUID])), + previousDevURLs: app.devApplicationURLs, + } + return loadAppFromContext({ + project, + activeConfig, + specifications: app.specifications, + remoteFlags: app.remoteFlags ?? [], + reloadState, }) - return loader.loaded() } -type LoadedAppConfigFromConfigState = CurrentAppConfiguration - export function getDotEnvFileName(configurationPath: string) { const configurationShorthand: string | undefined = getAppConfigurationShorthand(configurationPath) return configurationShorthand ? `${dotEnvFileNames.production}.${configurationShorthand}` : dotEnvFileNames.production } -export async function loadDotEnv(appDirectory: string, configurationPath: string): Promise { - let dotEnvFile: DotEnvFile | undefined - const dotEnvPath = joinPath(appDirectory, getDotEnvFileName(configurationPath)) - if (await fileExists(dotEnvPath)) { - dotEnvFile = await readAndParseDotEnv(dotEnvPath) - } - return dotEnvFile -} - class AppLoader { private readonly mode: AppLoaderMode private readonly errors: AppErrors = new AppErrors() private readonly specifications: TModuleSpec[] private readonly remoteFlags: Flag[] private readonly loadedConfiguration: ConfigurationLoaderResult - private readonly previousApp: AppLinkedInterface | undefined + private readonly reloadState: ReloadState | undefined + private readonly project: Project - constructor({mode, loadedConfiguration, previousApp}: AppLoaderConstructorArgs) { + constructor({mode, loadedConfiguration, reloadState, project}: AppLoaderConstructorArgs) { this.mode = mode ?? 'strict' this.specifications = loadedConfiguration.specifications this.remoteFlags = loadedConfiguration.remoteFlags this.loadedConfiguration = loadedConfiguration - this.previousApp = previousApp + this.reloadState = reloadState + this.project = project + } + + private get activeConfigFile(): TomlFile | undefined { + const configPath = this.loadedConfiguration.configPath + return this.project.appConfigFiles.find((file) => file.path === configPath) } async loaded() { @@ -431,24 +472,20 @@ class AppLoader { - const webTomlPaths = await findWebConfigPaths(appDirectory, webDirectories) - const webs = await Promise.all(webTomlPaths.map((path) => loadSingleWeb(path, this.abortOrReport.bind(this)))) + const activeConfig = this.activeConfigFile + const webFiles = activeConfig ? webFilesForConfig(this.project, activeConfig) : this.project.webConfigFiles + const webTomlPaths = webFiles.map((file) => file.path) + const webs = await Promise.all( + webFiles.map((webFile) => loadSingleWeb(webFile.path, this.abortOrReport.bind(this), webFile.content)), + ) this.validateWebs(webs) - const webTomlsInStandardLocation = await glob(joinPath(appDirectory, `web/**/${configurationFileNames.web}`)) - const usedCustomLayout = webDirectories !== undefined || webTomlsInStandardLocation.length !== webTomlPaths.length + const allWebsUnderStandardDir = webTomlPaths.every((webPath) => { + const rel = relativePath(appDirectory, webPath) + return rel.startsWith('web/') + }) + const usedCustomLayout = webDirectories !== undefined || !allWebsUnderStandardDir return {webs, usedCustomLayout} } @@ -565,10 +609,6 @@ class AppLoader { - return extension.handle === configuration.handle - }) - const extensionInstance = new ExtensionInstance({ configuration, configurationPath, @@ -577,9 +617,12 @@ class AppLoader { if (this.specifications.length === 0) return [] - const extensionPromises = await this.createExtensionInstances(appDirectory, appConfiguration.extension_directories) + const extensionPromises = await this.createExtensionInstances(appDirectory) const configExtensionPromises = await this.createConfigExtensionInstances(appDirectory, appConfiguration) const webhookPromises = this.createWebhookSubscriptionInstances(appDirectory, appConfiguration) @@ -624,16 +667,17 @@ class AppLoader { - return joinPath(appDirectory, extensionPath, '*.extension.toml') - }) - extensionConfigPaths.push(`!${joinPath(appDirectory, '**/node_modules/**')}`) - const configPaths = await glob(extensionConfigPaths) + private async createExtensionInstances(appDirectory: string) { + // Use pre-discovered extension files from Project, filtered by active config + const activeConfig = this.activeConfigFile + const extensionFiles = activeConfig + ? extensionFilesForConfig(this.project, activeConfig) + : this.project.extensionConfigFiles - return configPaths.map(async (configurationPath) => { + return extensionFiles.map(async (extensionFile) => { + const configurationPath = extensionFile.path const directory = dirname(configurationPath) - const obj = await loadConfigurationFileContent(configurationPath) + const obj = extensionFile.content const parseResult = ExtensionsArraySchema.safeParse(obj) if (!parseResult.success) { this.abortOrReport( @@ -648,7 +692,12 @@ class AppLoader { const mergedConfig = {...configuration, ...extensionConfig} @@ -823,7 +872,7 @@ class AppLoader { - const specifications = options.specifications ?? (await loadLocalExtensionsSpecifications()) - const state = await getAppConfigurationState(options.directory, options.userProvidedConfigName) - const result = await loadAppConfigurationFromState(state, specifications, options.remoteFlags ?? []) - await logMetadataFromAppLoadingProcess(result.configurationLoadResultMetadata) - return result -} - -interface AppConfigurationLoaderConstructorArgs { - directory: string - userProvidedConfigName: string | undefined - specifications?: ExtensionSpecification[] - remoteFlags?: Flag[] -} - type LinkedConfigurationSource = // Config file was passed via a flag to a command | 'flag' @@ -885,137 +913,24 @@ type ConfigurationLoaderResult< configurationLoadResultMetadata: ConfigurationLoadResultMetadata } -interface AppConfigurationStateBasics { - appDirectory: string - configurationPath: string - configSource: LinkedConfigurationSource - configurationFileName: AppConfigurationFileName -} - -export type AppConfigurationState = AppConfigurationStateBasics & { - basicConfiguration: BasicAppConfigurationWithoutModules - isLinked: boolean -} - /** - * Get the app configuration state from the file system. + * Get the app configuration context from the file system. * - * This takes a shallow look at the app folder, and indicates if the app has been linked or is still in template form. + * Discovers the project and selects the active config. That's it — no parsing + * or intermediate state construction. Callers that need a parsed config should + * use `loadAppFromContext`. * * @param workingDirectory - Typically either the CWD or came from the `--path` argument. The function will find the root folder of the app. * @param userProvidedConfigName - Some commands allow the manual specification of the config name to use. Otherwise, the function may prompt/use the cached preference. - * @returns Detail about the app configuration state. + * @returns The project and active config selection. */ -export async function getAppConfigurationState( +export async function getAppConfigurationContext( workingDirectory: string, userProvidedConfigName?: string, -): Promise { - // partially loads the app config. doesn't actually check for config validity beyond the absolute minimum - let configName = userProvidedConfigName - - const appDirectory = await getAppDirectory(workingDirectory) - - const cachedCurrentConfigName = getCachedAppInfo(appDirectory)?.configFile - const cachedCurrentConfigPath = cachedCurrentConfigName ? joinPath(appDirectory, cachedCurrentConfigName) : null - if (!configName && cachedCurrentConfigPath && !fileExistsSync(cachedCurrentConfigPath)) { - const warningContent = { - headline: `Couldn't find ${cachedCurrentConfigName}`, - body: [ - "If you have multiple config files, select a new one. If you only have one config file, it's been selected as your default.", - ], - } - configName = await use({directory: appDirectory, warningContent, shouldRenderSuccess: false}) - } - - configName = configName ?? cachedCurrentConfigName - - // Determine source after resolution so it reflects the actual selection path - let configSource: LinkedConfigurationSource - if (userProvidedConfigName) { - configSource = 'flag' - } else if (configName) { - configSource = 'cached' - } else { - configSource = 'default' - } - - const {configurationPath, configurationFileName} = await getConfigurationPath(appDirectory, configName) - const file = await loadConfigurationFileContent(configurationPath) - - const parsedConfig = await parseConfigurationFile(AppSchema, configurationPath) - - const isLinked = parsedConfig.client_id !== '' - - return { - appDirectory, - configurationPath, - basicConfiguration: { - ...file, - ...parsedConfig, - }, - configSource, - configurationFileName, - isLinked, - } -} - -/** - * Given app configuration state, load the app configuration. - * - * This is typically called after getting remote-aware extension specifications. The app configuration is validated acordingly. - */ -async function loadAppConfigurationFromState( - configState: AppConfigurationState, - specifications: TModuleSpec[], - remoteFlags: Flag[], -): Promise> { - const file: JsonMapType = { - ...configState.basicConfiguration, - } as JsonMapType - const appVersionedSchema = getAppVersionedSchema(specifications) - const schemaForConfigurationFile = appVersionedSchema as SchemaForConfig - - const configuration = await parseConfigurationFile( - schemaForConfigurationFile, - configState.configurationPath, - abort, - file, - ) - const allClientIdsByConfigName = await getAllLinkedConfigClientIds(configState.appDirectory, { - [configState.configurationFileName]: configuration.client_id, - }) - - let configurationLoadResultMetadata: ConfigurationLoadResultMetadata = { - usesLinkedConfig: false, - allClientIdsByConfigName, - } - - let gitTracked = false - try { - gitTracked = await checkIfGitTracked(configState.appDirectory, configState.configurationPath) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // leave as false - } - - configurationLoadResultMetadata = { - ...configurationLoadResultMetadata, - usesLinkedConfig: true, - name: configState.configurationFileName, - gitTracked, - source: configState.configSource, - usesCliManagedUrls: configuration.build?.automatically_update_urls_on_dev, - } - - return { - directory: configState.appDirectory, - configPath: configState.configurationPath, - configuration, - configurationLoadResultMetadata, - configSchema: schemaForConfigurationFile, - specifications, - remoteFlags, - } +): Promise<{project: Project; activeConfig: ActiveConfig}> { + const project = await Project.load(workingDirectory) + const activeConfig = await selectActiveConfig(project, userProvidedConfigName) + return {project, activeConfig} } async function checkIfGitTracked(appDirectory: string, configurationPath: string) { @@ -1028,7 +943,7 @@ async function checkIfGitTracked(appDirectory: string, configurationPath: string return isTracked } -export async function getConfigurationPath(appDirectory: string, configName: string | undefined) { +async function getConfigurationPath(appDirectory: string, configName: string | undefined) { const configurationFileName = getAppConfigurationFileName(configName) const configurationPath = joinPath(appDirectory, configurationFileName) @@ -1039,108 +954,31 @@ export async function getConfigurationPath(appDirectory: string, configName: str } } -/** - * Sometimes we want to run app commands from a nested folder (for example within an extension). So we need to - * traverse up the filesystem to find the root app directory. - * - * @param directory - The current working directory, or the `--path` option - */ -export async function getAppDirectory(directory: string) { - if (!(await fileExists(directory))) { - throw new AbortError(outputContent`Couldn't find directory ${outputToken.path(directory)}`) - } - - // In order to find the chosen config for the app, we need to find the directory of the app. - // But we can't know the chosen config because the cache key is the directory itself. So we - // look for all possible `shopify.app.*toml` files and stop at the first directory that contains one. - const appDirectory = await findPathUp( - async (directory) => { - const found = await glob(joinPath(directory, appConfigurationFileNameGlob)) - if (found.length > 0) { - return directory - } - }, - { - cwd: directory, - type: 'directory', - }, - ) - - if (appDirectory) { - return appDirectory - } else { - throw new AbortError( - outputContent`Couldn't find an app toml file at ${outputToken.path(directory)}, is this an app directory?`, - ) - } -} - /** * Looks for all likely linked config files in the app folder, parses, and returns a mapping of name to client ID. * * @param prefetchedConfigs - A mapping of config names to client IDs that have already been fetched from the filesystem. */ -async function getAllLinkedConfigClientIds( - appDirectory: string, +function getAllLinkedConfigClientIds( + appConfigFiles: TomlFile[], prefetchedConfigs: {[key: string]: string | number | undefined}, -): Promise<{[key: string]: string}> { - const candidates = await glob(joinPath(appDirectory, appConfigurationFileNameGlob)) - - const entries: [string, string][] = ( - await Promise.all( - candidates.map(async (candidateFile) => { - const configName = basename(candidateFile) - if (prefetchedConfigs[configName] !== undefined && typeof prefetchedConfigs[configName] === 'string') { - return [configName, prefetchedConfigs[configName]] as [string, string] - } - try { - const configuration = await parseConfigurationFile( - // we only care about the client ID, so no need to parse the entire file - zod.object({client_id: zod.string().optional()}), - candidateFile, - // we're not interested in error reporting at all - noopAbortOrReport, - ) - if (configuration.client_id !== undefined) { - return [configName, configuration.client_id] as [string, string] - } - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // can ignore errors in parsing - } - }), - ) - ).filter((entry) => entry !== undefined) +): {[key: string]: string} { + const entries: [string, string][] = appConfigFiles + .map((tomlFile) => { + const configName = basename(tomlFile.path) + if (prefetchedConfigs[configName] !== undefined && typeof prefetchedConfigs[configName] === 'string') { + return [configName, prefetchedConfigs[configName]] as [string, string] + } + const clientId = tomlFile.content.client_id + if (typeof clientId === 'string' && clientId !== '') { + return [configName, clientId] as [string, string] + } + return undefined + }) + .filter((entry) => entry !== undefined) return Object.fromEntries(entries) } -export async function loadHiddenConfig( - appDirectory: string, - configuration: AppConfiguration, -): Promise { - if (!configuration.client_id || typeof configuration.client_id !== 'string') return {} - - const hiddenConfigPath = await getOrCreateAppConfigHiddenPath(appDirectory) - - try { - const allConfigs: {[key: string]: AppHiddenConfig} = JSON.parse(await readFile(hiddenConfigPath)) - const currentAppConfig = allConfigs[configuration.client_id] - - if (currentAppConfig) return currentAppConfig - - // Migration from legacy format, can be safely removed in version >=3.77 - const oldConfig = allConfigs.dev_store_url - if (oldConfig !== undefined && typeof oldConfig === 'string') { - await patchAppHiddenConfigFile(hiddenConfigPath, configuration.client_id, {dev_store_url: oldConfig}) - return {dev_store_url: oldConfig} - } - return {} - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - return {} - } -} - async function getProjectType(webs: Web[]): Promise<'node' | 'php' | 'ruby' | 'frontend' | undefined> { const backendWebs = webs.filter((web) => isWebType(web, WebType.Backend)) const frontendWebs = webs.filter((web) => isWebType(web, WebType.Frontend)) @@ -1283,42 +1121,11 @@ async function logMetadataFromAppLoadingProcess(loadMetadata: ConfigurationLoadR }) } -const appConfigurationFileNameRegex = /^shopify\.app(\.[-\w]+)?\.toml$/ -const appConfigurationFileNameGlob = 'shopify.app*.toml' -export type AppConfigurationFileName = 'shopify.app.toml' | `shopify.app.${string}.toml` - -/** - * Gets the name of the app configuration file (e.g. `shopify.app.production.toml`) based on a provided config name. - * - * @param configName - Optional config name to base the file name upon - * @returns Either the default app configuration file name (`shopify.app.toml`), the given config name (if it matched the valid format), or `shopify.app..toml` if it was an arbitrary string - */ -export function getAppConfigurationFileName(configName?: string): AppConfigurationFileName { - if (!configName) { - return configurationFileNames.app - } - - if (isValidFormatAppConfigurationFileName(configName)) { - return configName - } else { - return `shopify.app.${slugify(configName)}.toml` - } -} - -/** - * Given a path to an app configuration file, extract the shorthand section from the file name. - * - * This is undefined for `shopify.app.toml` files, or returns e.g. `production` for `shopify.app.production.toml`. - */ -export function getAppConfigurationShorthand(path: string) { - const match = basename(path).match(appConfigurationFileNameRegex) - return match?.[1]?.slice(1) -} - -/** Checks if configName is a valid one (`shopify.app.toml`, or `shopify.app..toml`) */ -export function isValidFormatAppConfigurationFileName(configName: string): configName is AppConfigurationFileName { - if (appConfigurationFileNameRegex.test(configName)) { - return true - } - return false -} +// Re-export config file naming utilities from their leaf module. +// These were moved to break the circular dependency: loader ↔ active-config ↔ use ↔ loader. +export { + getAppConfigurationFileName, + getAppConfigurationShorthand, + isValidFormatAppConfigurationFileName, + type AppConfigurationFileName, +} from './config-file-naming.js' diff --git a/packages/app/src/cli/models/project/active-config.ts b/packages/app/src/cli/models/project/active-config.ts index fa4dcc568c2..dbf8dd86eeb 100644 --- a/packages/app/src/cli/models/project/active-config.ts +++ b/packages/app/src/cli/models/project/active-config.ts @@ -1,13 +1,15 @@ import {Project} from './project.js' import {resolveDotEnv, resolveHiddenConfig} from './config-selection.js' -import {AppHiddenConfig, BasicAppConfigurationWithoutModules} from '../app/app.js' -import {AppConfigurationFileName, AppConfigurationState, getConfigurationPath} from '../app/loader.js' +import {AppHiddenConfig} from '../app/app.js' +import {getAppConfigurationFileName} from '../app/config-file-naming.js' import {getCachedAppInfo} from '../../services/local-storage.js' import use from '../../services/app/config/use.js' import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' import {fileExistsSync} from '@shopify/cli-kit/node/fs' -import {joinPath, basename} from '@shopify/cli-kit/node/path' +import {joinPath} from '@shopify/cli-kit/node/path' +import {AbortError} from '@shopify/cli-kit/node/error' +import {outputContent, outputToken} from '@shopify/cli-kit/node/output' /** @public */ export type ConfigSource = 'flag' | 'cached' | 'default' @@ -81,42 +83,18 @@ export async function selectActiveConfig(project: Project, userProvidedConfigNam source = 'default' } - // Resolve the config file name and verify it exists - const {configurationPath, configurationFileName} = await getConfigurationPath(project.directory, configName) - - // Look up the TomlFile from the project's pre-loaded files + // Resolve the config file name and look it up in the project's pre-loaded files + const configurationFileName = getAppConfigurationFileName(configName) const file = project.appConfigByName(configurationFileName) if (!file) { - // Fallback: the project didn't discover this file (shouldn't happen, but be safe) - const fallbackFile = await TomlFile.read(configurationPath) - return buildActiveConfig(project, fallbackFile, source) + throw new AbortError( + outputContent`Couldn't find ${configurationFileName} in ${outputToken.path(project.directory)}.`, + ) } return buildActiveConfig(project, file, source) } -/** - * Bridge from the new Project/ActiveConfig model to the legacy AppConfigurationState. - * - * This allows callers that still consume AppConfigurationState to work with - * the new selection logic without changes. - * @public - */ -export function toAppConfigurationState( - project: Project, - activeConfig: ActiveConfig, - basicConfiguration: BasicAppConfigurationWithoutModules, -): AppConfigurationState { - return { - appDirectory: project.directory, - configurationPath: activeConfig.file.path, - basicConfiguration, - configSource: activeConfig.source, - configurationFileName: basename(activeConfig.file.path) as AppConfigurationFileName, - isLinked: activeConfig.isLinked, - } -} - async function buildActiveConfig(project: Project, file: TomlFile, source: ConfigSource): Promise { const clientId = typeof file.content.client_id === 'string' ? file.content.client_id : undefined const isLinked = Boolean(clientId) && clientId !== '' diff --git a/packages/app/src/cli/models/project/config-selection.test.ts b/packages/app/src/cli/models/project/config-selection.test.ts index 8d2f392d739..a23ef74a1ab 100644 --- a/packages/app/src/cli/models/project/config-selection.test.ts +++ b/packages/app/src/cli/models/project/config-selection.test.ts @@ -142,6 +142,66 @@ describe('extensionFilesForConfig', () => { }) }) + test('supports glob patterns in extension_directories', async () => { + // extension_directories globs are joined with /*.extension.toml by both + // Project.load() discovery and extensionFilesForConfig() filtering. + // For "plugins/*", the discovery glob is "plugins/*/*.extension.toml", + // matching extensions at plugins//shopify.extension.toml. + await inTemporaryDirectory(async (dir) => { + await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "abc"\nextension_directories = ["plugins/*"]') + + // Should match: plugins/checkout (within plugins/*) + await mkdir(joinPath(dir, 'plugins', 'checkout')) + await writeFile( + joinPath(dir, 'plugins', 'checkout', 'shopify.extension.toml'), + 'type = "function"\nname = "matched"', + ) + + // Should NOT match: other-dir/my-ext (outside plugins/*) + await mkdir(joinPath(dir, 'other-dir', 'my-ext')) + await writeFile( + joinPath(dir, 'other-dir', 'my-ext', 'shopify.extension.toml'), + 'type = "function"\nname = "not-matched"', + ) + + const project = await Project.load(dir) + const activeConfig = project.appConfigFiles[0]! + + const extFiles = extensionFilesForConfig(project, activeConfig) + + expect(extFiles).toHaveLength(1) + expect(extFiles[0]!.content.name).toBe('matched') + }) + }) + + test('supports ** deep glob patterns', async () => { + await inTemporaryDirectory(async (dir) => { + await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "abc"\nextension_directories = ["extensions/**"]') + + // Deeply nested extension should be found + await mkdir(joinPath(dir, 'extensions', 'nested', 'deep', 'my-ext')) + await writeFile( + joinPath(dir, 'extensions', 'nested', 'deep', 'my-ext', 'shopify.extension.toml'), + 'type = "function"\nname = "deep-ext"', + ) + + // Top-level extension should also be found + await mkdir(joinPath(dir, 'extensions', 'top-ext')) + await writeFile( + joinPath(dir, 'extensions', 'top-ext', 'shopify.extension.toml'), + 'type = "function"\nname = "top-ext"', + ) + + const project = await Project.load(dir) + const activeConfig = project.appConfigFiles[0]! + + const extFiles = extensionFilesForConfig(project, activeConfig) + const names = extFiles.map((file) => file.content.name).sort() + + expect(names).toStrictEqual(['deep-ext', 'top-ext']) + }) + }) + test('filters to active config extension_directories only', async () => { await inTemporaryDirectory(async (dir) => { await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"\nextension_directories = ["ext-a/*"]') @@ -187,6 +247,28 @@ describe('webFilesForConfig', () => { }) }) + test('supports glob patterns in web_directories', async () => { + await inTemporaryDirectory(async (dir) => { + await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "abc"\nweb_directories = ["services/*"]') + + // Should match: services/backend (within services/*) + await mkdir(joinPath(dir, 'services', 'backend')) + await writeFile(joinPath(dir, 'services', 'backend', 'shopify.web.toml'), 'name = "backend"\nroles = ["backend"]') + + // Should NOT match: other/web (outside services/*) + await mkdir(joinPath(dir, 'other', 'web')) + await writeFile(joinPath(dir, 'other', 'web', 'shopify.web.toml'), 'name = "not-matched"\nroles = ["backend"]') + + const project = await Project.load(dir) + const activeConfig = project.appConfigFiles[0]! + + const webFiles = webFilesForConfig(project, activeConfig) + + expect(webFiles).toHaveLength(1) + expect(webFiles[0]!.content.name).toBe('backend') + }) + }) + test('filters to active config web_directories', async () => { await inTemporaryDirectory(async (dir) => { await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"\nweb_directories = ["web-a"]') diff --git a/packages/app/src/cli/models/project/config-selection.ts b/packages/app/src/cli/models/project/config-selection.ts index dee3504d589..dddc1415d18 100644 --- a/packages/app/src/cli/models/project/config-selection.ts +++ b/packages/app/src/cli/models/project/config-selection.ts @@ -6,6 +6,7 @@ import {patchAppHiddenConfigFile} from '../../services/app/patch-app-configurati import {getOrCreateAppConfigHiddenPath} from '../../utilities/app/config/hidden-app-config.js' import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' +import {matchGlob} from '@shopify/cli-kit/node/fs' import {relativePath} from '@shopify/cli-kit/node/path' /** @@ -69,35 +70,31 @@ export async function resolveHiddenConfig(project: Project, clientId: string | u * Filter extension config files to those belonging to the active config's * extension_directories. If the active config doesn't specify extension_directories, * uses the default (extensions/*). + * + * Uses real glob matching (via minimatch) to preserve the same semantics as + * Project.load()'s discovery globs, including patterns like "foo/*/bar". * @public */ export function extensionFilesForConfig(project: Project, activeConfig: TomlFile): TomlFile[] { const configDirs = activeConfig.content.extension_directories - if (!Array.isArray(configDirs) || configDirs.length === 0) { - // Default: extensions/* — filter project files by path prefix - return project.extensionConfigFiles.filter((file) => { - const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/') - return relPath.startsWith('extensions/') - }) - } + const dirs = Array.isArray(configDirs) && configDirs.length > 0 ? (configDirs as string[]) : ['extensions/*'] - // Filter to files within the active config's declared directories. - // Glob patterns are reduced to prefixes (e.g., "custom/*" → "custom/"). - // This is a simplification — complex globs like "foo/*/bar" will over-match. - // In practice, only simple directory patterns are used in app configs. - const dirPrefixes = (configDirs as string[]).map((dir) => { - return dir.replace(/\*.*$/, '').replace(/\/?$/, '/') - }) + // Replicate the same glob patterns used by discoverExtensionFiles in project.ts: + // each directory pattern becomes "/*.extension.toml" + const globPatterns = dirs.map((dir) => `${dir}/*.extension.toml`) return project.extensionConfigFiles.filter((file) => { const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/') - return dirPrefixes.some((prefix) => relPath.startsWith(prefix)) + return globPatterns.some((pattern) => matchGlob(relPath, pattern)) }) } /** * Filter web config files to those belonging to the active config's * web_directories. If not specified, returns all web files. + * + * Uses real glob matching (via minimatch) to preserve the same semantics as + * Project.load()'s discovery globs. * @public */ export function webFilesForConfig(project: Project, activeConfig: TomlFile): TomlFile[] { @@ -106,10 +103,12 @@ export function webFilesForConfig(project: Project, activeConfig: TomlFile): Tom return project.webConfigFiles } - const dirPrefixes = (configDirs as string[]).map((dir) => dir.replace(/\*.*$/, '').replace(/\/?$/, '/')) + // Replicate the same glob patterns used by discoverWebFiles in project.ts: + // each directory pattern becomes "/shopify.web.toml" + const globPatterns = (configDirs as string[]).map((dir) => `${dir}/shopify.web.toml`) return project.webConfigFiles.filter((file) => { const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/') - return dirPrefixes.some((prefix) => relPath.startsWith(prefix)) + return globPatterns.some((pattern) => matchGlob(relPath, pattern)) }) } diff --git a/packages/app/src/cli/models/project/project-integration.test.ts b/packages/app/src/cli/models/project/project-integration.test.ts index f0805cbbc1c..d1f147b4f7b 100644 --- a/packages/app/src/cli/models/project/project-integration.test.ts +++ b/packages/app/src/cli/models/project/project-integration.test.ts @@ -1,10 +1,14 @@ import {Project} from './project.js' import {resolveDotEnv, resolveHiddenConfig, extensionFilesForConfig, webFilesForConfig} from './config-selection.js' -import {loadApp} from '../app/loader.js' +import {loadApp, reloadApp} from '../app/loader.js' +import {AppLinkedInterface} from '../app/app.js' import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js' +import {handleWatcherEvents} from '../../services/dev/app-events/app-event-watcher-handler.js' +import {EventType} from '../../services/dev/app-events/app-event-watcher.js' import {describe, expect, test} from 'vitest' import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' +import {AbortController} from '@shopify/cli-kit/node/abort' /** * Integration tests verifying that Project + config-selection produce @@ -43,6 +47,7 @@ api_version = "2024-01" type = "product_discounts" name = "My Discount" handle = "my-discount" +api_version = "2024-01" [build] command = "cargo build" @@ -76,11 +81,13 @@ dev = "npm run dev" await writeFile(joinPath(dir, 'package.json'), JSON.stringify({name: 'test-app', dependencies: {}})) } +// Load specifications once — this is expensive (loads all extension specs from disk) +const specifications = await loadLocalExtensionsSpecifications() + describe('Project integration', () => { test('Project discovers the same directory as the old loader', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) - const specifications = await loadLocalExtensionsSpecifications() const project = await Project.load(dir) const app = await loadApp({ @@ -97,7 +104,6 @@ describe('Project integration', () => { test('Project discovers the same extension files as the old loader', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) - const specifications = await loadLocalExtensionsSpecifications() const project = await Project.load(dir) const app = await loadApp({ @@ -125,7 +131,6 @@ describe('Project integration', () => { test('Project discovers the same web files as the old loader', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) - const specifications = await loadLocalExtensionsSpecifications() const project = await Project.load(dir) const app = await loadApp({ @@ -149,7 +154,6 @@ describe('Project integration', () => { test('resolveDotEnv matches the old loader dotenv', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) - const specifications = await loadLocalExtensionsSpecifications() const project = await Project.load(dir) const app = await loadApp({ @@ -171,7 +175,6 @@ describe('Project integration', () => { test('resolveHiddenConfig matches the old loader hidden config', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) - const specifications = await loadLocalExtensionsSpecifications() const project = await Project.load(dir) const app = await loadApp({ @@ -190,7 +193,6 @@ describe('Project integration', () => { test('Project metadata matches the old loader metadata', async () => { await inTemporaryDirectory(async (dir) => { await setupRealApp(dir) - const specifications = await loadLocalExtensionsSpecifications() const project = await Project.load(dir) const app = await loadApp({ @@ -268,4 +270,129 @@ extension_directories = ["staging-ext/*"] expect(stagingDotenv?.variables.STAGING_VAR).toBe('staging-value') }) }) + + test('Project.load re-scans filesystem and finds extensions added after initial load', async () => { + await inTemporaryDirectory(async (dir) => { + await setupRealApp(dir) + + // Initial load — should find 1 extension file + const project1 = await Project.load(dir) + expect(project1.extensionConfigFiles).toHaveLength(1) + + // Add a new extension to disk (simulates `shopify generate extension` mid-dev) + await mkdir(joinPath(dir, 'extensions', 'another-function')) + await writeFile( + joinPath(dir, 'extensions', 'another-function', 'shopify.extension.toml'), + ` +type = "product_discounts" +name = "Another Discount" +handle = "another-discount" +api_version = "2024-01" + +[build] +command = "cargo build" + `.trim(), + ) + + // Fresh Project.load should find the new file + const project2 = await Project.load(dir) + expect(project2.extensionConfigFiles).toHaveLength(2) + + // extensionFilesForConfig should also include it + const activeConfig = (await import('./active-config.js')).selectActiveConfig + const config = await activeConfig(project2) + const extFiles = extensionFilesForConfig(project2, config.file) + expect(extFiles).toHaveLength(2) + }) + }) + + test('reloadApp finds extensions added after initial load', async () => { + await inTemporaryDirectory(async (dir) => { + await setupRealApp(dir) + // Initial load with report mode (matches dev behavior) + const app = await loadApp({ + directory: dir, + userProvidedConfigName: undefined, + specifications, + mode: 'report', + }) + const initialRealExtensions = app.realExtensions + const initialCount = initialRealExtensions.length + + // Add a new extension to disk (simulates `shopify generate extension` mid-dev) + await mkdir(joinPath(dir, 'extensions', 'another-function')) + await writeFile( + joinPath(dir, 'extensions', 'another-function', 'shopify.extension.toml'), + ` +type = "product_discounts" +name = "Another Discount" +handle = "another-discount" +api_version = "2024-01" + +[build] +command = "cargo build" + `.trim(), + ) + + // Reload should find the new extension + const reloadedApp = await reloadApp(app as AppLinkedInterface) + const reloadedRealExtensions = reloadedApp.realExtensions + expect(reloadedRealExtensions.length).toBe(initialCount + 1) + }) + }) + + test('handleWatcherEvents produces Created event for extension added mid-dev', async () => { + await inTemporaryDirectory(async (dir) => { + await setupRealApp(dir) + // Initial load + const app = await loadApp({ + directory: dir, + userProvidedConfigName: undefined, + specifications, + mode: 'report', + }) + + // Add a new extension to disk + const newExtDir = joinPath(dir, 'extensions', 'another-function') + await mkdir(newExtDir) + await writeFile( + joinPath(newExtDir, 'shopify.extension.toml'), + ` +type = "product_discounts" +name = "Another Discount" +handle = "another-discount" +api_version = "2024-01" + +[build] +command = "cargo build" + `.trim(), + ) + + // Simulate the file watcher event that would fire + const appEvent = await handleWatcherEvents( + [ + { + type: 'extension_folder_created', + path: newExtDir, + extensionPath: newExtDir, + startTime: [0, 0] as [number, number], + }, + ], + app as AppLinkedInterface, + {stdout: process.stdout, stderr: process.stderr, signal: new AbortController().signal}, + ) + + // The event should indicate the app was reloaded and the new extension was created + expect(appEvent).toBeDefined() + expect(appEvent!.appWasReloaded).toBe(true) + expect(appEvent!.app.realExtensions.length).toBeGreaterThan(app.realExtensions.length) + + const createdEvents = appEvent!.extensionEvents.filter((ev) => ev.type === EventType.Created) + expect(createdEvents.length).toBeGreaterThanOrEqual(1) + + // The reloaded app should include the new extension + const handles = appEvent!.app.realExtensions.map((ext) => ext.configuration.handle ?? ext.configuration.name) + expect(handles).toContain('another-discount') + }) + }) }) diff --git a/packages/app/src/cli/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index c1bd6d673aa..40b41159915 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -74,6 +74,8 @@ client_id="test-api-key"` developerPlatformClient: expect.any(Object), specifications: [], organization: mockOrganization, + project: expect.any(Object), + activeConfig: expect.any(Object), }) expect(link).not.toHaveBeenCalled() }) @@ -152,22 +154,12 @@ client_id="test-api-key"` vi.mocked(link).mockResolvedValue({ remoteApp: mockRemoteApp, - state: { - appDirectory: tmp, - configurationPath: `${tmp}/shopify.app.toml`, - configSource: 'cached', - configurationFileName: 'shopify.app.toml', - isLinked: true, - basicConfiguration: { - client_id: 'test-api-key', - }, - }, + configFileName: 'shopify.app.toml', configuration: { client_id: 'test-api-key', name: 'test-app', - application_url: 'https://test-app.com', - embedded: false, - }, + path: normalizePath(joinPath(tmp, 'shopify.app.toml')), + } as any, }) // When @@ -218,7 +210,7 @@ client_id="test-api-key"` name = "test-app" client_id="test-api-key"` await writeAppConfig(tmp, content) - const loadSpy = vi.spyOn(loader, 'loadAppUsingConfigurationState') + const loadSpy = vi.spyOn(loader, 'loadAppFromContext') // When await linkedAppContext({ @@ -231,7 +223,7 @@ client_id="test-api-key"` // Then expect(vi.mocked(addUidToTomlsIfNecessary)).not.toHaveBeenCalled() - expect(loadSpy).toHaveBeenCalledWith(expect.any(Object), expect.objectContaining({mode: 'report'})) + expect(loadSpy).toHaveBeenCalledWith(expect.objectContaining({mode: 'report'})) loadSpy.mockRestore() }) }) @@ -243,7 +235,7 @@ client_id="test-api-key"` name = "test-app" client_id="test-api-key"` await writeAppConfig(tmp, content) - const loadSpy = vi.spyOn(loader, 'loadAppUsingConfigurationState') + const loadSpy = vi.spyOn(loader, 'loadAppFromContext') // When await linkedAppContext({ @@ -255,7 +247,7 @@ client_id="test-api-key"` // Then expect(vi.mocked(addUidToTomlsIfNecessary)).toHaveBeenCalled() - expect(loadSpy).toHaveBeenCalledWith(expect.any(Object), expect.objectContaining({mode: 'strict'})) + expect(loadSpy).toHaveBeenCalledWith(expect.objectContaining({mode: 'strict'})) loadSpy.mockRestore() }) }) diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index b33a067c34b..69051e36b59 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -7,11 +7,14 @@ import {addUidToTomlsIfNecessary} from './app/add-uid-to-extension-toml.js' import {loadLocalExtensionsSpecifications} from '../models/extensions/load-specifications.js' import {Organization, OrganizationApp, OrganizationSource} from '../models/organization.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' -import {getAppConfigurationState, loadAppUsingConfigurationState, loadApp} from '../models/app/loader.js' +import {getAppConfigurationContext, loadApp, loadAppFromContext} from '../models/app/loader.js' import {RemoteAwareExtensionSpecification} from '../models/extensions/specification.js' import {AppLinkedInterface, AppInterface} from '../models/app/app.js' +import {Project} from '../models/project/project.js' import metadata from '../metadata.js' import {tryParseInt} from '@shopify/cli-kit/common/string' +import {basename} from '@shopify/cli-kit/node/path' +import type {ActiveConfig} from '../models/project/active-config.js' export interface LoadedAppContextOutput { app: AppLinkedInterface @@ -19,6 +22,8 @@ export interface LoadedAppContextOutput { developerPlatformClient: DeveloperPlatformClient organization: Organization specifications: RemoteAwareExtensionSpecification[] + project: Project + activeConfig: ActiveConfig } /** @@ -68,26 +73,28 @@ export async function linkedAppContext({ userProvidedConfigName, unsafeReportMode = false, }: LoadedAppContextOptions): Promise { - // Get current app configuration state - let configState = await getAppConfigurationState(directory, userProvidedConfigName) + let {project, activeConfig} = await getAppConfigurationContext(directory, userProvidedConfigName) let remoteApp: OrganizationApp | undefined - if (!configState.isLinked || forceRelink) { - const configName = forceRelink ? undefined : configState.configurationFileName + if (!activeConfig.isLinked || forceRelink) { + const configName = forceRelink ? undefined : basename(activeConfig.file.path) const result = await link({directory, apiKey: clientId, configName}) remoteApp = result.remoteApp - configState = result.state + // Re-load project and re-select active config since link may have written new config + const reloaded = await getAppConfigurationContext(directory, result.configFileName) + project = reloaded.project + activeConfig = reloaded.activeConfig } - // If the clientId is provided, update the configuration state with the new clientId - if (clientId && clientId !== configState.basicConfiguration.client_id) { - configState.basicConfiguration.client_id = clientId - } + // Determine the effective client ID + // Note: activeConfig.file.content is JsonMapType — client_id is untyped. + // The cast is safe here because activeConfig comes from a linked app (has client_id). + const configClientId = activeConfig.file.content.client_id as string + const effectiveClientId = clientId ?? configClientId // Fetch the remote app, using a different clientID if provided via flag. if (!remoteApp) { - const apiKey = configState.basicConfiguration.client_id - remoteApp = await appFromIdentifiers({apiKey}) + remoteApp = await appFromIdentifiers({apiKey: effectiveClientId}) } const developerPlatformClient = remoteApp.developerPlatformClient @@ -96,11 +103,14 @@ export async function linkedAppContext({ // Fetch the remote app's specifications const specifications = await fetchSpecifications({developerPlatformClient, app: remoteApp}) - // Load the local app using the configuration state and the remote app's specifications - const localApp = await loadAppUsingConfigurationState(configState, { + // Load the local app using the pre-resolved context and the remote app's specifications + const localApp = await loadAppFromContext({ + project, + activeConfig, specifications, remoteFlags: remoteApp.flags, mode: unsafeReportMode ? 'report' : 'strict', + clientIdOverride: clientId && clientId !== configClientId ? clientId : undefined, }) // If the remoteApp is the same as the linked one, update the cached info. @@ -120,7 +130,7 @@ export async function linkedAppContext({ await addUidToTomlsIfNecessary(localApp.allExtensions, developerPlatformClient) } - return {app: localApp, remoteApp, developerPlatformClient, specifications, organization} + return {project, activeConfig, app: localApp, remoteApp, developerPlatformClient, specifications, organization} } async function logMetadata(app: {apiKey: string}, organization: Organization, resetUsed: boolean) { diff --git a/packages/app/src/cli/services/app/config/link.test.ts b/packages/app/src/cli/services/app/config/link.test.ts index b7fc5a99969..83a2a57d21d 100644 --- a/packages/app/src/cli/services/app/config/link.test.ts +++ b/packages/app/src/cli/services/app/config/link.test.ts @@ -28,7 +28,6 @@ vi.mock('../../../models/app/loader.js', async () => { return { ...loader, loadApp: vi.fn(), - loadAppConfiguration: vi.fn(), loadOpaqueApp: vi.fn(), } }) @@ -82,7 +81,7 @@ describe('link', () => { vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) // When - const {configuration, state, remoteApp} = await link(options) + const {configuration, configFileName, remoteApp} = await link(options) // Then expect(selectConfigName).not.toHaveBeenCalled() @@ -107,14 +106,7 @@ describe('link', () => { }, }) - expect(state).toEqual({ - basicConfiguration: configuration, - appDirectory: options.directory, - configurationPath: expect.stringMatching(/\/shopify.app.default-value.toml$/), - configSource: 'flag', - configurationFileName: 'shopify.app.default-value.toml', - isLinked: true, - }) + expect(configFileName).toBe('shopify.app.default-value.toml') expect(remoteApp).toEqual(mockRemoteApp({developerPlatformClient})) }) @@ -224,7 +216,7 @@ embedded = false }) // When - const {configuration, state} = await link(options) + const {configuration, configFileName} = await link(options) // Then const content = await readFile(joinPath(tmp, 'shopify.app.toml')) @@ -293,14 +285,7 @@ embedded = false }, }) expect(content).toEqual(expectedContent) - expect(state).toEqual({ - basicConfiguration: configuration, - appDirectory: options.directory, - configurationPath: expect.stringMatching(/\/shopify.app.toml$/), - configSource: 'cached', - configurationFileName: 'shopify.app.toml', - isLinked: true, - }) + expect(configFileName).toBe('shopify.app.toml') }) }) diff --git a/packages/app/src/cli/services/app/config/link.ts b/packages/app/src/cli/services/app/config/link.ts index 9060c66c6c9..7d57920934a 100644 --- a/packages/app/src/cli/services/app/config/link.ts +++ b/packages/app/src/cli/services/app/config/link.ts @@ -10,7 +10,6 @@ import {OrganizationApp} from '../../../models/organization.js' import {selectConfigName} from '../../../prompts/config.js' import { AppConfigurationFileName, - AppConfigurationState, getAppConfigurationFileName, loadApp, loadOpaqueApp, @@ -48,9 +47,9 @@ export interface LinkOptions { } interface LinkOutput { - configuration: CurrentAppConfiguration remoteApp: OrganizationApp - state: AppConfigurationState + configFileName: AppConfigurationFileName + configuration: CurrentAppConfiguration } /** * Link a local app configuration file to a remote app on the Shopify platform. @@ -95,16 +94,7 @@ export default async function link(options: LinkOptions, shouldRenderSuccess = t renderSuccessMessage(configFileName, mergedAppConfiguration.name, localAppOptions.packageManager) } - const state: AppConfigurationState = { - basicConfiguration: mergedAppConfiguration, - appDirectory, - configurationPath: joinPath(appDirectory, configFileName), - configSource: options.configName ? 'flag' : 'cached', - configurationFileName: configFileName, - isLinked: mergedAppConfiguration.client_id !== '', - } - - return {configuration: mergedAppConfiguration, remoteApp, state} + return {remoteApp, configFileName, configuration: mergedAppConfiguration} } /** diff --git a/packages/app/src/cli/services/app/config/use.test.ts b/packages/app/src/cli/services/app/config/use.test.ts index 6abc700a42e..0d4a8cede3f 100644 --- a/packages/app/src/cli/services/app/config/use.test.ts +++ b/packages/app/src/cli/services/app/config/use.test.ts @@ -1,11 +1,6 @@ import use, {UseOptions} from './use.js' -import { - buildVersionedAppSchema, - testApp, - testAppWithConfig, - testDeveloperPlatformClient, -} from '../../../models/app/app.test-data.js' -import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models/app/loader.js' +import {testApp, testAppWithConfig, testDeveloperPlatformClient} from '../../../models/app/app.test-data.js' +import {getAppConfigurationFileName, getAppConfigurationContext} from '../../../models/app/loader.js' import {clearCurrentConfigFile, setCachedAppInfo} from '../../local-storage.js' import {selectConfigFile} from '../../../prompts/config.js' import {describe, expect, test, vi} from 'vitest' @@ -20,6 +15,21 @@ vi.mock('../../../models/app/loader.js') vi.mock('@shopify/cli-kit/node/ui') vi.mock('../../context.js') +function mockContext(directory: string, configuration: Record) { + vi.mocked(getAppConfigurationContext).mockResolvedValue({ + project: {} as any, + activeConfig: { + file: { + path: joinPath(directory, 'shopify.app.toml'), + content: configuration, + }, + source: 'flag', + isLinked: Boolean(configuration.client_id), + hiddenConfig: {}, + } as any, + }) +} + describe('use', () => { test('clears currentConfiguration when reset is true', async () => { await inTemporaryDirectory(async (tmp) => { @@ -38,7 +48,7 @@ describe('use', () => { // Then expect(clearCurrentConfigFile).toHaveBeenCalledWith(tmp) expect(setCachedAppInfo).not.toHaveBeenCalled() - expect(loadAppConfiguration).not.toHaveBeenCalled() + expect(getAppConfigurationContext).not.toHaveBeenCalled() expect(renderSuccess).toHaveBeenCalledWith({ headline: 'Cleared current configuration.', body: [ @@ -77,18 +87,9 @@ describe('use', () => { } vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.no-id.toml') - const {schema: configSchema} = await buildVersionedAppSchema() const appWithoutClientID = testApp() - // Create a configuration without client_id to test the error case const {client_id: clientId, ...configWithoutClientId} = appWithoutClientID.configuration - vi.mocked(loadAppConfiguration).mockResolvedValue({ - directory: tmp, - configPath: joinPath(tmp, 'shopify.app.no-id.toml'), - configuration: configWithoutClientId as any, - configSchema, - specifications: [], - remoteFlags: [], - }) + mockContext(tmp, configWithoutClientId) // When const result = use(options) @@ -120,7 +121,7 @@ describe('use', () => { "message": "Expected array, received string" } ]'`) - vi.mocked(loadAppConfiguration).mockRejectedValue(error) + vi.mocked(getAppConfigurationContext).mockRejectedValue(error) // When const result = use(options) @@ -130,6 +131,31 @@ describe('use', () => { }) }) + test('accepts syntactically valid config with client_id even if schema-incomplete', async () => { + // The use() command intentionally does NOT run full schema validation. + // It only requires syntactically valid TOML + a client_id. + // Full validation is deferred to the next command that loads the app. + await inTemporaryDirectory(async (tmp) => { + // Given — config has client_id but is missing required fields like application_url + createConfigFile(tmp, 'shopify.app.minimal.toml') + const options: UseOptions = { + directory: tmp, + configName: 'minimal', + } + vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.minimal.toml') + mockContext(tmp, {client_id: 'some-id'}) + + // When + await use(options) + + // Then — config is accepted and cached + expect(setCachedAppInfo).toHaveBeenCalledWith({ + directory: tmp, + configFile: 'shopify.app.minimal.toml', + }) + }) + }) + test('saves currentConfiguration config name to localstorage', async () => { await inTemporaryDirectory(async (tmp) => { // Given @@ -149,15 +175,7 @@ describe('use', () => { application_url: 'https://example.com', }, }) - const {schema: configSchema} = await buildVersionedAppSchema() - vi.mocked(loadAppConfiguration).mockResolvedValue({ - directory: tmp, - configPath: joinPath(tmp, 'shopify.app.staging.toml'), - configuration: app.configuration, - configSchema, - specifications: [], - remoteFlags: [], - }) + mockContext(tmp, app.configuration) // When await use(options) @@ -191,15 +209,7 @@ describe('use', () => { webhooks: {api_version: '2023-04'}, }, }) - const {schema: configSchema} = await buildVersionedAppSchema() - vi.mocked(loadAppConfiguration).mockResolvedValue({ - directory: tmp, - configPath: joinPath(tmp, 'shopify.app.local.toml'), - configuration: app.configuration, - configSchema, - specifications: [], - remoteFlags: [], - }) + mockContext(tmp, app.configuration) // When await use(options) @@ -235,15 +245,7 @@ describe('use', () => { await inTemporaryDirectory(async (directory) => { // Given const {configuration} = testApp({}) - const {schema: configSchema} = await buildVersionedAppSchema() - vi.mocked(loadAppConfiguration).mockResolvedValue({ - directory, - configPath: joinPath(directory, 'shopify.app.something.toml'), - configuration, - configSchema, - specifications: [], - remoteFlags: [], - }) + mockContext(directory, configuration) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.something.toml') createConfigFile(directory, 'shopify.app.something.toml') const options = { @@ -266,15 +268,7 @@ describe('use', () => { await inTemporaryDirectory(async (directory) => { // Given const {configuration} = testApp({}) - const {schema: configSchema} = await buildVersionedAppSchema() - vi.mocked(loadAppConfiguration).mockResolvedValue({ - directory, - configPath: joinPath(directory, 'shopify.app.something.toml'), - configuration, - configSchema, - specifications: [], - remoteFlags: [], - }) + mockContext(directory, configuration) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.something.toml') createConfigFile(directory, 'shopify.app.something.toml') const options = { diff --git a/packages/app/src/cli/services/app/config/use.ts b/packages/app/src/cli/services/app/config/use.ts index fa0a2ffee55..41747049098 100644 --- a/packages/app/src/cli/services/app/config/use.ts +++ b/packages/app/src/cli/services/app/config/use.ts @@ -1,7 +1,6 @@ -import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models/app/loader.js' +import {getAppConfigurationFileName, getAppConfigurationContext} from '../../../models/app/loader.js' import {clearCurrentConfigFile, setCachedAppInfo} from '../../local-storage.js' import {selectConfigFile} from '../../../prompts/config.js' -import {AppConfiguration, CurrentAppConfiguration} from '../../../models/app/app.js' import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' import {AbortError} from '@shopify/cli-kit/node/error' import {fileExists} from '@shopify/cli-kit/node/fs' @@ -47,11 +46,8 @@ export default async function use({ const configFileName = (await getConfigFileName(directory, configName)).valueOrAbort() - const {configuration} = await loadAppConfiguration({ - userProvidedConfigName: configFileName, - directory, - }) - setCurrentConfigPreference(configuration, {configFileName, directory}) + const {activeConfig} = await getAppConfigurationContext(directory, configFileName) + setCurrentConfigPreference(activeConfig.file.content, {configFileName, directory}) if (shouldRenderSuccess) { renderSuccess({ @@ -63,18 +59,18 @@ export default async function use({ } /** - * Sets the prefered app configuration file to use from now on. + * Sets the preferred app configuration file to use from now on. * - * @param configuration - The configuration taken from this file. Used to ensure we're not remembering a malformed or incomplete configuration. - * @returns - Nothing, but does confirm that the configuration is an up to date one (and not fresh from a template). + * Only reads `client_id` from the configuration — full schema validation + * is deferred to the next command that actually loads the app. */ export function setCurrentConfigPreference( - configuration: AppConfiguration, + configuration: {client_id?: string}, options: { configFileName: string directory: string }, -): asserts configuration is CurrentAppConfiguration { +): void { const {configFileName, directory} = options if (configuration.client_id) { setCachedAppInfo({ diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 5e3a9c9ae06..340d205a472 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -18,14 +18,13 @@ import { import {getAppIdentifiers} from '../models/app/identifiers.js' import {selectOrganizationPrompt} from '../prompts/dev.js' import { - DEFAULT_CONFIG, testDeveloperPlatformClient, testAppWithConfig, testOrganizationApp, testThemeExtensions, } from '../models/app/app.test-data.js' import metadata from '../metadata.js' -import {AppConfigurationState, getAppConfigurationFileName, isWebType, loadApp} from '../models/app/loader.js' +import {getAppConfigurationFileName, isWebType, loadApp} from '../models/app/loader.js' import {AppLinkedInterface} from '../models/app/app.js' import * as loadSpecifications from '../models/extensions/load-specifications.js' import { @@ -77,18 +76,6 @@ const STORE1: OrganizationStore = { provisionable: true, } -const state: AppConfigurationState = { - basicConfiguration: { - ...DEFAULT_CONFIG, - client_id: APP2.apiKey, - }, - appDirectory: 'tmp', - configurationPath: 'shopify.app.toml', - configSource: 'flag', - configurationFileName: 'shopify.app.toml', - isLinked: true, -} - const deployOptions = (app: AppLinkedInterface, reset = false, force = false): DeployOptions => { return { app, @@ -172,7 +159,7 @@ beforeEach(async () => { vi.mocked(link).mockResolvedValue({ configuration: testAppWithConfig({config: {client_id: APP2.apiKey}}).configuration, remoteApp: APP2, - state, + configFileName: 'shopify.app.toml', }) // this is needed because using importActual to mock the ui module diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts index 7042f12ddf7..ca479009e0a 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts @@ -23,12 +23,14 @@ vi.mock('@shopify/cli-kit/node/import-extractor', () => ({ extractJSImports: vi.fn(() => []), })) -// Mock fs module for fileExistsSync +// Mock fs module for fileExistsSync, mkdir, and writeFile vi.mock('@shopify/cli-kit/node/fs', async () => { const actual = await vi.importActual('@shopify/cli-kit/node/fs') return { ...actual, fileExistsSync: vi.fn(), + mkdir: vi.fn(), + writeFile: vi.fn(), } }) @@ -719,6 +721,83 @@ describe('file-watcher events', () => { }) }) + test('creates extension directories if they do not exist before starting watcher', async () => { + const realFs = await vi.importActual('@shopify/cli-kit/node/fs') + + await inTemporaryDirectory(async (dir) => { + const extDir = joinPath(dir, 'extensions') + const configPath = joinPath(dir, 'shopify.app.toml') + await realFs.writeFile(configPath, '') + + const app = testAppLinked({ + allExtensions: [], + directory: dir, + configPath, + configuration: { + client_id: 'test-client-id', + name: 'my-app', + application_url: 'https://example.com', + embedded: true, + access_scopes: {scopes: ''}, + extension_directories: ['extensions'], + }, + }) + + // Use real mkdir for this test + vi.mocked(mkdir).mockImplementation((path: string) => realFs.mkdir(path)) + + const mockWatcher = { + on: vi.fn().mockReturnThis(), + close: vi.fn().mockResolvedValue(undefined), + } + vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any) + + const fileWatcher = new FileWatcher(app, outputOptions) + await fileWatcher.start() + + expect(realFs.fileExistsSync(extDir)).toBe(true) + }) + }) + + test('strips glob suffixes when creating extension directories', async () => { + const realFs = await vi.importActual('@shopify/cli-kit/node/fs') + + await inTemporaryDirectory(async (dir) => { + const extDir = joinPath(dir, 'extensions') + const configPath = joinPath(dir, 'shopify.app.toml') + await realFs.writeFile(configPath, '') + + const app = testAppLinked({ + allExtensions: [], + directory: dir, + configPath, + configuration: { + client_id: 'test-client-id', + name: 'my-app', + application_url: 'https://example.com', + embedded: true, + access_scopes: {scopes: ''}, + extension_directories: ['extensions/**'], + }, + }) + + vi.mocked(mkdir).mockImplementation((path: string) => realFs.mkdir(path)) + + const mockWatcher = { + on: vi.fn().mockReturnThis(), + close: vi.fn().mockResolvedValue(undefined), + } + vi.spyOn(chokidar, 'watch').mockReturnValue(mockWatcher as any) + + const fileWatcher = new FileWatcher(app, outputOptions) + await fileWatcher.start() + + // Should create extensions/, not extensions/** + expect(realFs.fileExistsSync(extDir)).toBe(true) + expect(realFs.fileExistsSync(joinPath(extDir, '**'))).toBe(false) + }) + }) + describe('refreshWatchedFiles', () => { test('closes and recreates the watcher with updated paths', async () => { // Given diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.ts index 8d614082cdf..fae4b59f190 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.ts @@ -6,7 +6,7 @@ import {FSWatcher} from 'chokidar' import {outputDebug} from '@shopify/cli-kit/node/output' import {AbortSignal} from '@shopify/cli-kit/node/abort' import {startHRTime, StartTime} from '@shopify/cli-kit/node/hrtime' -import {fileExistsSync, matchGlob, readFileSync} from '@shopify/cli-kit/node/fs' +import {fileExistsSync, matchGlob, mkdir, readFileSync} from '@shopify/cli-kit/node/fs' import {debounce} from '@shopify/cli-kit/common/function' import ignore from 'ignore' import {Writable} from 'stream' @@ -87,6 +87,23 @@ export class FileWatcher { async start(): Promise { const extensionDirectories = [...(this.app.configuration.extension_directories ?? ['extensions'])] const fullExtensionDirectories = extensionDirectories.map((directory) => joinPath(this.app.directory, directory)) + + // Ensure extension directories exist so chokidar can watch them. + // Chokidar v3 silently ignores non-existent directories. + // Strip glob suffixes (e.g. extensions/** → extensions) since mkdir needs real paths. + // Errors are non-fatal — if mkdir fails (e.g. permissions), chokidar will still + // try to watch the path and handle it gracefully. + await Promise.all( + fullExtensionDirectories.map(async (dir) => { + try { + await mkdir(dir.replace(/\/\*+$/, '')) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // Non-fatal: directory may be unwritable (e.g. test fixtures) + } + }), + ) + const watchPaths = [this.app.configPath, ...fullExtensionDirectories] // Get all watched files from extensions diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts index c5af5c50506..96af8edb562 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-process.test.ts @@ -13,7 +13,7 @@ import { testUIExtension, testWebhookExtensions, } from '../../../../models/app/app.test-data.js' -import {getUploadURL} from '../../../bundle.js' +import {getUploadURL, writeManifestToBundle} from '../../../bundle.js' import {formData} from '@shopify/cli-kit/node/http' import {describe, expect, test, vi, beforeEach, afterEach} from 'vitest' import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort' @@ -455,6 +455,86 @@ describe('pushUpdatesForDevSession', () => { }) }) + test('writes full manifest to bundle on session update, not just create', async () => { + // Given + vi.mocked(readdir).mockResolvedValue([]) + vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') + + const updatedExtension = await testUIExtension({handle: 'updated-ext', uid: 'updated-uid'}) + updatedExtension.deployConfig = vi.fn().mockResolvedValue({}) + const existingExtension = await testUIExtension({handle: 'existing-ext', uid: 'existing-uid'}) + existingExtension.deployConfig = vi.fn().mockResolvedValue({}) + const appWithMultipleExtensions = testAppLinked({ + allExtensions: [updatedExtension, existingExtension], + }) + + // When + await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) + await appWatcher.start({stdout, stderr, signal: abortController.signal}) + await flushPromises() + + // Capture manifests at call time (the object is mutated after writeManifestToBundle) + const capturedManifests: any[] = [] + vi.mocked(writeManifestToBundle).mockImplementation(async (manifest: any) => { + capturedManifests.push(structuredClone(manifest)) + }) + + // Emit UPDATE event with only one extension changed + appWatcher.emit('all', { + app: appWithMultipleExtensions, + extensionEvents: [{type: 'updated', extension: updatedExtension}], + }) + await flushPromises() + + // Then - writeManifestToBundle should have been called with ALL modules, not just the updated one + expect(capturedManifests).toHaveLength(1) + const moduleUids = capturedManifests[0].modules.map((mod: any) => mod.uid) + expect(moduleUids).toContain(updatedExtension.uid) + expect(moduleUids).toContain(existingExtension.uid) + expect(moduleUids.length).toBeGreaterThanOrEqual(2) + }) + + test('writes full manifest including new extension on created event', async () => { + // Given + vi.mocked(readdir).mockResolvedValue([]) + vi.mocked(getUploadURL).mockResolvedValue('https://gcs.url') + + const existingExtension = await testUIExtension({handle: 'existing-ext', uid: 'existing-uid'}) + existingExtension.deployConfig = vi.fn().mockResolvedValue({}) + const newExtension = await testUIExtension({handle: 'new-ext', uid: 'new-uid'}) + newExtension.deployConfig = vi.fn().mockResolvedValue({}) + + // The app after reload includes both the existing and newly created extension + const appAfterReload = testAppLinked({ + allExtensions: [existingExtension, newExtension], + }) + + // When + await pushUpdatesForDevSession({stderr, stdout, abortSignal: abortController.signal}, options) + await appWatcher.start({stdout, stderr, signal: abortController.signal}) + await flushPromises() + + // Capture manifests at call time (the object is mutated after writeManifestToBundle) + const capturedManifests: any[] = [] + vi.mocked(writeManifestToBundle).mockImplementation(async (manifest: any) => { + capturedManifests.push(structuredClone(manifest)) + }) + + // Emit event with a new extension created mid-dev (simulates generate extension) + appWatcher.emit('all', { + app: appAfterReload, + extensionEvents: [{type: 'created', extension: newExtension}], + }) + await flushPromises() + + // Then - writeManifestToBundle should include ALL modules (existing + new) + expect(capturedManifests).toHaveLength(1) + const moduleUids = capturedManifests[0].modules.map((mod: any) => mod.uid) + expect(moduleUids).toContain(existingExtension.uid) + expect(moduleUids).toContain(newExtension.uid) + expect(moduleUids.length).toBeGreaterThanOrEqual(2) + }) + test('assetsURL is only generated if affected extensions have assets', async () => { // Given vi.mocked(formData).mockReturnValue({append: vi.fn(), getHeaders: vi.fn()} as any) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts index 5a24c0916f0..e9b5d6c6ebc 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts @@ -352,11 +352,12 @@ export class DevSession { const appManifest = await appEvent.app.manifest(undefined) - // Only use inherited for UPDATE session. Create still needs the manifest in the bundle. + // Always write the full manifest to the bundle so it stays current + await writeManifestToBundle(appManifest, this.bundlePath) + + // For UPDATE sessions, only send changed modules in the API payload if (this.statusManager.status.isReady) { appManifest.modules = appManifest.modules.filter((module) => updatedUids.includes(module.uid)) - } else { - await writeManifestToBundle(appManifest, this.bundlePath) } const existingDirs = await readdir(this.bundlePath) diff --git a/packages/app/src/cli/services/function/common.test.ts b/packages/app/src/cli/services/function/common.test.ts index 045c1790981..1f0d1805d4a 100644 --- a/packages/app/src/cli/services/function/common.test.ts +++ b/packages/app/src/cli/services/function/common.test.ts @@ -17,6 +17,8 @@ import {renderAutocompletePrompt, renderFatalError} from '@shopify/cli-kit/node/ import {joinPath} from '@shopify/cli-kit/node/path' import {isTerminalInteractive} from '@shopify/cli-kit/node/context/local' import {fileExists} from '@shopify/cli-kit/node/fs' +import type {Project} from '../../models/project/project.js' +import type {ActiveConfig} from '../../models/project/active-config.js' vi.mock('../app-context.js') vi.mock('@shopify/cli-kit/node/ui') @@ -36,6 +38,8 @@ beforeEach(async () => { developerPlatformClient: testDeveloperPlatformClient(), specifications: [], organization: testOrganization(), + project: {} as unknown as Project, + activeConfig: {isLinked: true, hiddenConfig: {}} as unknown as ActiveConfig, }) vi.mocked(renderFatalError).mockReturnValue('') vi.mocked(renderAutocompletePrompt).mockResolvedValue(ourFunction) diff --git a/packages/app/src/cli/services/function/ui/components/Replay/hooks/useFunctionWatcher.test.tsx b/packages/app/src/cli/services/function/ui/components/Replay/hooks/useFunctionWatcher.test.tsx index 5ca209b21fe..57bdece1b04 100644 --- a/packages/app/src/cli/services/function/ui/components/Replay/hooks/useFunctionWatcher.test.tsx +++ b/packages/app/src/cli/services/function/ui/components/Replay/hooks/useFunctionWatcher.test.tsx @@ -65,6 +65,9 @@ const SECOND_EXEC_RESPONSE = { describe('useFunctionWatcher', () => { beforeEach(() => { vi.useFakeTimers() + // Prevent real chokidar/fsevents watchers from being created in tests. + // The hook only needs the AppEventWatcher as an EventEmitter, not real file watching. + vi.spyOn(AppEventWatcher.prototype, 'start').mockResolvedValue() }) afterEach(() => { diff --git a/packages/app/src/cli/services/store-context.test.ts b/packages/app/src/cli/services/store-context.test.ts index e622f2dad67..6b3bd1e0306 100644 --- a/packages/app/src/cli/services/store-context.test.ts +++ b/packages/app/src/cli/services/store-context.test.ts @@ -15,6 +15,8 @@ import {vi, describe, test, expect} from 'vitest' import {hashString} from '@shopify/cli-kit/node/crypto' import {inTemporaryDirectory, mkdir, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' +import type {Project} from '../models/project/project.js' +import type {ActiveConfig} from '../models/project/active-config.js' vi.mock('./dev/fetch') vi.mock('./dev/select-store') @@ -43,6 +45,8 @@ describe('storeContext', () => { developerPlatformClient: mockDeveloperPlatformClient, remoteApp: testOrganizationApp(), specifications: [], + project: {} as unknown as Project, + activeConfig: {isLinked: true, hiddenConfig: {}} as unknown as ActiveConfig, } test('uses explicitly provided storeFqdn', async () => { diff --git a/packages/e2e/tests/dev-hot-reload.spec.ts b/packages/e2e/tests/dev-hot-reload.spec.ts new file mode 100644 index 00000000000..13c731c14cd --- /dev/null +++ b/packages/e2e/tests/dev-hot-reload.spec.ts @@ -0,0 +1,198 @@ +/* eslint-disable no-console */ +/* eslint-disable no-restricted-imports */ +import {tomlAppFixture as test} from '../setup/toml-app.js' +import {requireEnv} from '../setup/env.js' +import {expect} from '@playwright/test' +import * as fs from 'fs' +import * as path from 'path' + +/** + * Dev hot-reload tests. + * + * These exercise the file watcher and dev-session reload pipeline end-to-end: + * - Editing the app config TOML triggers a reload + * - Creating a new extension mid-dev is detected by the file watcher + * - Deleting an extension mid-dev is detected by the file watcher + * + * All tests start `shopify app dev` via PTY (with no custom extensions — just the + * built-in config extensions from the app TOML), wait for the initial "Ready" message, + * then mutate the filesystem and assert on the CLI's output messages. + * + * Key output strings we assert on (from dev-session-logger.ts / dev-session-status-manager.ts): + * - "Ready, watching for changes in your app" — initial ready + * - "Updated dev preview on " — successful dev session update + * - "App config updated" — app TOML change detected + * - "Extension created" — new extension detected by watcher + * - "Extension deleted" — extension removal detected by watcher + * + * For the app config edit test, we modify scopes in shopify.app.toml which triggers + * the reload pipeline through the config extensions (app_access, etc.) without needing + * any custom extensions — this matches the existing toml-config fixture exactly. + * + * For create/delete tests, we use flow_trigger extensions (build mode "none" — no + * compilation, no theme-check). The dev session API may reject these extensions with + * a validation error, but the watcher detection and app reload still happen and are + * what we assert on. The CLI stays alive after API errors. + */ + +const READY_MESSAGE = 'Ready, watching for changes in your app' +const UPDATED_MESSAGE = 'Updated dev preview' + +/** + * Write a minimal flow_trigger extension to disk. Flow triggers have build mode 'none' + * so they don't trigger compilation, theme-check, or npm installs. + */ +function writeFlowTriggerExtension(appDir: string, name: string) { + const extDir = path.join(appDir, 'extensions', name) + fs.mkdirSync(extDir, {recursive: true}) + + fs.writeFileSync( + path.join(extDir, 'shopify.extension.toml'), + ` +type = "flow_trigger" +name = "${name}" +handle = "${name}" +description = "E2E test trigger" +`.trimStart(), + ) + + return extDir +} + +test.describe('Dev hot reload', () => { + test('editing app config TOML triggers reload', async ({cli, env, tomlAppDir}) => { + test.setTimeout(6 * 60 * 1000) + requireEnv(env, 'clientId', 'storeFqdn') + + // Start dev with no custom extensions — just the app config + const proc = await cli.spawn(['app', 'dev', '--path', tomlAppDir, '--skip-dependencies-installation'], { + env: {CI: ''}, + }) + + try { + await proc.waitForOutput(READY_MESSAGE, 3 * 60 * 1000) + + // Edit the app config TOML — change the scopes. This fires 'extensions_config_updated' + // on the app config path, triggering a full app reload. The app_access config extension + // will be detected as changed in the diff. + const tomlPath = path.join(tomlAppDir, 'shopify.app.toml') + const original = fs.readFileSync(tomlPath, 'utf8') + fs.writeFileSync( + tomlPath, + original.replace( + 'scopes = "read_products,write_products,read_orders"', + 'scopes = "read_products,write_products"', + ), + ) + + // The reload pipeline fires: file watcher → app reload → diff → dev session UPDATE. + // The logger emits "App config updated" for app config extension events. + await proc.waitForOutput('App config updated', 2 * 60 * 1000) + + // After the update completes, "Updated dev preview" is logged. + await proc.waitForOutput(UPDATED_MESSAGE, 2 * 60 * 1000) + + const output = proc.getOutput() + + // The scopes change was detected and the dev session was updated. + expect(output, 'Expected app config update in output').toContain('App config updated') + expect(output, 'Expected dev preview update in output').toContain(UPDATED_MESSAGE) + + // Clean exit + proc.sendKey('q') + const exitCode = await proc.waitForExit(30_000) + expect(exitCode, `dev exited with non-zero code. Output:\n${output}`).toBe(0) + } catch (error) { + console.error(`[hot-reload app-config] Captured PTY output:\n${proc.getOutput()}`) + throw error + } finally { + proc.kill() + } + }) + + test('creating a new extension mid-dev is detected', async ({cli, env, tomlAppDir}) => { + test.setTimeout(6 * 60 * 1000) + requireEnv(env, 'clientId', 'storeFqdn') + + // Start dev with no custom extensions + const proc = await cli.spawn(['app', 'dev', '--path', tomlAppDir, '--skip-dependencies-installation'], { + env: {CI: ''}, + }) + + try { + await proc.waitForOutput(READY_MESSAGE, 3 * 60 * 1000) + + // Create a new extension on disk while dev is running. + // The file watcher sees the new shopify.extension.toml, waits for the + // .shopify.lock file to be absent, then fires 'extension_folder_created'. + // This triggers a full app reload. + writeFlowTriggerExtension(tomlAppDir, 'mid-dev-ext') + + // Wait for the watcher to detect the new extension. The reload-app handler + // diffs old vs new app and logs "Extension created" for new extensions. + // NOTE: The dev session API may reject the extension with a validation error, + // but the watcher detection and reload still happen — that's what we test here. + await proc.waitForOutput('Extension created', 2 * 60 * 1000) + + const output = proc.getOutput() + expect(output, 'Expected extension created event in output').toContain('Extension created') + + // The CLI should NOT crash after an API error — it stays alive for further changes. + // Verify this by checking the process is still running (sendKey would throw if dead). + proc.sendKey('q') + const exitCode = await proc.waitForExit(30_000) + expect(exitCode, `dev exited with non-zero code. Output:\n${output}`).toBe(0) + } catch (error) { + console.error(`[hot-reload create] Captured PTY output:\n${proc.getOutput()}`) + throw error + } finally { + proc.kill() + } + }) + + test('deleting an extension mid-dev is detected', async ({cli, env, tomlAppDir}) => { + test.setTimeout(6 * 60 * 1000) + requireEnv(env, 'clientId', 'storeFqdn') + + // Start dev with no custom extensions + const proc = await cli.spawn(['app', 'dev', '--path', tomlAppDir, '--skip-dependencies-installation'], { + env: {CI: ''}, + }) + + try { + await proc.waitForOutput(READY_MESSAGE, 3 * 60 * 1000) + + // First, create an extension mid-dev so we have something to delete. + // We know from the previous test that creation is detected even if the API + // rejects the extension. + writeFlowTriggerExtension(tomlAppDir, 'doomed-ext') + await proc.waitForOutput('Extension created', 2 * 60 * 1000) + + // Give the dev session time to settle (process the create event fully) + // before triggering the delete. The watcher debounce is 200ms. + await new Promise((resolve) => setTimeout(resolve, 5000)) + + // Delete the extension directory while dev is running. + // The file watcher detects the shopify.extension.toml unlink and fires + // 'extension_folder_deleted'. The handler removes the extension from the + // app and emits a Deleted event. + fs.rmSync(path.join(tomlAppDir, 'extensions', 'doomed-ext'), {recursive: true, force: true}) + + // Wait for the watcher to detect the deletion. + await proc.waitForOutput('Extension deleted', 2 * 60 * 1000) + + const output = proc.getOutput() + expect(output, 'Expected extension deleted event in output').toContain('Extension deleted') + + // The CLI should stay alive and exit cleanly after create+delete cycle. + proc.sendKey('q') + const exitCode = await proc.waitForExit(30_000) + expect(exitCode, `dev exited with non-zero code. Output:\n${output}`).toBe(0) + } catch (error) { + console.error(`[hot-reload delete] Captured PTY output:\n${proc.getOutput()}`) + throw error + } finally { + proc.kill() + } + }) +}) diff --git a/packages/e2e/tests/multi-config-dev.spec.ts b/packages/e2e/tests/multi-config-dev.spec.ts new file mode 100644 index 00000000000..da593166d45 --- /dev/null +++ b/packages/e2e/tests/multi-config-dev.spec.ts @@ -0,0 +1,167 @@ +/* eslint-disable no-console */ +/* eslint-disable no-restricted-imports */ +import {tomlAppFixture as test} from '../setup/toml-app.js' +import {requireEnv} from '../setup/env.js' +import {expect} from '@playwright/test' +import * as fs from 'fs' +import * as path from 'path' + +/** + * Multi-config dev tests. + * + * These verify the three-stage pipeline (Project → config selection → app loading) + * correctly selects and isolates configs when multiple shopify.app..toml files exist. + * + * The tomlAppFixture creates a temp directory with the valid-app fixture and injects the + * real client_id. We add a second config (shopify.app.staging.toml) that uses the same + * client_id but a different set of scopes and extension_directories, then verify that: + * 1. `shopify app dev -c staging` uses the staging config (filename in App info) + * 2. Without -c, the default shopify.app.toml is used + * + * NOTE: The `-c` / `--config` flag is exclusive with `--client-id` (and its env var + * SHOPIFY_FLAG_CLIENT_ID). When passing `-c`, we must clear SHOPIFY_FLAG_CLIENT_ID + * from the process environment so oclif doesn't reject the mutually exclusive flags. + * + * NOTE: The "App:" row in the App info tab shows the remote app title from Partners, + * not the local TOML `name` field. Since both configs use the same client_id, the + * remote title is identical. We assert on the Config: row (filename) instead. + */ +test.describe('Multi-config dev', () => { + test('dev with -c flag loads the named config', async ({cli, env, tomlAppDir}) => { + test.setTimeout(6 * 60 * 1000) + requireEnv(env, 'clientId', 'storeFqdn') + + // Create a second config: shopify.app.staging.toml + // Uses the same client_id (required to hit the same Partners app) but different + // scopes and extension_directories so we can verify isolation. + const stagingToml = ` +client_id = "${env.clientId}" +name = "E2E Staging Config" +application_url = "https://example.com" +embedded = true + +extension_directories = ["staging-ext"] + +[access_scopes] +scopes = "read_products" + +[auth] +redirect_urls = ["https://example.com/auth/callback"] + +[webhooks] +api_version = "2025-01" + +[build] +automatically_update_urls_on_dev = true +include_config_on_deploy = true +`.trimStart() + + fs.writeFileSync(path.join(tomlAppDir, 'shopify.app.staging.toml'), stagingToml) + + // Create the staging extension directory (empty — no extensions). + fs.mkdirSync(path.join(tomlAppDir, 'staging-ext'), {recursive: true}) + + // Start dev with the -c flag pointing to the staging config. + // IMPORTANT: --config and --client-id are mutually exclusive in appFlags, + // so we must remove SHOPIFY_FLAG_CLIENT_ID from the env when using -c. + // Setting to undefined (not '') causes the spawn helper to exclude it + // from the child process environment entirely. + const proc = await cli.spawn( + ['app', 'dev', '--path', tomlAppDir, '-c', 'staging', '--skip-dependencies-installation'], + { + env: {CI: '', SHOPIFY_FLAG_CLIENT_ID: undefined} as NodeJS.ProcessEnv, + }, + ) + + try { + // Wait for the dev session to become ready. This proves: + // 1. The CLI resolved shopify.app.staging.toml (not default) + // 2. Project.load + selectActiveConfig(project, 'staging') worked + // 3. The app loaded and created a dev session successfully + await proc.waitForOutput('Ready, watching for changes in your app', 3 * 60 * 1000) + + const output = proc.getOutput() + + // The info banner at startup confirms which config file is being used. + // It prints "Using shopify.app.staging.toml for default values:" + expect(output, 'Expected staging config in info banner').toContain('Using shopify.app.staging.toml') + + // The staging config has scopes = "read_products" (vs the default's + // "read_products,write_products,read_orders"). The access scopes line + // in the dev output confirms the correct config was loaded. + expect(output, 'Expected staging scopes in output').toContain('read_products') + expect(output, 'Should not contain write_products from default config').not.toContain('write_products') + + // Press 'a' to switch to the "App info" tab, which displays the config filename. + proc.sendKey('a') + await new Promise((resolve) => setTimeout(resolve, 2000)) + + const outputAfterTab = proc.getOutput() + + // Verify the staging config filename appears in the App info tab. + // The UI renders `Config: shopify.app.staging.toml` (just the basename). + expect(outputAfterTab, 'Expected staging config filename in App info tab').toContain('shopify.app.staging.toml') + + // Clean exit + proc.sendKey('q') + const exitCode = await proc.waitForExit(30_000) + expect(exitCode, `dev exited with non-zero code. Output:\n${outputAfterTab}`).toBe(0) + } catch (error) { + console.error(`[multi-config dev] Captured PTY output:\n${proc.getOutput()}`) + throw error + } finally { + proc.kill() + } + }) + + test('dev without -c flag uses default config', async ({cli, env, tomlAppDir}) => { + test.setTimeout(6 * 60 * 1000) + requireEnv(env, 'clientId', 'storeFqdn') + + // Add a staging config so multiple configs exist + const stagingToml = ` +client_id = "${env.clientId}" +name = "E2E Staging Config" +application_url = "https://example.com" +embedded = true + +[access_scopes] +scopes = "read_products" + +[auth] +redirect_urls = ["https://example.com/auth/callback"] + +[webhooks] +api_version = "2025-01" +`.trimStart() + + fs.writeFileSync(path.join(tomlAppDir, 'shopify.app.staging.toml'), stagingToml) + + // Start dev without -c flag — should use shopify.app.toml + const proc = await cli.spawn(['app', 'dev', '--path', tomlAppDir, '--skip-dependencies-installation'], { + env: {CI: ''}, + }) + + try { + await proc.waitForOutput('Ready, watching for changes in your app', 3 * 60 * 1000) + + const output = proc.getOutput() + + // The info banner should reference the default config. + // Match the full phrase to avoid a false match against "shopify.app.staging.toml". + expect(output, 'Expected default config in info banner').toContain('Using shopify.app.toml for default values') + + // The default config has the broader scopes including write_products + expect(output, 'Expected default scopes (write_products) in output').toContain('write_products') + + proc.sendKey('q') + const exitCode = await proc.waitForExit(30_000) + expect(exitCode, `dev exited with non-zero code. Output:\n${output}`).toBe(0) + } catch (error) { + console.error(`[multi-config default] Captured PTY output:\n${proc.getOutput()}`) + throw error + } finally { + proc.kill() + } + }) +})