Merge pull request #6949 from Shopify/rcb/config-model-extract-path

Extract path from AppConfiguration into App.configPath

Author: ryancbahan

packages/app/src/cli/commands/app/config/pull.ts

@@ -30,16 +30,17 @@ This command reuses the existing linked app and organization and skips all inter
       userProvidedConfigName: flags.config,
     })
 
-    const {configuration} = await pull({
+    const {configuration, configPath} = await pull({
       directory: flags.path,
       configName: flags.config,
+      configPath: app.configPath,
       configuration: app.configuration,
       remoteApp,
     })
 
     renderSuccess({
       headline: `Pulled latest configuration for "${configuration.name}"`,
-      body: `Updated ${basename(configuration.path)} with the remote data.`,
+      body: `Updated ${basename(configPath)} with the remote data.`,
     })
 
     return {app}

packages/app/src/cli/commands/app/env/pull.ts

@@ -36,7 +36,7 @@ export default class EnvPull extends AppLinkedCommand {
       forceRelink: flags.reset,
       userProvidedConfigName: flags.config,
     })
-    const envFile = joinPath(app.directory, flags['env-file'] ?? getDotEnvFileName(app.configuration.path))
+    const envFile = joinPath(app.directory, flags['env-file'] ?? getDotEnvFileName(app.configPath))
     outputResult(await pullEnv({app, remoteApp, organization, envFile}))
     return {app}
   }

packages/app/src/cli/models/app/app.test-data.ts

@@ -1,7 +1,7 @@
 import {
   App,
   AppSchema,
-  AppConfigurationWithoutPath,
+  AppConfiguration,
   AppInterface,
   AppLinkedInterface,
   CurrentAppConfiguration,
@@ -82,7 +82,6 @@ import {vi} from 'vitest'
 import {joinPath} from '@shopify/cli-kit/node/path'
 
 export const DEFAULT_CONFIG = {
-  path: '/tmp/project/shopify.app.toml',
   application_url: 'https://myapp.com',
   client_id: 'api-key',
   name: 'my app',
@@ -96,14 +95,15 @@ export const DEFAULT_CONFIG = {
   },
 }
 
-export function testApp(app: Partial<AppInterface> = {}, schemaType: 'current' | 'legacy' = 'legacy'): AppInterface {
+export function testApp(app: Partial<AppInterface> = {}): AppInterface {
   const getConfig = () => {
     return DEFAULT_CONFIG as CurrentAppConfiguration
   }
 
   const newApp = new App({
     name: app.name ?? 'App',
     directory: app.directory ?? '/tmp/project',
+    configPath: app.configPath ?? '/tmp/project/shopify.app.toml',
     packageManager: app.packageManager ?? 'yarn',
     configuration: app.configuration ?? getConfig(),
     nodeDependencies: app.nodeDependencies ?? {},
@@ -137,7 +137,7 @@ export function testApp(app: Partial<AppInterface> = {}, schemaType: 'current' |
 }
 
 export function testAppLinked(app: Partial<AppInterface> = {}): AppLinkedInterface {
-  return testApp(app, 'current') as AppLinkedInterface
+  return testApp(app) as AppLinkedInterface
 }
 
 interface TestAppWithConfigOptions {
@@ -188,7 +188,7 @@ export function testOrganizationApp(app: Partial<OrganizationApp> = {}): Organiz
   return {...defaultApp, ...app}
 }
 
-export const placeholderAppConfiguration: AppConfigurationWithoutPath = {
+export const placeholderAppConfiguration: AppConfiguration = {
   client_id: '',
 }
 

packages/app/src/cli/models/app/app.test.ts

@@ -29,7 +29,6 @@ import {joinPath} from '@shopify/cli-kit/node/path'
 import {AbortError} from '@shopify/cli-kit/node/error'
 
 const CORRECT_CURRENT_APP_SCHEMA: CurrentAppConfiguration = {
-  path: '',
   name: 'app 1',
   client_id: '12345',
   extension_directories: ['extensions/*'],
@@ -574,14 +573,11 @@ describe('allExtensions', () => {
 
   test('keeps declarative webhook config when flag is enabled', async () => {
     const webhookExtensions = await testWebhookExtensions({complianceTopics: true})
-    const app = testApp(
-      {
-        configuration: CORRECT_CURRENT_APP_SCHEMA,
-        allExtensions: webhookExtensions,
-        remoteFlags: [],
-      },
-      'current',
-    )
+    const app = testApp({
+      configuration: CORRECT_CURRENT_APP_SCHEMA,
+      allExtensions: webhookExtensions,
+      remoteFlags: [],
+    })
 
     const webhookConfig = app.allExtensions.find((ext) => ext.handle === 'webhooks')!
       .configuration as unknown as WebhookTestConfig
@@ -596,33 +592,27 @@ describe('allExtensions', () => {
 
   test('includes configuration extensions when include_config_on_deploy is enabled', async () => {
     const configExtension = await testAppAccessConfigExtension()
-    const app = testApp(
-      {
-        configuration: CORRECT_CURRENT_APP_SCHEMA,
-        allExtensions: [configExtension],
-      },
-      'current',
-    )
+    const app = testApp({
+      configuration: CORRECT_CURRENT_APP_SCHEMA,
+      allExtensions: [configExtension],
+    })
 
     expect(app.allExtensions).toContain(configExtension)
   })
 
   test('includes configuration extensions by default when include_config_on_deploy is undefined', async () => {
     const configExtension = await testAppAccessConfigExtension()
-    const app = testApp(
-      {
-        configuration: {
-          ...CORRECT_CURRENT_APP_SCHEMA,
-          build: {
-            automatically_update_urls_on_dev: true,
-            dev_store_url: 'https://google.com',
-            include_config_on_deploy: undefined,
-          },
+    const app = testApp({
+      configuration: {
+        ...CORRECT_CURRENT_APP_SCHEMA,
+        build: {
+          automatically_update_urls_on_dev: true,
+          dev_store_url: 'https://google.com',
+          include_config_on_deploy: undefined,
         },
-        allExtensions: [configExtension],
       },
-      'current',
-    )
+      allExtensions: [configExtension],
+    })
 
     expect(app.allExtensions).toContain(configExtension)
   })
@@ -637,13 +627,10 @@ describe('allExtensions', () => {
       },
     }
     const configExtension = await testAppAccessConfigExtension()
-    const app = testApp(
-      {
-        configuration,
-        allExtensions: [configExtension],
-      },
-      'current',
-    )
+    const app = testApp({
+      configuration,
+      allExtensions: [configExtension],
+    })
 
     expect(app.allExtensions).toHaveLength(0)
   })

packages/app/src/cli/models/app/app.ts

@@ -84,13 +84,12 @@ export interface AppHiddenConfig {
  *
  * Try to avoid using this: generally you should be working with a more specific type.
  */
-export type AppConfiguration = zod.infer<typeof AppSchema> & {path: string}
-export type AppConfigurationWithoutPath = zod.infer<typeof AppSchema>
+export type AppConfiguration = zod.infer<typeof AppSchema>
 
 /**
  * App configuration for a normal, linked, app. Doesn't include properties that are module derived.
  */
-export type BasicAppConfigurationWithoutModules = zod.infer<typeof AppSchema> & {path: string}
+export type BasicAppConfigurationWithoutModules = zod.infer<typeof AppSchema>
 
 /**
  * The build section for a normal, linked app. The options here tweak the CLI's behavior when working with the app.
@@ -103,12 +102,12 @@ export type CliBuildPreferences = BasicAppConfigurationWithoutModules['build']
 export type CurrentAppConfiguration = BasicAppConfigurationWithoutModules & AppConfigurationUsedByCli
 
 /** Validation schema that produces a provided app configuration type */
-export type SchemaForConfig<TConfig extends {path: string}> = ZodObjectOf<Omit<TConfig, 'path'>>
+export type SchemaForConfig<TConfig> = ZodObjectOf<TConfig>
 
 export function getAppVersionedSchema(
   specs: ExtensionSpecification[],
   allowDynamicallySpecifiedConfigs = true,
-): ZodObjectOf<Omit<CurrentAppConfiguration, 'path'>> {
+): ZodObjectOf<CurrentAppConfiguration> {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   const schema = specs.reduce<any>((schema, spec) => spec.contributeToAppConfigurationSchema(schema), AppSchema)
 
@@ -144,7 +143,7 @@ export function appHiddenConfigPath(appDirectory: string) {
  * Get the field names from the configuration that aren't found in the basic built-in app configuration schema.
  */
 export function filterNonVersionedAppFields(configuration: object): string[] {
-  const builtInFieldNames = Object.keys(AppSchema.shape).concat('path', 'organization_id')
+  const builtInFieldNames = Object.keys(AppSchema.shape).concat('organization_id')
   return Object.keys(configuration).filter((fieldName) => {
     return !builtInFieldNames.includes(fieldName)
   })
@@ -194,6 +193,7 @@ export interface AppConfigurationInterface<
   TModuleSpec extends ExtensionSpecification = ExtensionSpecification,
 > {
   directory: string
+  configPath: string
   configuration: TConfig
   configSchema: SchemaForConfig<TConfig>
   specifications: TModuleSpec[]
@@ -284,6 +284,7 @@ export class App<
   name: string
   idEnvironmentVariableName: 'SHOPIFY_API_KEY' = 'SHOPIFY_API_KEY' as const
   directory: string
+  configPath: string
   packageManager: PackageManager
   configuration: TConfig
   nodeDependencies: {[key: string]: string}
@@ -292,7 +293,7 @@ export class App<
   dotenv?: DotEnvFile
   errors?: AppErrors
   specifications: TModuleSpec[]
-  configSchema: ZodObjectOf<Omit<TConfig, 'path'>>
+  configSchema: SchemaForConfig<TConfig>
   remoteFlags: Flag[]
   realExtensions: ExtensionInstance[]
   devApplicationURLs?: ApplicationURLs
@@ -301,6 +302,7 @@ export class App<
   constructor({
     name,
     directory,
+    configPath,
     packageManager,
     configuration,
     nodeDependencies,
@@ -317,6 +319,7 @@ export class App<
   }: AppConstructor<TConfig, TModuleSpec>) {
     this.name = name
     this.directory = directory
+    this.configPath = configPath
     this.packageManager = packageManager
     this.configuration = configuration
     this.nodeDependencies = nodeDependencies

packages/app/src/cli/models/app/identifiers.test.ts

@@ -46,11 +46,10 @@ describe('updateAppIdentifiers', () => {
       const app = testAppWithConfig({
         app: {
           directory: tmpDir,
+          configPath: joinPath(tmpDir, 'shopify.app.staging.toml'),
           allExtensions: [uiExtension],
         },
-        config: {
-          path: joinPath(tmpDir, 'shopify.app.staging.toml'),
-        },
+        config: {},
       })
 
       // When

packages/app/src/cli/models/app/identifiers.ts

@@ -54,7 +54,7 @@ export async function updateAppIdentifiers(
 
   if (!dotenvFile) {
     dotenvFile = {
-      path: joinPath(app.directory, getDotEnvFileName(app.configuration.path)),
+      path: joinPath(app.directory, getDotEnvFileName(app.configPath)),
       variables: {},
     }
   }

packages/app/src/cli/models/app/loader.test.ts

@@ -13,7 +13,7 @@ import {
   loadHiddenConfig,
 } from './loader.js'
 import {parseHumanReadableError} from './error-parsing.js'
-import {App, AppInterface, AppLinkedInterface, AppSchema, WebConfigurationSchema} from './app.js'
+import {App, AppConfiguration, 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'
@@ -2908,8 +2908,7 @@ describe('parseConfigurationObject', () => {
     const abortOrReport = vi.fn()
 
     const {schema} = await buildVersionedAppSchema()
-    const {path, ...toParse} = configurationObject
-    await parseConfigurationObject(schema, 'tmp', toParse, abortOrReport)
+    await parseConfigurationObject(schema, 'tmp', configurationObject, abortOrReport)
 
     expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp')
   })
@@ -3479,7 +3478,7 @@ describe('WebhooksSchema', () => {
     )} in tmp:\n\n${parseHumanReadableError(err)}`
     const abortOrReport = vi.fn()
 
-    const {path, ...toParse} = getWebhookConfig(webhookConfigOverrides)
+    const toParse = getWebhookConfig(webhookConfigOverrides)
     const parsedConfiguration = await parseConfigurationObject(WebhooksSchema, 'tmp', toParse, abortOrReport)
     return {abortOrReport, expectedFormatted, parsedConfiguration}
   }
@@ -3491,7 +3490,6 @@ describe('getAppConfigurationState', () => {
       `client_id="abcdef"`,
       {
         basicConfiguration: {
-          path: expect.stringMatching(/shopify.app.toml$/),
           client_id: 'abcdef',
         },
         isLinked: true,
@@ -3502,7 +3500,6 @@ describe('getAppConfigurationState', () => {
       something_extra="keep"`,
       {
         basicConfiguration: {
-          path: expect.stringMatching(/shopify.app.toml$/),
           client_id: 'abcdef',
           something_extra: 'keep',
         },
@@ -3513,7 +3510,6 @@ describe('getAppConfigurationState', () => {
       `client_id=""`,
       {
         basicConfiguration: {
-          path: expect.stringMatching(/shopify.app.toml$/),
           client_id: '',
         },
         isLinked: false,
@@ -3692,9 +3688,8 @@ describe('loadHiddenConfig', () => {
     await inTemporaryDirectory(async (tmpDir) => {
       // Given
       const configuration = {
-        path: joinPath(tmpDir, 'shopify.app.toml'),
         client_id: '12345',
-      }
+      } as AppConfiguration
       await writeFile(joinPath(tmpDir, '.gitignore'), '')
 
       // When
@@ -3714,9 +3709,8 @@ describe('loadHiddenConfig', () => {
     await inTemporaryDirectory(async (tmpDir) => {
       // Given
       const configuration = {
-        path: joinPath(tmpDir, 'shopify.app.toml'),
         client_id: '12345',
-      }
+      } as AppConfiguration
       const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json')
       await mkdir(dirname(hiddenConfigPath))
       await writeFile(
@@ -3739,9 +3733,8 @@ describe('loadHiddenConfig', () => {
     await inTemporaryDirectory(async (tmpDir) => {
       // Given
       const configuration = {
-        path: joinPath(tmpDir, 'shopify.app.toml'),
         client_id: 'not-found',
-      }
+      } as AppConfiguration
       const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json')
       await mkdir(dirname(hiddenConfigPath))
       await writeFile(
@@ -3763,9 +3756,8 @@ describe('loadHiddenConfig', () => {
     await inTemporaryDirectory(async (tmpDir) => {
       // Given
       const configuration = {
-        path: joinPath(tmpDir, 'shopify.app.toml'),
         client_id: 'not-found',
-      }
+      } as AppConfiguration
       const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json')
       await mkdir(dirname(hiddenConfigPath))
       await writeFile(
@@ -3787,9 +3779,8 @@ describe('loadHiddenConfig', () => {
     await inTemporaryDirectory(async (tmpDir) => {
       // Given
       const configuration = {
-        path: joinPath(tmpDir, 'shopify.app.toml'),
         client_id: '12345',
-      }
+      } as AppConfiguration
       const hiddenConfigPath = joinPath(tmpDir, '.shopify', 'project.json')
       await mkdir(dirname(hiddenConfigPath))
       await writeFile(hiddenConfigPath, 'invalid json')