From 1e81644cccfaba144bee6d6487638b10ddfd356a Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Mon, 16 Mar 2026 14:01:43 -0600 Subject: [PATCH 1/7] Integrate Project model into loader and app-context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires the Project domain model into the existing loading pipeline: - getAppConfigurationState uses Project.load() for filesystem discovery - getAppConfigurationContext returns project + activeConfig + state as independent values (project is never nested inside state) - AppLoader reads from Project's pre-loaded data: extension files, web files, dotenv, hidden config, deps, package manager, workspaces - No duplicate filesystem scanning — Project discovers once, loader reads from it - AppConfigurationState no longer carries project as a field - LoadedAppContextOutput exposes project and activeConfig as top-level fields for commands - All extension/web file discovery filtered to active config's directories via config-selection functions Zero behavioral changes. All 3801 existing tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../app/src/cli/models/app/loader.test.ts | 8 +- packages/app/src/cli/models/app/loader.ts | 229 +++++++++--------- .../app/src/cli/services/app-context.test.ts | 2 + packages/app/src/cli/services/app-context.ts | 16 +- .../src/cli/services/function/common.test.ts | 4 + .../src/cli/services/store-context.test.ts | 4 + 6 files changed, 137 insertions(+), 126 deletions(-) diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index 7b44163ed3f..b23f6b6d422 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -256,7 +256,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 +267,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 +485,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 +1058,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 () => { diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 2521101367c..0834818e4f7 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -20,13 +20,7 @@ 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, RemoteAwareExtensionSpecification} 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' @@ -35,17 +29,20 @@ import {loadLocalExtensionsSpecifications} from '../extensions/load-specificatio 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, toAppConfigurationState} 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 {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' @@ -56,8 +53,7 @@ import {joinWithAnd, slugify} 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: @@ -73,8 +69,6 @@ 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. */ @@ -198,6 +192,8 @@ interface AppLoaderConstructorArgs< 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 } export async function checkFolderIsValidApp(directory: string) { @@ -240,8 +236,12 @@ async function findWebConfigPaths(appDirectory: string, webDirectories?: string[ 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} @@ -257,7 +257,10 @@ async function loadSingleWeb(webConfigPath: string, abortOrReport: AbortOrReport * 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'> & { + options: Omit< + AppLoaderConstructorArgs, + 'loadedConfiguration' | 'project' + > & { directory: string userProvidedConfigName: string | undefined specifications: TModuleSpec[] @@ -266,12 +269,13 @@ export async function loadApp> { const specifications = options.specifications - const state = await getAppConfigurationState(options.directory, options.userProvidedConfigName) + const {project, state} = await getAppConfigurationContext(options.directory, options.userProvidedConfigName) const loadedConfiguration = await loadAppConfigurationFromState(state, specifications, options.remoteFlags ?? []) const loader = new AppLoader({ mode: options.mode, loadedConfiguration, + project, }) return loader.loaded() } @@ -340,17 +344,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,12 +364,13 @@ export async function loadOpaqueApp(options: { } export async function reloadApp(app: AppLinkedInterface): Promise { - const state = await getAppConfigurationState(app.directory, basename(app.configPath)) + const {project, state} = await getAppConfigurationContext(app.directory, basename(app.configuration.path)) const loadedConfiguration = await loadAppConfigurationFromState(state, app.specifications, app.remoteFlags ?? []) const loader = new AppLoader({ loadedConfiguration, previousApp: app, + project, }) return loader.loaded() @@ -378,10 +382,12 @@ export async function loadAppUsingConfigurationState( specifications, remoteFlags, mode, + project, }: { specifications: RemoteAwareExtensionSpecification[] remoteFlags?: Flag[] mode: AppLoaderMode + project: Project }, ): Promise> { const loadedConfiguration = await loadAppConfigurationFromState(configState, specifications, remoteFlags ?? []) @@ -389,6 +395,7 @@ export async function loadAppUsingConfigurationState( const loader = new AppLoader({ mode, loadedConfiguration, + project, }) return loader.loaded() } @@ -416,13 +423,20 @@ class AppLoader private readonly previousApp: AppLinkedInterface | undefined + private readonly project: Project - constructor({mode, loadedConfiguration, previousApp}: AppLoaderConstructorArgs) { + constructor({mode, loadedConfiguration, previousApp, project}: AppLoaderConstructorArgs) { this.mode = mode ?? 'strict' this.specifications = loadedConfiguration.specifications this.remoteFlags = loadedConfiguration.remoteFlags this.loadedConfiguration = loadedConfiguration this.previousApp = previousApp + this.project = project + } + + private get activeConfigFile(): TomlFile | undefined { + const configPath = this.loadedConfiguration.configuration.path + return this.project.appConfigFiles.find((file) => file.path === configPath) } async loaded() { @@ -431,22 +445,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}`)) @@ -624,16 +640,18 @@ class AppLoader { - return joinPath(appDirectory, extensionPath, '*.extension.toml') - }) - extensionConfigPaths.push(`!${joinPath(appDirectory, '**/node_modules/**')}`) - const configPaths = await glob(extensionConfigPaths) + private async createExtensionInstances(appDirectory: string, _extensionDirectories?: 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 + const configPaths = extensionFiles.map((file) => file.path) return configPaths.map(async (configurationPath) => { const directory = dirname(configurationPath) - const obj = await loadConfigurationFileContent(configurationPath) + const projectFile = extensionFiles.find((file) => file.path === configurationPath) + const obj = projectFile?.content ?? (await loadConfigurationFileContent(configurationPath)) const parseResult = ExtensionsArraySchema.safeParse(obj) if (!parseResult.success) { this.abortOrReport( @@ -648,7 +666,12 @@ class AppLoader { const mergedConfig = {...configuration, ...extensionConfig} @@ -897,6 +920,30 @@ export type AppConfigurationState = AppConfigurationStateBasics & { isLinked: boolean } +/** + * Get the app configuration context from the file system. + * + * Returns the Project, ActiveConfig, and AppConfigurationState as separate values. + * Prefer this over getAppConfigurationState when you need access to the Project. + * + * @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 The project, active config selection, and legacy configuration state. + */ +export async function getAppConfigurationContext( + workingDirectory: string, + userProvidedConfigName?: string, +): Promise<{project: Project; activeConfig: ActiveConfig; state: AppConfigurationState}> { + const project = await Project.load(workingDirectory) + const activeConfig = await selectActiveConfig(project, userProvidedConfigName) + const file = activeConfig.file.content + + const parsedConfig = await parseConfigurationFile(AppSchema, activeConfig.file.path, abort, file) + const basicConfiguration = {...file, ...parsedConfig} + const state = toAppConfigurationState(project, activeConfig, basicConfiguration) + return {project, activeConfig, state} +} + /** * Get the app configuration state from the file system. * @@ -910,53 +957,8 @@ export async function getAppConfigurationState( 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, - } + const {state} = await getAppConfigurationContext(workingDirectory, userProvidedConfigName) + return state } /** @@ -1083,34 +1085,23 @@ export async function getAppDirectory(directory: string) { async function getAllLinkedConfigClientIds( appDirectory: string, prefetchedConfigs: {[key: string]: string | number | undefined}, + existingProject?: Project, ): 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) + const project = existingProject ?? (await Project.load(appDirectory)) + + const entries: [string, string][] = project.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) } diff --git a/packages/app/src/cli/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index c1bd6d673aa..270d149b9dc 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() }) diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index b33a067c34b..0413da69f6a 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -7,11 +7,13 @@ 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, loadAppUsingConfigurationState, loadApp} 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 type {ActiveConfig} from '../models/project/active-config.js' export interface LoadedAppContextOutput { app: AppLinkedInterface @@ -19,6 +21,8 @@ export interface LoadedAppContextOutput { developerPlatformClient: DeveloperPlatformClient organization: Organization specifications: RemoteAwareExtensionSpecification[] + project: Project + activeConfig: ActiveConfig } /** @@ -69,7 +73,7 @@ export async function linkedAppContext({ unsafeReportMode = false, }: LoadedAppContextOptions): Promise { // Get current app configuration state - let configState = await getAppConfigurationState(directory, userProvidedConfigName) + let {project, activeConfig, state: configState} = await getAppConfigurationContext(directory, userProvidedConfigName) let remoteApp: OrganizationApp | undefined if (!configState.isLinked || forceRelink) { @@ -77,6 +81,11 @@ export async function linkedAppContext({ 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, configState.configurationFileName) + project = reloaded.project + activeConfig = reloaded.activeConfig + configState = reloaded.state } // If the clientId is provided, update the configuration state with the new clientId @@ -101,6 +110,7 @@ export async function linkedAppContext({ specifications, remoteFlags: remoteApp.flags, mode: unsafeReportMode ? 'report' : 'strict', + project, }) // 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/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/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 () => { From e775694ff36f150ae484e04303a6db33842979fb Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Tue, 17 Mar 2026 16:43:59 -0600 Subject: [PATCH 2/7] Decompose loader into composable stages with narrow interfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the monolithic loadApp pipeline with composable stages: - loadApp is now a thin wrapper: getAppConfigurationContext → loadAppFromContext - loadAppFromContext takes narrow Project + ActiveConfig directly - getAppConfigurationContext is discovery-only (no parsing/state construction) - ReloadState replaces passing entire AppLinkedInterface through reloads - AppLoader takes reloadState? instead of previousApp? - link() returns {remoteApp, configFileName, configuration} (no state) - linkedAppContext uses activeConfig directly, no AppConfigurationState Remove dead code: AppConfigurationState, toAppConfigurationState, loadAppConfigurationFromState, loadAppUsingConfigurationState, loadAppConfiguration, getAppConfigurationState, getAppDirectory, loadDotEnv, loadHiddenConfig, findWebConfigPaths, loadWebsForAppCreation. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../app/src/cli/commands/app/config/link.ts | 2 +- .../app/src/cli/models/app/loader.test.ts | 178 +------ packages/app/src/cli/models/app/loader.ts | 452 ++++++------------ .../src/cli/models/project/active-config.ts | 42 +- .../app/src/cli/services/app-context.test.ts | 24 +- packages/app/src/cli/services/app-context.ts | 32 +- .../src/cli/services/app/config/link.test.ts | 23 +- .../app/src/cli/services/app/config/link.ts | 16 +- .../src/cli/services/app/config/use.test.ts | 83 +--- .../app/src/cli/services/app/config/use.ts | 9 +- packages/app/src/cli/services/context.test.ts | 17 +- 11 files changed, 239 insertions(+), 639 deletions(-) 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/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index b23f6b6d422..d971e354e6d 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -3,14 +3,12 @@ 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' @@ -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' @@ -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,6 +3621,7 @@ value = true }) }) +<<<<<<< HEAD describe('loadHiddenConfig', () => { test('returns empty object if hidden config file does not exist', async () => { await inTemporaryDirectory(async (tmpDir) => { @@ -3705,94 +3644,7 @@ describe('loadHiddenConfig', () => { }) }) - 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 0834818e4f7..7e8c69ba1c2 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -6,31 +6,25 @@ import { WebType, getAppScopesArray, AppConfigurationInterface, - AppConfiguration, CurrentAppConfiguration, getAppVersionedSchema, AppSchema, - BasicAppConfigurationWithoutModules, SchemaForConfig, AppLinkedInterface, - AppHiddenConfig, } from './app.js' import {parseHumanReadableError} from './error-parsing.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} from '../extensions/specification.js' +import {ExtensionSpecification} 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, toAppConfigurationState} from '../project/active-config.js' +import {selectActiveConfig} from '../project/active-config.js' import { resolveDotEnv, resolveHiddenConfig, @@ -38,10 +32,9 @@ import { 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 {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' @@ -63,6 +56,19 @@ import type {ActiveConfig} from '../project/active-config.js' */ 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) => { @@ -190,10 +196,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) { @@ -205,37 +211,24 @@ 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, @@ -256,26 +249,98 @@ async function loadSingleWeb( * 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< - AppLoaderConstructorArgs, - 'loadedConfiguration' | 'project' - > & { - 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 + } + delete rawConfig.path + + 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, + )) as CurrentAppConfiguration + + const allClientIdsByConfigName = getAllLinkedConfigClientIds(project.appConfigFiles, { + [configurationFileName]: configuration.client_id, + }) + + let configurationLoadResultMetadata: ConfigurationLoadResultMetadata = { + usesLinkedConfig: false, + allClientIdsByConfigName, + } + + 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 {project, state} = await getAppConfigurationContext(options.directory, options.userProvidedConfigName) - const loadedConfiguration = await loadAppConfigurationFromState(state, specifications, options.remoteFlags ?? []) + configurationLoadResultMetadata = { + ...configurationLoadResultMetadata, + usesLinkedConfig: true, + name: configurationFileName, + gitTracked, + source: activeConfig.source, + usesCliManagedUrls: configuration.build?.automatically_update_urls_on_dev, + } + + const loadedConfiguration: ConfigurationLoaderResult = { + directory: project.directory, + configuration, + configurationLoadResultMetadata, + configSchema, + specifications, + remoteFlags, + } const loader = new AppLoader({ - mode: options.mode, + mode, loadedConfiguration, project, + reloadState, }) return loader.loaded() } @@ -364,73 +429,40 @@ export async function loadOpaqueApp(options: { } export async function reloadApp(app: AppLinkedInterface): Promise { - const {project, state} = await getAppConfigurationContext(app.directory, basename(app.configuration.path)) - const loadedConfiguration = await loadAppConfigurationFromState(state, app.specifications, app.remoteFlags ?? []) - - const loader = new AppLoader({ - loadedConfiguration, - previousApp: app, - project, - }) - - return loader.loaded() -} - -export async function loadAppUsingConfigurationState( - configState: AppConfigurationState, - { - specifications, - remoteFlags, - mode, - project, - }: { - specifications: RemoteAwareExtensionSpecification[] - remoteFlags?: Flag[] - mode: AppLoaderMode - project: Project - }, -): Promise> { - const loadedConfiguration = await loadAppConfigurationFromState(configState, specifications, remoteFlags ?? []) - - const loader = new AppLoader({ - mode, - loadedConfiguration, + const {project, activeConfig} = await getAppConfigurationContext(app.directory, basename(app.configuration.path)) + 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, project}: 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 } @@ -449,18 +481,16 @@ class AppLoader { + const rel = relativePath(appDirectory, webPath) + return rel.startsWith('web/') + }) + const usedCustomLayout = webDirectories !== undefined || !allWebsUnderStandardDir return {webs, usedCustomLayout} } @@ -581,10 +614,6 @@ class AppLoader { - return extension.handle === configuration.handle - }) - const extensionInstance = new ExtensionInstance({ configuration, configurationPath, @@ -593,9 +622,12 @@ 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' @@ -908,116 +919,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 context from the file system. * - * Returns the Project, ActiveConfig, and AppConfigurationState as separate values. - * Prefer this over getAppConfigurationState when you need access to the Project. + * 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 The project, active config selection, and legacy configuration state. + * @returns The project and active config selection. */ export async function getAppConfigurationContext( workingDirectory: string, userProvidedConfigName?: string, -): Promise<{project: Project; activeConfig: ActiveConfig; state: AppConfigurationState}> { +): Promise<{project: Project; activeConfig: ActiveConfig}> { const project = await Project.load(workingDirectory) const activeConfig = await selectActiveConfig(project, userProvidedConfigName) - const file = activeConfig.file.content - - const parsedConfig = await parseConfigurationFile(AppSchema, activeConfig.file.path, abort, file) - const basicConfiguration = {...file, ...parsedConfig} - const state = toAppConfigurationState(project, activeConfig, basicConfiguration) - return {project, activeConfig, state} -} - -/** - * Get the app configuration state 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. - * - * @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. - */ -export async function getAppConfigurationState( - workingDirectory: string, - userProvidedConfigName?: string, -): Promise { - const {state} = await getAppConfigurationContext(workingDirectory, userProvidedConfigName) - return state -} - -/** - * 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, - } + return {project, activeConfig} } async function checkIfGitTracked(appDirectory: string, configurationPath: string) { @@ -1030,7 +949,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) @@ -1041,55 +960,16 @@ 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}, - existingProject?: Project, -): Promise<{[key: string]: string}> { - const project = existingProject ?? (await Project.load(appDirectory)) - - const entries: [string, string][] = project.appConfigFiles +): {[key: string]: string} { + const entries: [string, string][] = appConfigFiles .map((tomlFile) => { const configName = basename(tomlFile.path) if (prefetchedConfigs[configName] !== undefined && typeof prefetchedConfigs[configName] === 'string') { @@ -1105,33 +985,6 @@ async function getAllLinkedConfigClientIds( 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)) @@ -1275,7 +1128,6 @@ 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` /** diff --git a/packages/app/src/cli/models/project/active-config.ts b/packages/app/src/cli/models/project/active-config.ts index fa4dcc568c2..977575f0f9d 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/loader.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/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index 270d149b9dc..40b41159915 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -154,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 @@ -220,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({ @@ -233,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() }) }) @@ -245,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({ @@ -257,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 0413da69f6a..bf9bc4144a3 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -7,12 +7,13 @@ 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 {getAppConfigurationContext, 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 { @@ -72,31 +73,26 @@ export async function linkedAppContext({ userProvidedConfigName, unsafeReportMode = false, }: LoadedAppContextOptions): Promise { - // Get current app configuration state - let {project, activeConfig, state: configState} = await getAppConfigurationContext(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, configState.configurationFileName) + const reloaded = await getAppConfigurationContext(directory, result.configFileName) project = reloaded.project activeConfig = reloaded.activeConfig - configState = reloaded.state } - // 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 + 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 @@ -105,12 +101,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', - project, + clientIdOverride: clientId && clientId !== configClientId ? clientId : undefined, }) // If the remoteApp is the same as the linked one, update the cached info. 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..4d2e2e43e0c 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) @@ -149,15 +150,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 +184,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) @@ -234,16 +219,8 @@ describe('use', () => { test('renders warning when warning message is specified', async () => { 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: [], - }) + const {configuration} = testApp({}, 'current') + mockContext(directory, configuration) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.something.toml') createConfigFile(directory, 'shopify.app.something.toml') const options = { @@ -265,16 +242,8 @@ describe('use', () => { test('does not render success when shouldRenderSuccess is false', async () => { 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: [], - }) + const {configuration} = testApp({}, 'current') + 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..6e24e5d2dd0 100644 --- a/packages/app/src/cli/services/app/config/use.ts +++ b/packages/app/src/cli/services/app/config/use.ts @@ -1,4 +1,4 @@ -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' @@ -47,11 +47,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 as AppConfiguration, {configFileName, directory}) if (shouldRenderSuccess) { renderSuccess({ diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 5e3a9c9ae06..a883af29ee2 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,17 +76,7 @@ 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 { @@ -172,7 +161,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 From e4487d98846f8b4d92fa3d4bbf762664ee4ae3c1 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Wed, 18 Mar 2026 16:02:19 -0600 Subject: [PATCH 3/7] fix rebase artifacts: conflict marker, missing import, missing configPath - Remove orphaned <<<<<<< HEAD marker and dead loadHiddenConfig tests - Re-add isAppConfigSpecification import (used at line 776) - Add missing configPath field to loadedConfiguration in loadAppFromContext - Fix configuration.path references to use configPath (path extracted from config type) - Fix testApp() call signature in use.test.ts Co-Authored-By: Claude Opus 4.6 (1M context) --- .../app/src/cli/models/app/loader.test.ts | 25 ------------------- packages/app/src/cli/models/app/loader.ts | 7 +++--- .../src/cli/services/app/config/use.test.ts | 4 +-- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index d971e354e6d..7909ea5ed1a 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -3621,31 +3621,6 @@ value = true }) }) -<<<<<<< HEAD -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({}) - }) - }) - - - 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 7e8c69ba1c2..3cb374f8035 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -17,7 +17,7 @@ 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} from '../extensions/specification.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' @@ -329,6 +329,7 @@ export async function loadAppFromContext = { directory: project.directory, + configPath: configurationPath, configuration, configurationLoadResultMetadata, configSchema, @@ -429,7 +430,7 @@ export async function loadOpaqueApp(options: { } export async function reloadApp(app: AppLinkedInterface): Promise { - const {project, activeConfig} = await getAppConfigurationContext(app.directory, basename(app.configuration.path)) + 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, @@ -477,7 +478,7 @@ class AppLoader { test('renders warning when warning message is specified', async () => { await inTemporaryDirectory(async (directory) => { // Given - const {configuration} = testApp({}, 'current') + const {configuration} = testApp({}) mockContext(directory, configuration) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.something.toml') createConfigFile(directory, 'shopify.app.something.toml') @@ -242,7 +242,7 @@ describe('use', () => { test('does not render success when shouldRenderSuccess is false', async () => { await inTemporaryDirectory(async (directory) => { // Given - const {configuration} = testApp({}, 'current') + const {configuration} = testApp({}) mockContext(directory, configuration) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.something.toml') createConfigFile(directory, 'shopify.app.something.toml') From c2ace143887d725493e5408fcdfea5bc0fd25871 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Wed, 18 Mar 2026 16:20:51 -0600 Subject: [PATCH 4/7] fix activeConfigFile getter: use configPath instead of configuration.path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same bug as the other two configuration.path references fixed in e4487d98 — configuration is Zod-parsed and has no path property. Without this fix, activeConfigFile always returns undefined, causing loadWebs() and createExtensionInstances() to load all project files instead of filtering by the active config. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/app/src/cli/models/app/loader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 3cb374f8035..2674be30c6a 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -468,7 +468,7 @@ class AppLoader file.path === configPath) } From 5f12f8e7a008ecda432799b8859164ecec321ade Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Wed, 18 Mar 2026 16:32:41 -0600 Subject: [PATCH 5/7] fix lint: unused import, unnecessary type assertion, extra blank line Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/app/src/cli/models/app/loader.test.ts | 2 +- packages/app/src/cli/models/app/loader.ts | 7 +------ packages/app/src/cli/services/context.test.ts | 2 -- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index 7909ea5ed1a..d5c3c321930 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -11,7 +11,7 @@ import { reloadApp, } 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' diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 2674be30c6a..ff61fdf6cd6 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -294,12 +294,7 @@ export async function loadAppFromContext { return { app, From 7e87e291751ec72d11f31b8de8856036f6162f6c Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Thu, 19 Mar 2026 08:12:50 -0600 Subject: [PATCH 6/7] address review: remove dead fallback, extract config-file-naming utility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Remove dead code fallback in createExtensionInstances — since configPaths is derived from extensionFiles, the find is guaranteed to match. Simplified to iterate extensionFiles directly. 2. Extract getAppConfigurationFileName, getAppConfigurationShorthand, isValidFormatAppConfigurationFileName, and AppConfigurationFileName into a leaf utility module (config-file-naming.ts). This breaks the circular import: loader → active-config → use → loader. Loader re-exports for backward compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/cli/models/app/config-file-naming.ts | 42 +++++++++++++ packages/app/src/cli/models/app/loader.ts | 62 ++++++------------- .../src/cli/models/project/active-config.ts | 2 +- 3 files changed, 61 insertions(+), 45 deletions(-) create mode 100644 packages/app/src/cli/models/app/config-file-naming.ts 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.ts b/packages/app/src/cli/models/app/loader.ts index ff61fdf6cd6..6ab9533be53 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -13,6 +13,11 @@ import { AppLinkedInterface, } 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' @@ -42,7 +47,7 @@ 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' @@ -674,12 +679,11 @@ class AppLoader file.path) - return configPaths.map(async (configurationPath) => { + return extensionFiles.map(async (extensionFile) => { + const configurationPath = extensionFile.path const directory = dirname(configurationPath) - const projectFile = extensionFiles.find((file) => file.path === configurationPath) - const obj = projectFile?.content ?? (await loadConfigurationFileContent(configurationPath)) + const obj = extensionFile.content const parseResult = ExtensionsArraySchema.safeParse(obj) if (!parseResult.success) { this.abortOrReport( @@ -698,7 +702,7 @@ class AppLoader { const mergedConfig = {...configuration, ...extensionConfig} @@ -1123,41 +1127,11 @@ async function logMetadataFromAppLoadingProcess(loadMetadata: ConfigurationLoadR }) } -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 -} +// 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 977575f0f9d..dbf8dd86eeb 100644 --- a/packages/app/src/cli/models/project/active-config.ts +++ b/packages/app/src/cli/models/project/active-config.ts @@ -1,7 +1,7 @@ import {Project} from './project.js' import {resolveDotEnv, resolveHiddenConfig} from './config-selection.js' import {AppHiddenConfig} from '../app/app.js' -import {getAppConfigurationFileName} from '../app/loader.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' From 9d589dfe2dece266124f8b2363bee91073f42861 Mon Sep 17 00:00:00 2001 From: Ryan Bahan Date: Thu, 19 Mar 2026 10:41:39 -0600 Subject: [PATCH 7/7] add integration tests for reload with new extensions mid-dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three new tests verifying that extensions added to disk after initial load are correctly discovered on reload: 1. Project.load re-scans filesystem — fresh glob finds new files 2. reloadApp finds new extensions — full reload path works 3. handleWatcherEvents produces Created event — the event handler correctly calls reloadApp and appDiff classifies the new extension as created Also fixed setupRealApp to include api_version in the function extension TOML (required by strict validation mode used during reload). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../project/project-integration.test.ts | 134 +++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-) 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..1000682445d 100644 --- a/packages/app/src/cli/models/project/project-integration.test.ts +++ b/packages/app/src/cli/models/project/project-integration.test.ts @@ -1,7 +1,9 @@ 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 {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' @@ -43,6 +45,7 @@ api_version = "2024-01" type = "product_discounts" name = "My Discount" handle = "my-discount" +api_version = "2024-01" [build] command = "cargo build" @@ -268,4 +271,133 @@ 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) + const specifications = await loadLocalExtensionsSpecifications() + + // 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) + 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) + const specifications = await loadLocalExtensionsSpecifications() + + // 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, + {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((e) => e.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') + }) + }) })