Merge pull request #6561 from Shopify/11-03-fix_how_app_name_is_set_from_the_apploader

Fix how app name is set from the AppLoader

Author: gonzaloriestra

.changeset/happy-jobs-type.md

@@ -0,0 +1,6 @@
+---
+'@shopify/cli-kit': minor
+'@shopify/app': minor
+---
+
+Use the handle to set the app version name, and the TOML name as a fallback

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

@@ -64,6 +64,7 @@ describe('load', () => {
   }
 
   const appConfiguration = `
+name = "my_app"
 scopes = "read_products"
 `
   const linkedAppConfiguration = `
@@ -264,7 +265,54 @@ wrong = "property"
     const app = await loadApp({directory: tmpDir, specifications: [], userProvidedConfigName: undefined})
 
     // Then
-    expect(app.name).toBe('my_app')
+    expect(app.name).toBe('for-testing')
+  })
+
+  test('uses handle from configuration as app name when present', async () => {
+    // Given
+    const appConfiguration = `
+name = "display-name"
+handle = "app-handle"
+client_id = "1234567890"
+application_url = "https://example.com/lala"
+embedded = true
+
+[webhooks]
+api_version = "2023-07"
+
+[auth]
+redirect_urls = [ "https://example.com/api/auth" ]
+`
+    await writeConfig(appConfiguration)
+
+    // When
+    const app = await loadTestingApp()
+
+    // Then
+    expect(app.name).toBe('app-handle')
+  })
+
+  test('uses name from configuration when handle is not present', async () => {
+    // Given
+    const appConfiguration = `
+name = "config-name"
+client_id = "1234567890"
+application_url = "https://example.com/lala"
+embedded = true
+
+[webhooks]
+api_version = "2023-07"
+
+[auth]
+redirect_urls = [ "https://example.com/api/auth" ]
+`
+    await writeConfig(appConfiguration)
+
+    // When
+    const app = await loadTestingApp()
+
+    // Then
+    expect(app.name).toBe('config-name')
   })
 
   test('defaults to npm as the package manager when the configuration is valid', async () => {

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

@@ -42,7 +42,6 @@ import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env'
 import {
   getDependencies,
   getPackageManager,
-  getPackageName,
   usesWorkspaces as appUsesWorkspaces,
 } from '@shopify/cli-kit/node/node-package-manager'
 import {resolveFramework} from '@shopify/cli-kit/node/framework'
@@ -343,7 +342,10 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
     const packageJSONPath = joinPath(directory, 'package.json')
 
     // These don't need to be processed again if the app is being reloaded
-    const name = this.previousApp?.name ?? (await loadAppName(directory))
+    // name is required by BrandingSchema, so it must be present in the configuration
+    const configName = (configuration as CurrentAppConfiguration).name
+    const configHandle: string | undefined = (configuration as CurrentAppConfiguration).handle
+    const name: string = this.previousApp?.name ?? configHandle ?? configName ?? ''
     const nodeDependencies = this.previousApp?.nodeDependencies ?? (await getDependencies(packageJSONPath))
     const packageManager = this.previousApp?.packageManager ?? (await getPackageManager(directory))
     const usesWorkspaces = this.previousApp?.usesWorkspaces ?? (await appUsesWorkspaces(directory))
@@ -1108,11 +1110,6 @@ export async function loadHiddenConfig(
   }
 }
 
-export async function loadAppName(appDirectory: string): Promise<string> {
-  const packageJSONPath = joinPath(appDirectory, 'package.json')
-  return (await getPackageName(packageJSONPath)) ?? basename(appDirectory)
-}
-
 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))

packages/app/src/cli/models/extensions/specifications/types/app_config.ts

@@ -12,6 +12,7 @@ import {WebhooksConfig} from './app_config_webhook.js'
  */
 export interface AppConfigurationUsedByCli {
   name: string
+  handle?: string
   application_url: string
   embedded: boolean
   app_proxy?: {

packages/app/src/cli/services/app-context.test.ts

@@ -49,7 +49,9 @@ describe('linkedAppContext', () => {
   test('returns linked app context when app is already linked', async () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
-      const content = `client_id="test-api-key"`
+      const content = `
+name = "test-app"
+client_id="test-api-key"`
       await writeAppConfig(tmp, content)
 
       // When
@@ -65,6 +67,7 @@ describe('linkedAppContext', () => {
         app: expect.objectContaining({
           configuration: {
             client_id: 'test-api-key',
+            name: 'test-app',
             path: normalizePath(joinPath(tmp, 'shopify.app.toml')),
           },
         }),
@@ -98,7 +101,10 @@ describe('linkedAppContext', () => {
           configurationPath: `${tmp}/shopify.app.stg.toml`,
           configSource: 'cached',
           configurationFileName: 'shopify.app.stg.toml',
-          basicConfiguration: {client_id: 'test-api-key', path: normalizePath(joinPath(tmp, 'shopify.app.stg.toml'))},
+          basicConfiguration: {
+            client_id: 'test-api-key',
+            path: normalizePath(joinPath(tmp, 'shopify.app.stg.toml')),
+          },
         },
         configuration: {
           client_id: 'test-api-key',
@@ -133,7 +139,9 @@ describe('linkedAppContext', () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
       vi.mocked(appFromIdentifiers).mockResolvedValue({...mockRemoteApp, apiKey: 'test-api-key-new'})
-      const content = `client_id="test-api-key-new"`
+      const content = `
+name = "test-app"
+client_id="test-api-key-new"`
       await writeAppConfig(tmp, content)
       localStorage.setCachedAppInfo({
         appId: 'test-api-key-old',
@@ -166,7 +174,9 @@ describe('linkedAppContext', () => {
   test('uses provided clientId when available and updates the app configuration', async () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
-      const content = `client_id="test-api-key"`
+      const content = `
+name = "test-app"
+client_id="test-api-key"`
       await writeAppConfig(tmp, content)
       const newClientId = 'new-api-key'
 
@@ -191,7 +201,9 @@ describe('linkedAppContext', () => {
   test('resets app when there is a valid toml but reset option is true', async () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
-      const content = `client_id="test-api-key"`
+      const content = `
+name = "test-app"
+client_id="test-api-key"`
       await writeAppConfig(tmp, content)
 
       vi.mocked(link).mockResolvedValue({
@@ -202,7 +214,10 @@ describe('linkedAppContext', () => {
           configurationPath: `${tmp}/shopify.app.toml`,
           configSource: 'cached',
           configurationFileName: 'shopify.app.toml',
-          basicConfiguration: {client_id: 'test-api-key', path: normalizePath(joinPath(tmp, 'shopify.app.toml'))},
+          basicConfiguration: {
+            client_id: 'test-api-key',
+            path: normalizePath(joinPath(tmp, 'shopify.app.toml')),
+          },
         },
         configuration: {
           client_id: 'test-api-key',
@@ -229,7 +244,9 @@ describe('linkedAppContext', () => {
   test('logs metadata', async () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
-      const content = `client_id="test-api-key"`
+      const content = `
+name = "test-app"
+client_id="test-api-key"`
       await writeAppConfig(tmp, content)
 
       // When
@@ -255,7 +272,9 @@ describe('linkedAppContext', () => {
   test('uses unsafeReportMode when provided', async () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
-      const content = `client_id="test-api-key"`
+      const content = `
+name = "test-app"
+client_id="test-api-key"`
       await writeAppConfig(tmp, content)
       const loadSpy = vi.spyOn(loader, 'loadAppUsingConfigurationState')
 
@@ -278,7 +297,9 @@ describe('linkedAppContext', () => {
   test('does not use unsafeReportMode when not provided', async () => {
     await inTemporaryDirectory(async (tmp) => {
       // Given
-      const content = `client_id="test-api-key"`
+      const content = `
+name = "test-app"
+client_id="test-api-key"`
       await writeAppConfig(tmp, content)
       const loadSpy = vi.spyOn(loader, 'loadAppUsingConfigurationState')