From 48c5296028fdc006d0aaf9be8e108d4f17a7746a Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Wed, 5 Nov 2025 12:50:51 +0100 Subject: [PATCH] Remove legacy schema --- packages/app/src/cli/commands/app/release.ts | 5 - .../app/src/cli/models/app/app.test-data.ts | 31 +- packages/app/src/cli/models/app/app.test.ts | 200 +----- packages/app/src/cli/models/app/app.ts | 140 +--- .../app/src/cli/models/app/loader.test.ts | 632 ++++++++---------- packages/app/src/cli/models/app/loader.ts | 173 ++--- ...config_privacy_compliance_webhooks.test.ts | 2 +- .../app/src/cli/services/app-context.test.ts | 120 ++-- packages/app/src/cli/services/app-context.ts | 7 +- .../services/app/config/link-service.test.ts | 22 +- .../src/cli/services/app/config/link.test.ts | 46 +- .../app/src/cli/services/app/config/link.ts | 43 +- .../app/src/cli/services/app/config/pull.ts | 4 +- .../src/cli/services/app/config/use.test.ts | 4 +- .../app/src/cli/services/app/config/use.ts | 4 +- .../app/src/cli/services/app/env/pull.test.ts | 9 +- .../app/src/cli/services/app/env/show.test.ts | 9 +- .../app/write-app-configuration-file.test.ts | 1 + packages/app/src/cli/services/context.test.ts | 2 +- .../context/identifiers-extensions.test.ts | 12 +- packages/app/src/cli/services/dev.ts | 72 +- .../dev/app-events/app-event-watcher.test.ts | 38 +- .../dev/app-events/file-watcher.test.ts | 7 +- .../src/cli/services/dev/select-app.test.ts | 9 +- .../cli/services/generate/extension.test.ts | 14 +- packages/app/src/cli/services/info.test.ts | 9 +- .../partners-client.test.ts | 13 +- packages/app/src/cli/utilities/types.ts | 5 - packages/features/steps/create-app.steps.ts | 40 -- 29 files changed, 664 insertions(+), 1009 deletions(-) delete mode 100644 packages/app/src/cli/utilities/types.ts diff --git a/packages/app/src/cli/commands/app/release.ts b/packages/app/src/cli/commands/app/release.ts index 6fe89496ed1..502dde7e717 100644 --- a/packages/app/src/cli/commands/app/release.ts +++ b/packages/app/src/cli/commands/app/release.ts @@ -2,7 +2,6 @@ import {appFlags} from '../../flags.js' import {release} from '../../services/release.js' import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js' import {linkedAppContext} from '../../services/app-context.js' -import {getAppConfigurationState} from '../../models/app/loader.js' import {Flags} from '@oclif/core' import {globalFlags} from '@shopify/cli-kit/node/cli' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' @@ -60,10 +59,6 @@ export default class Release extends AppLinkedCommand { if (!hasAnyForceFlags) { requiredNonTTYFlags.push('allow-updates') } - const configurationState = await getAppConfigurationState(flags.path, flags.config) - if (configurationState.state === 'template-only' && !clientId) { - requiredNonTTYFlags.push('client-id') - } this.failMissingNonTTYFlags(flags, requiredNonTTYFlags) const {app, remoteApp, developerPlatformClient} = await linkedAppContext({ diff --git a/packages/app/src/cli/models/app/app.test-data.ts b/packages/app/src/cli/models/app/app.test-data.ts index d44964c7a40..fa7f1e7e215 100644 --- a/packages/app/src/cli/models/app/app.test-data.ts +++ b/packages/app/src/cli/models/app/app.test-data.ts @@ -1,12 +1,10 @@ import { App, - AppConfiguration, - AppConfigurationSchema, + AppSchema, AppConfigurationWithoutPath, AppInterface, AppLinkedInterface, CurrentAppConfiguration, - LegacyAppConfiguration, WebType, getAppVersionedSchema, } from './app.js' @@ -94,16 +92,13 @@ export const DEFAULT_CONFIG = { embedded: true, access_scopes: { scopes: 'read_products', + use_legacy_install_flow: true, }, } export function testApp(app: Partial = {}, schemaType: 'current' | 'legacy' = 'legacy'): AppInterface { const getConfig = () => { - if (schemaType === 'legacy') { - return {scopes: '', extension_directories: [], path: ''} - } else { - return DEFAULT_CONFIG as CurrentAppConfiguration - } + return DEFAULT_CONFIG as CurrentAppConfiguration } const newApp = new App({ @@ -126,7 +121,7 @@ export function testApp(app: Partial = {}, schemaType: 'current' | dotenv: app.dotenv, errors: app.errors, specifications: app.specifications ?? [], - configSchema: (app.configSchema ?? AppConfigurationSchema) as any, + configSchema: (app.configSchema ?? AppSchema) as any, remoteFlags: app.remoteFlags ?? [], hiddenConfig: app.hiddenConfig ?? {}, devApplicationURLs: app.devApplicationURLs, @@ -150,20 +145,6 @@ interface TestAppWithConfigOptions { config: object } -export function testAppWithLegacyConfig({ - app = {}, - config = {}, -}: TestAppWithConfigOptions): AppInterface { - const configuration: AppConfiguration = { - path: '', - scopes: '', - name: 'name', - extension_directories: [], - ...config, - } - return testApp({...app, configuration}) as AppInterface -} - export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinkedInterface { const app = testAppLinked(options?.app) app.configuration = { @@ -207,7 +188,9 @@ export function testOrganizationApp(app: Partial = {}): Organiz return {...defaultApp, ...app} } -export const placeholderAppConfiguration: AppConfigurationWithoutPath = {scopes: ''} +export const placeholderAppConfiguration: AppConfigurationWithoutPath = { + client_id: '', +} export async function testUIExtension( uiExtension: Omit, 'configuration'> & { diff --git a/packages/app/src/cli/models/app/app.test.ts b/packages/app/src/cli/models/app/app.test.ts index 636a82fb113..34dbbbe087d 100644 --- a/packages/app/src/cli/models/app/app.test.ts +++ b/packages/app/src/cli/models/app/app.test.ts @@ -1,14 +1,9 @@ import { AppSchema, CurrentAppConfiguration, - LegacyAppConfiguration, - TemplateConfigSchema, getAppScopes, getAppScopesArray, - getTemplateScopesArray, getUIExtensionRendererVersion, - isCurrentAppSchema, - isLegacyAppSchema, validateExtensionsHandlesInCollection, validateFunctionExtensionsWithUiHandle, } from './app.js' @@ -63,69 +58,32 @@ const CORRECT_CURRENT_APP_SCHEMA: CurrentAppConfiguration = { }, } -const CORRECT_LEGACY_APP_SCHEMA: LegacyAppConfiguration = { - path: '', - extension_directories: [], - web_directories: [], - scopes: 'write_products', -} - describe('app schema validation', () => { - describe('legacy schema validator', () => { - test('checks whether legacy app schema is valid -- pass', () => { - expect(isLegacyAppSchema(CORRECT_LEGACY_APP_SCHEMA)).toBe(true) - }) - test('checks whether legacy app schema is valid -- fail', () => { - const config = { - ...CORRECT_LEGACY_APP_SCHEMA, - some_other_key: 'i am not valid, i will fail', - } - expect(isLegacyAppSchema(config)).toBe(false) - }) + test('extension_directories should be transformed to double asterisks', () => { + const config = { + ...CORRECT_CURRENT_APP_SCHEMA, + extension_directories: ['extensions/*'], + } + const parsed = AppSchema.parse(config) + expect(parsed.extension_directories).toEqual(['extensions/**']) }) - describe('current schema validator', () => { - test('checks whether current app schema is valid -- pass', () => { - expect(isCurrentAppSchema(CORRECT_CURRENT_APP_SCHEMA)).toBe(true) - }) - test('checks whether current app schema is valid -- fail', () => { - const config = { - ...CORRECT_CURRENT_APP_SCHEMA, - } - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete config.client_id - - expect(isCurrentAppSchema(config)).toBe(false) - }) - - test('extension_directories should be transformed to double asterisks', () => { - const config = { - ...CORRECT_CURRENT_APP_SCHEMA, - extension_directories: ['extensions/*'], - } - const parsed = AppSchema.parse(config) - expect(parsed.extension_directories).toEqual(['extensions/**']) - }) - - test('extension_directories is not transformed if it ends with double asterisks', () => { - const config = { - ...CORRECT_CURRENT_APP_SCHEMA, - extension_directories: ['extensions/**'], - } - const parsed = AppSchema.parse(config) - expect(parsed.extension_directories).toEqual(['extensions/**']) - }) + test('extension_directories is not transformed if it ends with double asterisks', () => { + const config = { + ...CORRECT_CURRENT_APP_SCHEMA, + extension_directories: ['extensions/**'], + } + const parsed = AppSchema.parse(config) + expect(parsed.extension_directories).toEqual(['extensions/**']) + }) - test('extension_directories is not transformed if it doesnt end with a wildcard', () => { - const config = { - ...CORRECT_CURRENT_APP_SCHEMA, - extension_directories: ['extensions'], - } - const parsed = AppSchema.parse(config) - expect(parsed.extension_directories).toEqual(['extensions']) - }) + test('extension_directories is not transformed if it doesnt end with a wildcard', () => { + const config = { + ...CORRECT_CURRENT_APP_SCHEMA, + extension_directories: ['extensions'], + } + const parsed = AppSchema.parse(config) + expect(parsed.extension_directories).toEqual(['extensions']) }) }) @@ -212,117 +170,19 @@ describe('getUIExtensionRendererVersion', () => { }) describe('getAppScopes', () => { - test('returns the scopes key when schema is legacy', () => { - const config = {path: '', scopes: 'read_themes,read_products'} - expect(getAppScopes(config)).toEqual('read_themes,read_products') - }) - - test('returns the access_scopes.scopes key when schema is current', () => { + test('returns the access_scopes.scopes key', () => { const config = {...DEFAULT_CONFIG, access_scopes: {scopes: 'read_themes,read_themes'}} expect(getAppScopes(config)).toEqual('read_themes,read_themes') }) }) describe('getAppScopesArray', () => { - test('returns the scopes key when schema is legacy', () => { - const config = {path: '', scopes: 'read_themes, read_order ,write_products'} - expect(getAppScopesArray(config)).toEqual(['read_themes', 'read_order', 'write_products']) - }) - - test('returns the access_scopes.scopes key when schema is current', () => { + test('returns the access_scopes.scopes key', () => { const config = {...DEFAULT_CONFIG, access_scopes: {scopes: 'read_themes, read_order ,write_products'}} expect(getAppScopesArray(config)).toEqual(['read_themes', 'read_order', 'write_products']) }) }) -describe('TemplateConfigSchema', () => { - test('parses config with legacy scopes format', () => { - const config = {scopes: 'read_products,write_products'} - const result = TemplateConfigSchema.parse(config) - expect(result.scopes).toEqual('read_products,write_products') - }) - - test('parses config with access_scopes format', () => { - const config = {access_scopes: {scopes: 'read_products,write_products'}} - const result = TemplateConfigSchema.parse(config) - expect(result.access_scopes?.scopes).toEqual('read_products,write_products') - }) - - test('preserves extra keys like metafields via passthrough', () => { - const config = { - scopes: 'write_products', - product: { - metafields: { - app: { - demo_info: { - type: 'single_line_text_field', - name: 'Demo Source Info', - }, - }, - }, - }, - webhooks: { - api_version: '2025-07', - subscriptions: [{uri: '/webhooks', topics: ['app/uninstalled']}], - }, - } - const result = TemplateConfigSchema.parse(config) - expect(result.product).toEqual(config.product) - expect(result.webhooks).toEqual(config.webhooks) - }) - - test('parses empty config', () => { - const config = {} - const result = TemplateConfigSchema.parse(config) - expect(result).toEqual({}) - }) -}) - -describe('getTemplateScopesArray', () => { - test('returns scopes from legacy format', () => { - const config = {scopes: 'read_themes,write_products'} - expect(getTemplateScopesArray(config)).toEqual(['read_themes', 'write_products']) - }) - - test('returns scopes from access_scopes format', () => { - const config = {access_scopes: {scopes: 'read_themes,write_products'}} - expect(getTemplateScopesArray(config)).toEqual(['read_themes', 'write_products']) - }) - - test('trims whitespace from scopes and sorts', () => { - const config = {scopes: ' write_products , read_themes '} - expect(getTemplateScopesArray(config)).toEqual(['read_themes', 'write_products']) - }) - - test('includes empty strings from consecutive commas (caller should handle)', () => { - const config = {scopes: 'read_themes,write_products'} - expect(getTemplateScopesArray(config)).toEqual(['read_themes', 'write_products']) - }) - - test('returns empty array when no scopes defined', () => { - const config = {} - expect(getTemplateScopesArray(config)).toEqual([]) - }) - - test('returns empty array when scopes is empty string', () => { - const config = {scopes: ''} - expect(getTemplateScopesArray(config)).toEqual([]) - }) - - test('returns empty array when access_scopes.scopes is empty', () => { - const config = {access_scopes: {scopes: ''}} - expect(getTemplateScopesArray(config)).toEqual([]) - }) - - test('prefers legacy scopes over access_scopes when both present', () => { - const config = { - scopes: 'read_themes', - access_scopes: {scopes: 'write_products'}, - } - expect(getTemplateScopesArray(config)).toEqual(['read_themes']) - }) -}) - describe('preDeployValidation', () => { test('throws an error when app-specific webhooks are used with legacy install flow', async () => { // Given @@ -418,18 +278,6 @@ Learn more: https://shopify.dev/docs/apps/build/authentication-authorization/app await expect(app.preDeployValidation()).resolves.not.toThrow() }) - test('does not throw an error for legacy schema apps', async () => { - // Given - const configuration: LegacyAppConfiguration = { - ...CORRECT_LEGACY_APP_SCHEMA, - scopes: 'read_orders', - } - const app = testApp(configuration, 'legacy') - - // When/Then - await expect(app.preDeployValidation()).resolves.not.toThrow() - }) - test('handles null/undefined subscriptions safely', async () => { // Given const configuration: CurrentAppConfiguration = { diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index ead77d10470..eac324f08d3 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -3,7 +3,6 @@ import {AppErrors, isWebType} from './loader.js' import {ensurePathStartsWithSlash} from './validation/common.js' import {Identifiers} from './identifiers.js' import {ExtensionInstance} from '../extensions/extension-instance.js' -import {isType} from '../../utilities/types.js' import {FunctionConfigType} from '../extensions/specifications/function.js' import {ExtensionSpecification, RemoteAwareExtensionSpecification} from '../extensions/specification.js' import {AppConfigurationUsedByCli} from '../extensions/specifications/types/app_config.js' @@ -11,10 +10,10 @@ import {EditorExtensionCollectionType} from '../extensions/specifications/editor import {UIExtensionSchema} from '../extensions/specifications/ui_extension.js' import {CreateAppOptions, Flag} from '../../utilities/developer-platform-client.js' import {AppAccessSpecIdentifier} from '../extensions/specifications/app_config_app_access.js' -import {WebhookSubscriptionSchema} from '../extensions/specifications/app_config_webhook_schemas/webhook_subscription_schema.js' import {configurationFileNames} from '../../constants.js' import {ApplicationURLs} from '../../services/dev/urls.js' import {patchAppHiddenConfigFile} from '../../services/app/patch-app-configuration-file.js' +import {WebhookSubscription} from '../extensions/specifications/types/app_config_webhook.js' import {joinPath} from '@shopify/cli-kit/node/path' import {ZodObjectOf, zod} from '@shopify/cli-kit/node/schema' import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' @@ -28,7 +27,6 @@ import { writeFileSync, } from '@shopify/cli-kit/node/fs' import {AbortError} from '@shopify/cli-kit/node/error' -import {normalizeDelimitedString} from '@shopify/cli-kit/common/string' import {JsonMapType} from '@shopify/cli-kit/node/toml' import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array' import {deepMergeObjects} from '@shopify/cli-kit/common/object' @@ -41,28 +39,6 @@ const ExtensionDirectoriesSchema = zod .transform(removeTrailingPathSeparator) .transform(fixSingleWildcards) -/** - * Schema for a freshly minted app template. - */ -export const LegacyAppSchema = zod - .object({ - client_id: zod.number().optional(), - name: zod.string().optional(), - scopes: zod - .string() - .transform((scopes) => normalizeDelimitedString(scopes) ?? '') - .default(''), - extension_directories: ExtensionDirectoriesSchema, - web_directories: zod.array(zod.string()).optional(), - webhooks: zod - .object({ - api_version: zod.string({required_error: 'String is required'}), - subscriptions: zod.array(WebhookSubscriptionSchema).optional(), - }) - .optional(), - }) - .strict() - function removeTrailingPathSeparator(value: string[] | undefined) { // eslint-disable-next-line no-useless-escape return value?.map((dir) => dir.replace(/[\/\\]+$/, '')) @@ -76,52 +52,23 @@ function fixSingleWildcards(value: string[] | undefined) { } /** - * Schema for loading template config during app init. - * Uses passthrough() to allow any extra keys from full-featured templates - * (e.g., metafields, metaobjects, webhooks) without strict validation. + * Schema for a normal, linked app. Properties from modules are not validated. */ -export const TemplateConfigSchema = zod +export const AppSchema = zod .object({ - scopes: zod - .string() - .transform((scopes) => normalizeDelimitedString(scopes) ?? '') - .optional(), - access_scopes: zod + client_id: zod.string(), + build: zod .object({ - scopes: zod.string().transform((scopes) => normalizeDelimitedString(scopes) ?? ''), + automatically_update_urls_on_dev: zod.boolean().optional(), + dev_store_url: zod.string().optional(), + include_config_on_deploy: zod.boolean().optional(), }) .optional(), + extension_directories: ExtensionDirectoriesSchema, web_directories: zod.array(zod.string()).optional(), }) .passthrough() -export type TemplateConfig = zod.infer - -export function getTemplateScopesArray(config: TemplateConfig): string[] { - const scopesString = config.scopes ?? config.access_scopes?.scopes ?? '' - if (scopesString.length === 0) return [] - return scopesString - .split(',') - .map((scope) => scope.trim()) - .sort() -} - -/** - * Schema for a normal, linked app. Properties from modules are not validated. - */ -export const AppSchema = zod.object({ - client_id: zod.string(), - build: zod - .object({ - automatically_update_urls_on_dev: zod.boolean().optional(), - dev_store_url: zod.string().optional(), - include_config_on_deploy: zod.boolean().optional(), - }) - .optional(), - extension_directories: ExtensionDirectoriesSchema, - web_directories: zod.array(zod.string()).optional(), -}) - /** * Hidden configuration for an app. Stored inside ./shopify/project.json * This is a set of values that are needed by the CLI that are not part of the app configuration. @@ -131,11 +78,6 @@ export interface AppHiddenConfig { dev_store_url?: string } -/** - * Utility schema that matches freshly minted or normal, linked, apps. - */ -export const AppConfigurationSchema = zod.union([LegacyAppSchema, AppSchema]) - // Types representing post-validated app configurations /** @@ -143,9 +85,8 @@ export const AppConfigurationSchema = zod.union([LegacyAppSchema, AppSchema]) * * Try to avoid using this: generally you should be working with a more specific type. */ -export type AppConfiguration = zod.infer & {path: string} - -export type AppConfigurationWithoutPath = zod.infer +export type AppConfiguration = zod.infer & {path: string} +export type AppConfigurationWithoutPath = zod.infer /** * App configuration for a normal, linked, app. Doesn't include properties that are module derived. @@ -162,11 +103,6 @@ export type CliBuildPreferences = BasicAppConfigurationWithoutModules['build'] */ export type CurrentAppConfiguration = BasicAppConfigurationWithoutModules & AppConfigurationUsedByCli -/** - * App configuration for a freshly minted app template. Very old apps *may* have a client_id provided. - */ -export type LegacyAppConfiguration = zod.infer & {path: string} - /** Validation schema that produces a provided app configuration type */ export type SchemaForConfig = ZodObjectOf> @@ -184,35 +120,12 @@ export function getAppVersionedSchema( } } -/** - * Check whether a shopify.app.toml schema is valid against the legacy schema definition. - * @param item - the item to validate - */ -export function isLegacyAppSchema(item: AppConfiguration): item is LegacyAppConfiguration { - const {path, ...rest} = item - return isType(LegacyAppSchema, rest) -} - -/** - * Check whether a shopify.app.toml schema is valid against the current schema definition. - * @param item - the item to validate - */ -export function isCurrentAppSchema(item: AppConfiguration): item is CurrentAppConfiguration { - const {path, ...rest} = item - return isType(AppSchema.nonstrict(), rest) -} - /** * Get scopes from a given app.toml config file. * @param config - a configuration file */ export function getAppScopes(config: AppConfiguration): string { - if (isLegacyAppSchema(config)) { - return config.scopes - } else if (isCurrentAppSchema(config)) { - return config.access_scopes?.scopes ?? '' - } - return '' + return (config as CurrentAppConfiguration).access_scopes?.scopes ?? '' } /** @@ -225,9 +138,7 @@ export function getAppScopesArray(config: AppConfiguration) { } export function usesLegacyScopesBehavior(config: AppConfiguration) { - if (isLegacyAppSchema(config)) return true - if (isCurrentAppSchema(config)) return config.access_scopes?.use_legacy_install_flow ?? false - return false + return (config as CurrentAppConfiguration).access_scopes?.use_legacy_install_flow === true } export function appHiddenConfigPath(appDirectory: string) { @@ -541,8 +452,7 @@ export class App< } get appIsEmbedded() { - if (isCurrentAppSchema(this.configuration)) return this.configuration.embedded - return this.appIsLaunchable() + return (this.configuration as CurrentAppConfiguration).embedded ?? this.appIsLaunchable() } creationDefaultOptions(): CreateAppOptions { @@ -586,25 +496,24 @@ export class App< } get includeConfigOnDeploy() { - if (isLegacyAppSchema(this.configuration)) return false return this.configuration.build?.include_config_on_deploy } private patchAppConfiguration(devApplicationURLs: ApplicationURLs) { - if (!isCurrentAppSchema(this.configuration)) return - + const config = this.configuration as CurrentAppConfiguration this.devApplicationURLs = devApplicationURLs - this.configuration.application_url = devApplicationURLs.applicationUrl + config.application_url = devApplicationURLs.applicationUrl if (devApplicationURLs.appProxy) { - this.configuration.app_proxy = { + config.app_proxy = { url: devApplicationURLs.appProxy.proxyUrl, subpath: devApplicationURLs.appProxy.proxySubPath, prefix: devApplicationURLs.appProxy.proxySubPathPrefix, } } - if (this.configuration.auth?.redirect_urls) { - this.configuration.auth.redirect_urls = devApplicationURLs.redirectUrlWhitelist + if (config.auth?.redirect_urls) { + config.auth.redirect_urls = devApplicationURLs.redirectUrlWhitelist } + this.configuration = config as TConfig } /** @@ -614,13 +523,14 @@ export class App< * @throws When app-specific webhooks are used with legacy install flow */ private validateWebhookLegacyFlowCompatibility(): void { - if (!isCurrentAppSchema(this.configuration)) return + // These fields might not exist on basic AppConfiguration but could be added by specifications + const config = this.configuration as CurrentAppConfiguration const hasAppSpecificWebhooks = - this.configuration.webhooks?.subscriptions?.some( - (subscription) => subscription.topics && subscription.topics.length > 0, + config.webhooks?.subscriptions?.some( + (subscription: WebhookSubscription) => subscription.topics && subscription.topics.length > 0, ) ?? false - const usesLegacyInstallFlow = this.configuration.access_scopes?.use_legacy_install_flow === true + const usesLegacyInstallFlow = config.access_scopes?.use_legacy_install_flow === true if (hasAppSpecificWebhooks && usesLegacyInstallFlow) { throw new AbortError( diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index bd7f91450b1..4b917331f0d 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -13,8 +13,9 @@ import { loadHiddenConfig, } from './loader.js' import {parseHumanReadableError} from './error-parsing.js' -import {App, AppLinkedInterface, LegacyAppSchema, 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' import metadata from '../../metadata.js' import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js' @@ -64,25 +65,108 @@ describe('load', () => { return loadApp({directory: tmpDir, specifications, userProvidedConfigName: undefined, ...extras}) } - const appConfiguration = ` -name = "my_app" -scopes = "read_products" -` - const linkedAppConfiguration = ` -name = "for-testing" -client_id = "1234567890" -application_url = "https://example.com/lala" -embedded = true + // Helper to get only real extensions (not configuration extensions) + function getRealExtensions(app: AppInterface) { + return app.allExtensions.filter((ext) => ext.specification.experience !== 'configuration') + } -[webhooks] -api_version = "2023-07" + /** + * Builds a TOML app configuration string with sensible defaults. + * Use this to avoid repeating similar configurations across tests. + * + * By default, includes [webhooks] and [auth] sections. Set webhooksApiVersion or redirectUrls + * to null to omit those sections when testing configurations without them. + */ + interface AppConfigOptions { + name?: string + handle?: string + clientId?: string + applicationUrl?: string + embedded?: boolean + webhooksApiVersion?: string | null + redirectUrls?: string[] | null + build?: {[key: string]: boolean | string} + access?: {[section: string]: {[key: string]: string | boolean}} + webDirectories?: string[] + extra?: string + } -[auth] -redirect_urls = [ "https://example.com/api/auth" ] + const buildAppConfiguration = (options: AppConfigOptions = {}): string => { + const { + name = 'my_app', + handle, + clientId = 'test-client-id', + applicationUrl = 'https://example.com', + embedded = true, + webhooksApiVersion = '2024-01', + redirectUrls = ['https://example.com/callback'], + build, + access, + webDirectories, + extra, + } = options + + const lines: string[] = [] + + lines.push(`name = "${name}"`) + if (handle) lines.push(`handle = "${handle}"`) + lines.push(`client_id = "${clientId}"`) + lines.push(`application_url = "${applicationUrl}"`) + lines.push(`embedded = ${embedded}`) + + if (webDirectories && webDirectories.length > 0) { + lines.push(`web_directories = ${JSON.stringify(webDirectories)}`) + } -[build] -automatically_update_urls_on_dev = true -` + if (webhooksApiVersion !== null) { + lines.push('') + lines.push('[webhooks]') + lines.push(`api_version = "${webhooksApiVersion}"`) + } + + if (redirectUrls !== null) { + lines.push('') + lines.push('[auth]') + lines.push(`redirect_urls = ${JSON.stringify(redirectUrls)}`) + } + + if (build) { + lines.push('') + lines.push('[build]') + for (const [key, value] of Object.entries(build)) { + lines.push(`${key} = ${typeof value === 'string' ? `"${value}"` : value}`) + } + } + + if (access) { + for (const [section, props] of Object.entries(access)) { + lines.push('') + lines.push(`[access.${section}]`) + for (const [key, value] of Object.entries(props)) { + lines.push(`${key} = ${typeof value === 'string' ? `"${value}"` : value}`) + } + } + } + + if (extra) { + lines.push('') + lines.push(extra) + } + + return lines.join('\n') + } + + // Minimal configuration without webhook subscriptions - used for tests that check exact extension counts + const minimalAppConfiguration = buildAppConfiguration() + + // Configuration for tests that check exact extension counts (no webhook subscriptions) + const appConfiguration = minimalAppConfiguration + // This configuration is used by tests that check specific metadata values + const linkedAppConfiguration = buildAppConfiguration({ + name: 'for-testing', + clientId: '1234567890', + build: {automatically_update_urls_on_dev: true}, + }) beforeAll(async () => { specifications = await loadLocalExtensionsSpecifications() @@ -200,22 +284,8 @@ automatically_update_urls_on_dev = true test('throws an error when the application_url is invalid', async () => { // Given - const appConfiguration = ` -name = "for-testing" -client_id = "1234567890" -application_url = "wrong" -embedded = true - -[webhooks] -api_version = "2023-07" - -[auth] -redirect_urls = [ "https://example.com/api/auth" ] - -[build] -automatically_update_urls_on_dev = true - ` - await writeConfig(appConfiguration) + const config = buildAppConfiguration({applicationUrl: 'wrong'}) + await writeConfig(config) // When/Then await expect(loadTestingApp()).rejects.toThrow(/\[application_url\]: Invalid URL/) @@ -234,16 +304,12 @@ automatically_update_urls_on_dev = true test('throws an error when the configuration file has invalid nested elements and the schema is generated from the specifications', async () => { // Given - const appConfiguration = ` -name = "for-testing" -client_id = "1234567890" -application_url = "https://example.com/lala" -embedded = true - -[access] -wrong = "property" -` - await writeConfig(appConfiguration) + const config = buildAppConfiguration({ + webhooksApiVersion: null, + redirectUrls: null, + extra: '[access]\nwrong = "property"', + }) + await writeConfig(config) // When/Then await expect(loadTestingApp()).rejects.toThrow() @@ -251,16 +317,13 @@ wrong = "property" test('loads the app when the configuration file has invalid nested elements but the schema isnt generated from the specifications', async () => { // Given - const appConfiguration = ` -name = "for-testing" -client_id = "1234567890" -application_url = "https://example.com/lala" -embedded = true - -[access] -wrong = "property" -` - await writeConfig(appConfiguration) + const config = buildAppConfiguration({ + name: 'for-testing', + webhooksApiVersion: null, + redirectUrls: null, + extra: '[access]\nwrong = "property"', + }) + await writeConfig(config) // When const app = await loadApp({directory: tmpDir, specifications: [], userProvidedConfigName: undefined}) @@ -271,20 +334,8 @@ wrong = "property" 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) + const config = buildAppConfiguration({name: 'display-name', handle: 'app-handle'}) + await writeConfig(config) // When const app = await loadTestingApp() @@ -295,19 +346,8 @@ redirect_urls = [ "https://example.com/api/auth" ] 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) + const config = buildAppConfiguration({name: 'config-name'}) + await writeConfig(config) // When const app = await loadTestingApp() @@ -522,9 +562,10 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp({mode: 'local'}) // Then - expect(app.allExtensions).toHaveLength(1) - expect(app.allExtensions[0]!.configuration.name).toBe('my_extension') - expect(app.allExtensions[0]!.configuration.type).toBe('theme') + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + expect(realExtensions[0]!.configuration.name).toBe('my_extension') + expect(realExtensions[0]!.configuration.type).toBe('theme') expect(app.errors).toBeUndefined() }) @@ -726,10 +767,8 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app with custom located web blocks', async () => { // Given - const {webDirectory} = await writeConfig(` - scopes = "" - web_directories = ["must_be_here"] - `) + const config = buildAppConfiguration({webDirectories: ['must_be_here']}) + const {webDirectory} = await writeConfig(config) await moveFile(webDirectory, joinPath(tmpDir, 'must_be_here')) // When @@ -741,10 +780,8 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app with custom located web blocks, only checks given directory', async () => { // Given - const {webDirectory} = await writeConfig(` - scopes = "" - web_directories = ["must_be_here"] - `) + const config = buildAppConfiguration({webDirectories: ['must_be_here']}) + const {webDirectory} = await writeConfig(config) await moveFile(webDirectory, joinPath(tmpDir, 'cannot_be_here')) // When @@ -809,8 +846,17 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app when it has a extension with a valid configuration using a supported extension type and in a non-conventional directory configured in the app configuration file', async () => { // Given await writeConfig(` - scopes = "" + name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true extension_directories = ["custom_extension"] + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" `) const customExtensionDirectory = joinPath(tmpDir, 'custom_extension') await mkdir(customExtensionDirectory) @@ -885,8 +931,11 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(2) - const extensions = app.allExtensions.sort((extA, extB) => (extA.name < extB.name ? -1 : 1)) + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(2) + const extensions = realExtensions.sort((extA: ExtensionInstance, extB: ExtensionInstance) => + extA.name < extB.name ? -1 : 1, + ) expect(extensions[0]!.configuration.name).toBe('my_extension_1') expect(extensions[0]!.idEnvironmentVariableName).toBe('SHOPIFY_MY_EXTENSION_1_ID') expect(extensions[1]!.configuration.name).toBe('my_extension_2') @@ -931,8 +980,11 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(2) - const extensions = app.allExtensions.sort((extA, extB) => (extA.name < extB.name ? -1 : 1)) + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(2) + const extensions = realExtensions.sort((extA: ExtensionInstance, extB: ExtensionInstance) => + extA.name < extB.name ? -1 : 1, + ) expect(extensions[0]!.configuration.name).toBe('my_extension_1') expect(extensions[0]!.configuration.type).toBe('checkout_post_purchase') expect(extensions[0]!.configuration.api_version).toBe('2022-07') @@ -1109,8 +1161,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1177,8 +1230,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1255,8 +1309,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1339,8 +1394,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1415,8 +1471,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1532,8 +1589,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1648,8 +1706,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1703,8 +1762,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1750,8 +1810,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1814,8 +1875,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1890,8 +1952,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1986,8 +2049,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -2053,8 +2117,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -2223,8 +2288,11 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(2) - const functions = app.allExtensions.sort((extA, extB) => (extA.name < extB.name ? -1 : 1)) + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(2) + const functions = realExtensions.sort((extA: ExtensionInstance, extB: ExtensionInstance) => + extA.name < extB.name ? -1 : 1, + ) expect(functions[0]!.configuration.name).toBe('my-function-1') expect(functions[1]!.configuration.name).toBe('my-function-2') expect(functions[0]!.idEnvironmentVariableName).toBe('SHOPIFY_MY_FUNCTION_1_ID') @@ -2289,7 +2357,9 @@ redirect_urls = [ "https://example.com/api/auth" ] expect(metadata.getAllPublicMetadata()).toMatchObject({ project_type: 'node', env_package_manager_workspaces: false, - ...configAsCodeLegacyMetadata(), + cmd_app_all_configs_any: true, + cmd_app_all_configs_clients: JSON.stringify({'shopify.app.toml': 'test-client-id'}), + cmd_app_linked_config_used: true, }) }) @@ -2307,7 +2377,9 @@ redirect_urls = [ "https://example.com/api/auth" ] expect(metadata.getAllPublicMetadata()).toMatchObject({ project_type: 'node', env_package_manager_workspaces: true, - ...configAsCodeLegacyMetadata(), + cmd_app_all_configs_any: true, + cmd_app_all_configs_clients: JSON.stringify({'shopify.app.toml': 'test-client-id'}), + cmd_app_linked_config_used: true, }) }) @@ -2327,21 +2399,7 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app when access.admin.direct_api_mode = "online"', async () => { // Given - const config = ` - name = "my_app" - client_id = "1234567890" - application_url = "https://example.com/lala" - embedded = true - - [webhooks] - api_version = "2023-07" - - [auth] - redirect_urls = [ "https://example.com/api/auth" ] - - [access.admin] - direct_api_mode = "online" - ` + const config = buildAppConfiguration({access: {admin: {direct_api_mode: 'online'}}}) await writeConfig(config) // When @@ -2353,21 +2411,7 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app when access.admin.direct_api_mode = "offline"', async () => { // Given - const config = ` - name = "my_app" - client_id = "1234567890" - application_url = "https://example.com/lala" - embedded = true - - [webhooks] - api_version = "2023-07" - - [auth] - redirect_urls = [ "https://example.com/api/auth" ] - - [access.admin] - direct_api_mode = "offline" - ` + const config = buildAppConfiguration({access: {admin: {direct_api_mode: 'offline'}}}) await writeConfig(config) // When @@ -2379,21 +2423,7 @@ redirect_urls = [ "https://example.com/api/auth" ] test('throws an error when access.admin.direct_api_mode is invalid', async () => { // Given - const config = ` - name = "my_app" - client_id = "1234567890" - application_url = "https://example.com/lala" - embedded = true - - [webhooks] - api_version = "2023-07" - - [auth] - redirect_urls = [ "https://example.com/api/auth" ] - - [access.admin] - direct_api_mode = "foo" - ` + const config = buildAppConfiguration({access: {admin: {direct_api_mode: 'foo'}}}) await writeConfig(config) // When @@ -2402,21 +2432,7 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app when access.admin.embedded_app_direct_api_access = true', async () => { // Given - const config = ` - name = "my_app" - client_id = "1234567890" - application_url = "https://example.com/lala" - embedded = true - - [webhooks] - api_version = "2023-07" - - [auth] - redirect_urls = [ "https://example.com/api/auth" ] - - [access.admin] - embedded_app_direct_api_access = true - ` + const config = buildAppConfiguration({access: {admin: {embedded_app_direct_api_access: true}}}) await writeConfig(config) // When @@ -2428,21 +2444,7 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app when access.admin.embedded_app_direct_api_access = false', async () => { // Given - const config = ` - name = "my_app" - client_id = "1234567890" - application_url = "https://example.com/lala" - embedded = true - - [webhooks] - api_version = "2023-07" - - [auth] - redirect_urls = [ "https://example.com/api/auth" ] - - [access.admin] - embedded_app_direct_api_access = false - ` + const config = buildAppConfiguration({access: {admin: {embedded_app_direct_api_access: false}}}) await writeConfig(config) // When @@ -2454,21 +2456,7 @@ redirect_urls = [ "https://example.com/api/auth" ] test('throws an error when access.admin.embedded_app_direct_api_access is invalid', async () => { // Given - const config = ` - name = "my_app" - client_id = "1234567890" - application_url = "https://example.com/lala" - embedded = true - - [webhooks] - api_version = "2023-07" - - [auth] - redirect_urls = [ "https://example.com/api/auth" ] - - [access.admin] - embedded_app_direct_api_access = "foo" - ` + const config = buildAppConfiguration({extra: '[access.admin]\nembedded_app_direct_api_access = "foo"'}) await writeConfig(config) // When @@ -2912,27 +2900,17 @@ describe('parseConfigurationObject', () => { expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp') }) - test('throws an error if fields are missing in a legacy schema TOML file', async () => { + test('throws an error when client_id is missing in app schema TOML file', async () => { const configurationObject = { scopes: [], } - const errorObject = [ - { - code: 'invalid_type', - expected: 'string', - received: 'array', - path: ['scopes'], - message: 'Expected string, received array', - }, - ] - const expectedFormatted = outputContent`\n${outputToken.errorText( - 'Validation errors', - )} in tmp:\n\n${parseHumanReadableError(errorObject)}` const abortOrReport = vi.fn() - await parseConfigurationObject(LegacyAppSchema, 'tmp', configurationObject, abortOrReport) + await parseConfigurationObject(AppSchema, 'tmp', configurationObject, abortOrReport) - expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp') + expect(abortOrReport).toHaveBeenCalledOnce() + const errorString = abortOrReport.mock.calls[0]![0].value + expect(errorString).toContain('[client_id]: Required') }) test('throws an error if fields are missing in a frontend config web TOML file', async () => { @@ -3496,41 +3474,35 @@ describe('WebhooksSchema', () => { describe('getAppConfigurationState', () => { test.each([ [ - `scopes = " write_xyz, write_abc "`, + `client_id="abcdef"`, { - state: 'template-only', - configSource: 'cached', - configurationFileName: 'shopify.app.toml', - appDirectory: expect.any(String), - configurationPath: expect.stringMatching(/shopify.app.toml$/), - startingOptions: { + basicConfiguration: { path: expect.stringMatching(/shopify.app.toml$/), - scopes: 'write_abc,write_xyz', + client_id: 'abcdef', }, - }, - ], - [ - `client_id="abcdef"`, - { - state: 'connected-app', - }, - ], - [ - ``, - { - state: 'template-only', + isTemplateForm: false, }, ], [ `client_id="abcdef" something_extra="keep"`, { - state: 'connected-app', basicConfiguration: { path: expect.stringMatching(/shopify.app.toml$/), client_id: 'abcdef', something_extra: 'keep', }, + isTemplateForm: false, + }, + ], + [ + `client_id=""`, + { + basicConfiguration: { + path: expect.stringMatching(/shopify.app.toml$/), + client_id: '', + }, + isTemplateForm: true, }, ], ])('loads from %s', async (content, resultShouldContain) => { @@ -3545,48 +3517,27 @@ describe('getAppConfigurationState', () => { }) }) - test('raises validation error when AppSchema is missing client_id', async () => { + test('marks config as template when client_id is empty string', async () => { await inTemporaryDirectory(async (tmpDir) => { - // We know this is a TOML file that follows the AppSchema because - // it can contain extra fields (something_extra). In LegacyAppSchema, all fields are optional. - // This content is also missing a client_id field which should throw a validation error. - const content = `something_extra = "some_value"` + const content = `client_id = ""\nsomething_extra = "some_value"` const appConfigPath = joinPath(tmpDir, 'shopify.app.toml') const packageJsonPath = joinPath(tmpDir, 'package.json') await writeFile(appConfigPath, content) await writeFile(packageJsonPath, '{}') - await expect(getAppConfigurationState(tmpDir, undefined)).rejects.toThrowError(/client_id/) + const result = await getAppConfigurationState(tmpDir, undefined) + + expect(result.basicConfiguration.client_id).toBe('') + expect(result.isTemplateForm).toBe(true) }) }) }) describe('loadConfigForAppCreation', () => { - test('extracts top-level scopes from legacy format', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const config = ` -scopes = "write_products,read_orders" -name = "my-app" - ` - await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) - await writeFile(joinPath(tmpDir, 'package.json'), '{}') - - const result = await loadConfigForAppCreation(tmpDir, 'my-app') - - expect(result).toEqual({ - isLaunchable: false, - scopesArray: ['read_orders', 'write_products'], - name: 'my-app', - directory: tmpDir, - isEmbedded: false, - }) - }) - }) - - test('extracts access_scopes.scopes from current format', async () => { + test('extracts access_scopes.scopes', async () => { await inTemporaryDirectory(async (tmpDir) => { const config = ` -client_id = "12345" +client_id = "" name = "my-app" [access_scopes] @@ -3601,15 +3552,16 @@ scopes = "read_orders,write_products" isLaunchable: false, scopesArray: ['read_orders', 'write_products'], name: 'my-app', - directory: tmpDir, + directory: normalizePath(tmpDir), isEmbedded: false, }) }) }) - test('defaults to empty scopes when scopes field is missing', async () => { + test('defaults to empty scopes when access_scopes section is missing', async () => { await inTemporaryDirectory(async (tmpDir) => { const config = ` +client_id = "" name = "my-app" ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) @@ -3621,7 +3573,7 @@ name = "my-app" isLaunchable: false, scopesArray: [], name: 'my-app', - directory: tmpDir, + directory: normalizePath(tmpDir), isEmbedded: false, }) }) @@ -3630,8 +3582,11 @@ name = "my-app" test('detects launchable app with frontend web', async () => { await inTemporaryDirectory(async (tmpDir) => { const config = ` -scopes = "write_products" +client_id = "" name = "my-app" + +[access_scopes] +scopes = "write_products" ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') @@ -3651,41 +3606,16 @@ dev = "echo 'dev'" isLaunchable: true, scopesArray: ['write_products'], name: 'my-app', - directory: tmpDir, + directory: normalizePath(tmpDir), isEmbedded: true, }) }) }) - test('ignores unknown configuration sections with legacy scopes', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const config = ` -scopes = "write_products" -name = "my-app" - -[product.metafields.app.example] -type = "single_line_text_field" -name = "Example" - ` - await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) - await writeFile(joinPath(tmpDir, 'package.json'), '{}') - - const result = await loadConfigForAppCreation(tmpDir, 'my-app') - - expect(result).toEqual({ - isLaunchable: false, - scopesArray: ['write_products'], - name: 'my-app', - directory: tmpDir, - isEmbedded: false, - }) - }) - }) - - test('ignores unknown configuration sections with access_scopes format', async () => { + test('ignores unknown configuration sections', async () => { await inTemporaryDirectory(async (tmpDir) => { const config = ` -client_id = "12345" +client_id = "" name = "my-app" [access_scopes] @@ -3704,7 +3634,7 @@ name = "Example" isLaunchable: false, scopesArray: ['write_products'], name: 'my-app', - directory: tmpDir, + directory: normalizePath(tmpDir), isEmbedded: false, }) }) @@ -3713,10 +3643,13 @@ name = "Example" test('ignores completely unrecognized configuration sections', async () => { await inTemporaryDirectory(async (tmpDir) => { const config = ` -scopes = "write_products" +client_id = "" name = "my-app" nonsense_field = "whatever" +[access_scopes] +scopes = "write_products" + [completely_made_up] foo = "bar" baz = 123 @@ -3733,7 +3666,7 @@ value = true isLaunchable: false, scopesArray: ['write_products'], name: 'my-app', - directory: tmpDir, + directory: normalizePath(tmpDir), isEmbedded: false, }) }) @@ -3741,22 +3674,6 @@ value = true }) describe('loadHiddenConfig', () => { - test('returns empty object if no client_id in configuration', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - path: joinPath(tmpDir, 'shopify.app.toml'), - scopes: 'write_products', - } - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({}) - }) - }) - test('returns empty object if hidden config file does not exist', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given @@ -3913,12 +3830,23 @@ redirect_urls = ["https://example.com/callback"] }) }) - test('returns loaded-app state for legacy app configuration', async () => { + test('returns loaded-app state for template app configuration', async () => { await inTemporaryDirectory(async (tmpDir) => { - // Given - a legacy (unlinked) app configuration + // Given - a template app configuration with empty client_id const config = ` -scopes = "write_products,read_orders" +client_id = "" name = "my-app" +application_url = "https://example.com" +embedded = true + +[webhooks] +api_version = "2023-07" + +[access_scopes] +scopes = "write_products,read_orders" + +[auth] +redirect_urls = ["https://example.com/callback"] ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') @@ -3942,9 +3870,12 @@ name = "my-app" await inTemporaryDirectory(async (tmpDir) => { // Given - a template with metafield configuration that would fail loadApp const config = ` -scopes = "write_products" +client_id = "" name = "my-app" +[access_scopes] +scopes = "write_products" + [product.metafields.app.example] type = "single_line_text_field" name = "Example" @@ -3970,10 +3901,11 @@ name = "Example" }) }) - test('returns loaded-template with access_scopes format', async () => { + test('returns loaded-template with extra unknown sections', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given - a template with access_scopes format and extra config const config = ` +client_id = "" name = "my-app" [access_scopes] @@ -4054,8 +3986,19 @@ type = "single_line_text_field" await inTemporaryDirectory(async (tmpDir) => { // Given - a custom config file name const config = ` -scopes = "write_products" +client_id = "12345" name = "my-app" +application_url = "https://example.com" +embedded = true + +[webhooks] +api_version = "2023-07" + +[access_scopes] +scopes = "write_products" + +[auth] +redirect_urls = ["https://example.com/callback"] ` await writeFile(joinPath(tmpDir, 'shopify.app.staging.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') @@ -4109,8 +4052,19 @@ foo = "bar" await inTemporaryDirectory(async (tmpDir) => { // Given - a valid config const config = ` -scopes = "write_products" +client_id = "12345" name = "my-app" +application_url = "https://example.com" +embedded = true + +[webhooks] +api_version = "2023-07" + +[access_scopes] +scopes = "write_products" + +[auth] +redirect_urls = ["https://example.com/callback"] ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index d83e575846c..83fa06feff4 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -6,20 +6,14 @@ import { WebType, getAppScopesArray, AppConfigurationInterface, - LegacyAppSchema, AppConfiguration, CurrentAppConfiguration, getAppVersionedSchema, - isCurrentAppSchema, AppSchema, - LegacyAppConfiguration, BasicAppConfigurationWithoutModules, SchemaForConfig, AppLinkedInterface, AppHiddenConfig, - isLegacyAppSchema, - TemplateConfigSchema, - getTemplateScopesArray, } from './app.js' import {parseHumanReadableError} from './error-parsing.js' import {configurationFileNames, dotEnvFileNames} from '../../constants.js' @@ -217,19 +211,19 @@ export async function checkFolderIsValidApp(directory: string) { } export async function loadConfigForAppCreation(directory: string, name: string): Promise { - const appDirectory = await getAppDirectory(directory) - const {configurationPath} = await getConfigurationPath(appDirectory, undefined) - - // Use permissive schema to allow templates with extra configuration (metafields, metaobjects, etc.) - const config = await parseConfigurationFile(TemplateConfigSchema, configurationPath) - const webs = await loadWebsForAppCreation(appDirectory, config.web_directories) + const state = await getAppConfigurationState(directory) + const config = state.basicConfiguration + const webs = await loadWebsForAppCreation(state.appDirectory, config.web_directories) const isLaunchable = webs.some((web) => isWebType(web, WebType.Frontend) || isWebType(web, WebType.Backend)) + const scopesString = (config as CurrentAppConfiguration).access_scopes?.scopes ?? '' + const scopesArray = scopesString.length ? scopesString.split(',').map((scope: string) => scope.trim()) : [] + return { isLaunchable, - scopesArray: getTemplateScopesArray(config), + scopesArray, name, - directory, + directory: state.appDirectory, // By default, and ONLY for `app init`, we consider the app as embedded if it is launchable. isEmbedded: isLaunchable, } @@ -308,6 +302,14 @@ export type OpaqueAppLoadResult = state: 'error' } +/** + * Extract scopes from raw config using access_scopes.scopes format. + */ +function extractScopesFromRawConfig(rawConfig: JsonMapType): string { + const accessScopes = rawConfig.access_scopes as {scopes?: string} | undefined + return accessScopes?.scopes ?? '' +} + /** * Load an app with relaxed validation, falling back to raw template loading if strict parsing fails. * @@ -344,13 +346,12 @@ export async function loadOpaqueApp(options: { const appDirectory = await getAppDirectory(options.directory) const {configurationPath} = await getConfigurationPath(appDirectory, options.configName) const rawConfig = await loadConfigurationFileContent(configurationPath) - const parsed = TemplateConfigSchema.parse(rawConfig) const packageManager = await getPackageManager(appDirectory) return { state: 'loaded-template', rawConfig, - scopes: getTemplateScopesArray(parsed).join(','), + scopes: extractScopesFromRawConfig(rawConfig), appDirectory, packageManager, } @@ -364,9 +365,6 @@ export async function loadOpaqueApp(options: { export async function reloadApp(app: AppLinkedInterface): Promise { const state = await getAppConfigurationState(app.directory, basename(app.configuration.path)) - if (state.state !== 'connected-app') { - throw new AbortError('Error loading the app, please check your app configuration.') - } const loadedConfiguration = await loadAppConfigurationFromState(state, app.specifications, app.remoteFlags ?? []) const loader = new AppLoader({ @@ -403,7 +401,7 @@ export async function loadAppUsingConfigurationState = TConfigState extends AppConfigurationStateLinked ? CurrentAppConfiguration - : LegacyAppConfiguration + : never export function getDotEnvFileName(configurationPath: string) { const configurationShorthand: string | undefined = getAppConfigurationShorthand(configurationPath) @@ -608,13 +606,9 @@ class AppLoader specification.uidStrategy === 'single') @@ -838,12 +832,13 @@ class AppLoader configuration.auth_callback_path).find((path) => path), - currentConfiguration.app_proxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (currentConfiguration as any).app_proxy, ) } } @@ -905,16 +900,11 @@ interface AppConfigurationStateBasics { } export type AppConfigurationStateLinked = AppConfigurationStateBasics & { - state: 'connected-app' basicConfiguration: BasicAppConfigurationWithoutModules + isTemplateForm: boolean } -type AppConfigurationStateTemplate = AppConfigurationStateBasics & { - state: 'template-only' - startingOptions: Omit -} - -type AppConfigurationState = AppConfigurationStateLinked | AppConfigurationStateTemplate +type AppConfigurationState = AppConfigurationStateLinked /** * Get the app configuration state from the file system. @@ -952,34 +942,21 @@ export async function getAppConfigurationState( const {configurationPath, configurationFileName} = await getConfigurationPath(appDirectory, configName) const file = await loadConfigurationFileContent(configurationPath) - const configFileHasNotBeenLinked = isLegacyAppSchema(file as AppConfiguration) + const parsedConfig = await parseConfigurationFile(AppSchema, configurationPath) - if (configFileHasNotBeenLinked) { - const parsedConfig = await parseConfigurationFile(LegacyAppSchema, configurationPath) - return { - appDirectory, - configurationPath, - state: 'template-only', - startingOptions: { - ...file, - ...parsedConfig, - }, - configSource, - configurationFileName, - } - } else { - const parsedConfig = await parseConfigurationFile(AppSchema, configurationPath) - return { - state: 'connected-app', - appDirectory, - configurationPath, - basicConfiguration: { - ...file, - ...parsedConfig, - }, - configSource, - configurationFileName, - } + // Check if the config is in template form by verifying if client_id is empty + const isTemplateForm = parsedConfig.client_id === '' + + return { + appDirectory, + configurationPath, + basicConfiguration: { + ...file, + ...parsedConfig, + }, + configSource, + configurationFileName, + isTemplateForm, } } @@ -996,32 +973,12 @@ async function loadAppConfigurationFromState< specifications: TModuleSpec[], remoteFlags: Flag[], ): Promise, TModuleSpec>> { - let file: JsonMapType - let schemaForConfigurationFile: SchemaForConfig> - { - let appSchema - switch (configState.state) { - case 'template-only': { - file = { - ...configState.startingOptions, - } - delete file.path - appSchema = LegacyAppSchema as unknown as SchemaForConfig> - break - } - case 'connected-app': { - file = { - ...configState.basicConfiguration, - } - delete file.path - const appVersionedSchema = getAppVersionedSchema(specifications) - appSchema = appVersionedSchema as SchemaForConfig> - break - } - } - - schemaForConfigurationFile = appSchema - } + const file: JsonMapType = { + ...configState.basicConfiguration, + } as JsonMapType + delete file.path + const appVersionedSchema = getAppVersionedSchema(specifications) + const schemaForConfigurationFile = appVersionedSchema as SchemaForConfig> const configuration = (await parseConfigurationFile( schemaForConfigurationFile, @@ -1040,30 +997,22 @@ async function loadAppConfigurationFromState< } // enhance metadata based on the config type - switch (configState.state) { - case 'template-only': { - // nothing to add - break - } - case 'connected-app': { - 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 - } + 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 as LoadedAppConfigFromConfigState).build - ?.automatically_update_urls_on_dev, - } - } + configurationLoadResultMetadata = { + ...configurationLoadResultMetadata, + usesLinkedConfig: true, + name: configState.configurationFileName, + gitTracked, + source: configState.configSource, + usesCliManagedUrls: (configuration as LoadedAppConfigFromConfigState).build + ?.automatically_update_urls_on_dev, } return { diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts b/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts index c7d99cb753c..4558a4c2c6d 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts @@ -99,7 +99,7 @@ describe('privacy_compliance_webhooks', () => { }, } const privacyComplianceSpec = spec - const appConfiguration = {application_url: 'https://example.com/', scopes: ''} + const appConfiguration = {client_id: 'test-client-id', application_url: 'https://example.com/', scopes: ''} // When const result = privacyComplianceSpec.transformLocalToRemote!(object, appConfiguration) diff --git a/packages/app/src/cli/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index 37ffaf956cc..36304a78997 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -80,61 +80,6 @@ client_id="test-api-key"` }) }) - test('links app when it is not linked, and config file is cached', async () => { - await inTemporaryDirectory(async (tmp) => { - const content = '' - await writeAppConfig(tmp, content, 'shopify.app.stg.toml') - localStorage.setCachedAppInfo({ - appId: 'test-api-key', - title: 'Test App', - directory: tmp, - orgId: 'test-org-id', - configFile: 'shopify.app.stg.toml', - }) - - // Given - vi.mocked(link).mockResolvedValue({ - remoteApp: mockRemoteApp, - state: { - state: 'connected-app', - appDirectory: tmp, - 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')), - }, - }, - configuration: { - client_id: 'test-api-key', - name: 'test-app', - application_url: 'https://test-app.com', - path: normalizePath(joinPath(tmp, 'shopify.app.stg.toml')), - embedded: false, - }, - }) - - // When - const result = await linkedAppContext({ - directory: tmp, - forceRelink: false, - userProvidedConfigName: undefined, - clientId: undefined, - }) - - // Then - expect(result).toEqual({ - app: expect.any(Object), - remoteApp: mockRemoteApp, - developerPlatformClient: expect.any(Object), - specifications: [], - organization: mockOrganization, - }) - expect(link).toHaveBeenCalledWith({directory: tmp, apiKey: undefined, configName: 'shopify.app.stg.toml'}) - }) - }) - test('updates cached app info when remoteApp matches', async () => { await inTemporaryDirectory(async (tmp) => { // Given @@ -209,11 +154,11 @@ client_id="test-api-key"` vi.mocked(link).mockResolvedValue({ remoteApp: mockRemoteApp, state: { - state: 'connected-app', appDirectory: tmp, configurationPath: `${tmp}/shopify.app.toml`, configSource: 'cached', configurationFileName: 'shopify.app.toml', + isTemplateForm: false, basicConfiguration: { client_id: 'test-api-key', path: normalizePath(joinPath(tmp, 'shopify.app.toml')), @@ -329,6 +274,15 @@ describe('localAppContext', () => { // Given const content = ` name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeAppConfig(tmp, content) @@ -359,6 +313,15 @@ describe('localAppContext', () => { // Given const content = ` name = "test-app-custom" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeAppConfig(tmp, content, 'shopify.app.custom.toml') @@ -384,6 +347,15 @@ describe('localAppContext', () => { // Given const appContent = ` name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` const extensionContent = ` type = "ui_extension" @@ -402,6 +374,7 @@ describe('localAppContext', () => { await writeFile(joinPath(srcDir, 'index.js'), '// Extension code') // Mock local specifications to include ui_extension with proper validation + // Also include app_access and webhooks specs that contribute auth and webhooks to schema vi.mocked(loadLocalExtensionsSpecifications).mockResolvedValue([ { identifier: 'ui_extension', @@ -427,6 +400,36 @@ describe('localAppContext', () => { validate: async () => ({isErr: () => false, isOk: () => true} as any), contributeToAppConfigurationSchema: (schema: any) => schema, } as any, + { + identifier: 'app_access', + experience: 'configuration', + uidStrategy: 'single', + appModuleFeatures: () => [], + parseConfigurationObject: (obj: any) => ({ + state: 'ok', + data: obj, + errors: undefined, + }), + contributeToAppConfigurationSchema: (schema: any) => { + // Mock contribution of auth field + return schema + }, + } as any, + { + identifier: 'webhooks', + experience: 'configuration', + uidStrategy: 'single', + appModuleFeatures: () => [], + parseConfigurationObject: (obj: any) => ({ + state: 'ok', + data: obj, + errors: undefined, + }), + contributeToAppConfigurationSchema: (schema: any) => { + // Mock contribution of webhooks field + return schema + }, + } as any, ]) // When @@ -435,8 +438,9 @@ describe('localAppContext', () => { }) // Then - expect(result.allExtensions).toHaveLength(1) - expect(result.allExtensions[0]).toEqual( + const realExtensions = result.allExtensions.filter((ext) => ext.specification.experience !== 'configuration') + expect(realExtensions).toHaveLength(1) + expect(realExtensions[0]).toEqual( expect.objectContaining({ configuration: expect.objectContaining({ name: 'test-extension', diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index 61d4e9c923b..d480c8162d1 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -72,11 +72,10 @@ export async function linkedAppContext({ let configState = await getAppConfigurationState(directory, userProvidedConfigName) let remoteApp: OrganizationApp | undefined - // If the app is not linked, force a link. - if (configState.state === 'template-only' || forceRelink) { + // If forceRelink is true, force a link. + if (forceRelink) { // If forceRelink is true, we don't want to use the cached config name and instead prompt the user for a new one. - const configName = forceRelink ? undefined : configState.configurationFileName - const result = await link({directory, apiKey: clientId, configName}) + const result = await link({directory, apiKey: clientId, configName: undefined}) remoteApp = result.remoteApp configState = result.state } diff --git a/packages/app/src/cli/services/app/config/link-service.test.ts b/packages/app/src/cli/services/app/config/link-service.test.ts index 361061d4233..408bf450757 100644 --- a/packages/app/src/cli/services/app/config/link-service.test.ts +++ b/packages/app/src/cli/services/app/config/link-service.test.ts @@ -3,18 +3,23 @@ import {testOrganizationApp, testDeveloperPlatformClient} from '../../../models/ import {DeveloperPlatformClient, selectDeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' import {OrganizationApp, OrganizationSource} from '../../../models/organization.js' import {appNamePrompt, createAsNewAppPrompt, selectOrganizationPrompt} from '../../../prompts/dev.js' +import {selectConfigName} from '../../../prompts/config.js' import {beforeEach, describe, expect, test, vi} from 'vitest' import {inTemporaryDirectory, readFile, writeFileSync} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' vi.mock('./use.js') vi.mock('../../../prompts/dev.js') +vi.mock('../../../prompts/config.js') vi.mock('../../local-storage') vi.mock('@shopify/cli-kit/node/ui') vi.mock('../../dev/fetch.js') vi.mock('../../../utilities/developer-platform-client.js') vi.mock('../../../models/app/validation/multi-cli-warning.js') -beforeEach(async () => {}) +beforeEach(async () => { + // Default mock for selectConfigName - tests that need a specific value can override + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') +}) function buildDeveloperPlatformClient(): DeveloperPlatformClient { return testDeveloperPlatformClient({ @@ -54,7 +59,19 @@ describe('link, with minimal mocking', () => { test('load from a fresh template, return a connected app', async () => { await inTemporaryDirectory(async (tmp) => { const initialContent = ` - scopes='write_something_unusual' +name = "test-app" +client_id = "test-client-id" +application_url = "https://example.com" +embedded = true + +[access_scopes] +scopes = "write_something_unusual" + +[auth] +redirect_urls = ["https://example.com/callback"] + +[webhooks] +api_version = "2024-01" ` const filePath = joinPath(tmp, 'shopify.app.toml') writeFileSync(filePath, initialContent) @@ -69,6 +86,7 @@ describe('link, with minimal mocking', () => { businessName: 'test', source: OrganizationSource.BusinessPlatform, }) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') const options = { directory: tmp, 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 e0d9ccaa684..85c4d0861f6 100644 --- a/packages/app/src/cli/services/app/config/link.test.ts +++ b/packages/app/src/cli/services/app/config/link.test.ts @@ -64,6 +64,8 @@ function buildDeveloperPlatformClient(extraFields: Partial { vi.mocked(fetchAppRemoteConfiguration).mockResolvedValue(DEFAULT_REMOTE_CONFIGURATION) + // Default mock for selectConfigName - tests that need a specific value can override + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') }) describe('link', () => { @@ -88,10 +90,10 @@ describe('link', () => { expect(configuration).toEqual({ client_id: '12345', name: 'app1', - extension_directories: [], application_url: 'https://example.com', embedded: true, access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -104,15 +106,16 @@ describe('link', () => { embedded: false, }, path: expect.stringMatching(/\/shopify.app.default-value.toml$/), + build: undefined, }) expect(state).toEqual({ - state: 'connected-app', basicConfiguration: configuration, appDirectory: options.directory, configurationPath: expect.stringMatching(/\/shopify.app.default-value.toml$/), configSource: 'flag', configurationFileName: 'shopify.app.default-value.toml', + isTemplateForm: false, }) expect(remoteApp).toEqual(mockRemoteApp({developerPlatformClient})) @@ -145,13 +148,13 @@ describe('link', () => { const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -166,10 +169,10 @@ embedded = false expect(configuration).toEqual({ client_id: '12345', name: 'app1', - extension_directories: [], application_url: 'https://example.com', embedded: true, access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -182,6 +185,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.staging.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -295,12 +299,12 @@ embedded = false }) expect(content).toEqual(expectedContent) expect(state).toEqual({ - state: 'connected-app', basicConfiguration: configuration, appDirectory: options.directory, configurationPath: expect.stringMatching(/\/shopify.app.toml$/), configSource: 'cached', configurationFileName: 'shopify.app.toml', + isTemplateForm: false, }) }) }) @@ -672,13 +676,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -708,11 +712,11 @@ embedded = false }) expect(configuration).toEqual({ client_id: '12345', - extension_directories: [], name: 'app1', application_url: 'https://example.com', embedded: true, access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -725,6 +729,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -753,13 +758,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -777,8 +782,8 @@ embedded = false name: 'app1', application_url: 'https://example.com', embedded: true, - extension_directories: [], access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -791,6 +796,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -806,7 +812,6 @@ embedded = false developerPlatformClient, } await mockLoadOpaqueAppWithApp(tmp) - vi.mocked(selectConfigName).mockResolvedValue('shopify.app.staging.toml') vi.mocked(appFromIdentifiers).mockImplementation(async ({apiKey}: {apiKey: string}) => { return (await developerPlatformClient.appFromIdentifiers(apiKey))! }) @@ -819,7 +824,6 @@ embedded = false expect(content).toContain('name = "app1"') expect(configuration).toEqual({ client_id: 'api-key', - extension_directories: [], name: 'app1', application_url: 'https://example.com', embedded: true, @@ -836,6 +840,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) }) }) @@ -1032,7 +1037,6 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true @@ -1040,6 +1044,7 @@ embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = "read_products,write_orders" +use_legacy_install_flow = true [auth] redirect_urls = [ "https://example.com/callback1" ] @@ -1053,12 +1058,12 @@ embedded = false expect(configuration).toEqual({ client_id: '12345', - extension_directories: [], name: 'app1', application_url: 'https://example.com', embedded: true, access_scopes: { scopes: 'read_products,write_orders', + use_legacy_install_flow: true, }, auth: { redirect_urls: ['https://example.com/callback1'], @@ -1070,6 +1075,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -1276,13 +1282,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://my-app-url.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1304,8 +1310,8 @@ embedded = false name: 'app1', application_url: 'https://my-app-url.com', embedded: true, - extension_directories: [], access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, build: undefined, @@ -1448,7 +1454,6 @@ embedded = true const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true @@ -1458,6 +1463,7 @@ include_config_on_deploy = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1500,7 +1506,6 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true @@ -1511,6 +1516,7 @@ include_config_on_deploy = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1552,13 +1558,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1598,7 +1604,6 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" handle = "handle" application_url = "https://example.com" @@ -1606,6 +1611,7 @@ embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1867,7 +1873,7 @@ async function mockApp( ) { const versionSchema = await buildVersionedAppSchema() const localApp = testApp(app) - localApp.configuration.client_id = schemaType === 'legacy' ? 12345 : '12345' + localApp.configuration.client_id = '12345' localApp.configSchema = versionSchema.schema localApp.specifications = versionSchema.configSpecifications localApp.directory = directory diff --git a/packages/app/src/cli/services/app/config/link.ts b/packages/app/src/cli/services/app/config/link.ts index 12997e2d39e..b7d56403458 100644 --- a/packages/app/src/cli/services/app/config/link.ts +++ b/packages/app/src/cli/services/app/config/link.ts @@ -3,7 +3,6 @@ import { AppConfiguration, CurrentAppConfiguration, getAppVersionedSchema, - isCurrentAppSchema, CliBuildPreferences, getAppScopes, } from '../../../models/app/app.js' @@ -23,7 +22,6 @@ import { InvalidApiKeyErrorMessage, } from '../../context.js' import {Flag, DeveloperPlatformClient, CreateAppOptions} from '../../../utilities/developer-platform-client.js' -import {configurationFileNames} from '../../../constants.js' import {writeAppConfigurationFile} from '../write-app-configuration-file.js' import {getCachedCommandInfo} from '../../local-storage.js' import {RemoteAwareExtensionSpecification} from '../../../models/extensions/specification.js' @@ -99,12 +97,12 @@ export default async function link(options: LinkOptions, shouldRenderSuccess = t } const state: AppConfigurationStateLinked = { - state: 'connected-app', basicConfiguration: mergedAppConfiguration, appDirectory, configurationPath: joinPath(appDirectory, configFileName), configSource: options.configName ? 'flag' : 'cached', configurationFileName: configFileName, + isTemplateForm: mergedAppConfiguration.client_id === '', } return {configuration: mergedAppConfiguration, remoteApp, state} @@ -195,16 +193,6 @@ interface ExistingConfig { } type LocalAppOptions = - | { - state: 'legacy' - configFormat: 'legacy' - scopes: string - localAppIdMatchedRemote: false - existingBuildOptions: undefined - existingConfig: ExistingConfig - appDirectory: string - packageManager: PackageManager - } | { state: 'reusable-current-app' configFormat: 'current' @@ -229,7 +217,7 @@ type LocalAppOptions = } | { state: 'unable-to-load-config' - configFormat: 'legacy' + configFormat: 'current' localAppIdMatchedRemote: false } )) @@ -265,18 +253,7 @@ export async function loadLocalAppOptions( case 'loaded-app': { const {app, configuration} = result - if (!isCurrentAppSchema(configuration)) { - return { - state: 'legacy', - configFormat: 'legacy', - scopes: getAppScopes(configuration), - localAppIdMatchedRemote: false, - existingBuildOptions: undefined, - existingConfig: configuration, - appDirectory: app.directory, - packageManager: app.packageManager, - } - } else if (configuration.client_id === remoteAppApiKey || options.isNewApp) { + if (configuration.client_id === remoteAppApiKey || options.isNewApp) { return { state: 'reusable-current-app', configFormat: 'current', @@ -303,10 +280,10 @@ export async function loadLocalAppOptions( case 'loaded-template': // Template with extra config keys (metafields, etc.) - treat as legacy for linking return { - state: 'legacy', - configFormat: 'legacy', + state: 'reusable-current-app', + configFormat: 'current', scopes: result.scopes, - localAppIdMatchedRemote: false, + localAppIdMatchedRemote: true, existingBuildOptions: undefined, existingConfig: result.rawConfig, appDirectory: result.appDirectory, @@ -316,7 +293,7 @@ export async function loadLocalAppOptions( case 'error': return { state: 'unable-to-load-config', - configFormat: 'legacy', + configFormat: 'current', scopes: '', localAppIdMatchedRemote: false, appDirectory: undefined, @@ -339,7 +316,7 @@ async function loadConfigurationFileName( options: LinkOptions, localAppInfo: { appDirectory?: string - format: 'legacy' | 'current' + format: 'current' }, ): Promise { // config name from the options takes precedence over everything else @@ -351,10 +328,6 @@ async function loadConfigurationFileName( const cache = getCachedCommandInfo() if (cache?.selectedToml) return cache.selectedToml as AppConfigurationFileName - if (localAppInfo.format === 'legacy') { - return configurationFileNames.app - } - const existingTomls = await getTomls(options.directory) const currentToml = existingTomls[remoteApp.apiKey] if (currentToml) return currentToml diff --git a/packages/app/src/cli/services/app/config/pull.ts b/packages/app/src/cli/services/app/config/pull.ts index e7ceefdbb03..65ad966db6e 100644 --- a/packages/app/src/cli/services/app/config/pull.ts +++ b/packages/app/src/cli/services/app/config/pull.ts @@ -1,7 +1,7 @@ // packages/app/src/cli/services/app/config/pull.ts import {LinkOptions, loadLocalAppOptions, overwriteLocalConfigFileWithRemoteAppConfiguration} from './link.js' -import {CurrentAppConfiguration, isCurrentAppSchema} from '../../../models/app/app.js' +import {CurrentAppConfiguration} from '../../../models/app/app.js' import {OrganizationApp} from '../../../models/organization.js' import {AppConfigurationFileName} from '../../../models/app/loader.js' import {fetchSpecifications} from '../../generate/fetch-extension-specifications.js' @@ -28,7 +28,7 @@ interface PullOutput { export default async function pull(options: PullOptions): Promise { const {directory, configName, configuration, remoteApp} = options - if (!isCurrentAppSchema(configuration) || !configuration.client_id) { + if (!configuration.client_id) { throw new AbortError( 'The selected configuration is not linked to a remote app.', 'Run `shopify app config link` first to link this configuration to a Shopify app.', 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 6f8418c59b0..bf20c69aea6 100644 --- a/packages/app/src/cli/services/app/config/use.test.ts +++ b/packages/app/src/cli/services/app/config/use.test.ts @@ -79,9 +79,11 @@ describe('use', () => { 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, - configuration: appWithoutClientID.configuration, + configuration: configWithoutClientId as any, configSchema, specifications: [], remoteFlags: [], diff --git a/packages/app/src/cli/services/app/config/use.ts b/packages/app/src/cli/services/app/config/use.ts index d99123cf58b..fa0a2ffee55 100644 --- a/packages/app/src/cli/services/app/config/use.ts +++ b/packages/app/src/cli/services/app/config/use.ts @@ -1,7 +1,7 @@ import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models/app/loader.js' import {clearCurrentConfigFile, setCachedAppInfo} from '../../local-storage.js' import {selectConfigFile} from '../../../prompts/config.js' -import {AppConfiguration, CurrentAppConfiguration, isCurrentAppSchema} from '../../../models/app/app.js' +import {AppConfiguration, CurrentAppConfiguration} from '../../../models/app/app.js' import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' import {AbortError} from '@shopify/cli-kit/node/error' import {fileExists} from '@shopify/cli-kit/node/fs' @@ -76,7 +76,7 @@ export function setCurrentConfigPreference( }, ): asserts configuration is CurrentAppConfiguration { const {configFileName, directory} = options - if (isCurrentAppSchema(configuration) && configuration.client_id) { + if (configuration.client_id) { setCachedAppInfo({ directory, configFile: configFileName, diff --git a/packages/app/src/cli/services/app/env/pull.test.ts b/packages/app/src/cli/services/app/env/pull.test.ts index c16c008aa63..8877c2c2bfa 100644 --- a/packages/app/src/cli/services/app/env/pull.test.ts +++ b/packages/app/src/cli/services/app/env/pull.test.ts @@ -1,5 +1,5 @@ import {pullEnv} from './pull.js' -import {AppInterface, AppLinkedInterface} from '../../../models/app/app.js' +import {AppInterface, AppLinkedInterface, AppConfiguration} from '../../../models/app/app.js' import {testApp, testOrganizationApp} from '../../../models/app/app.test-data.js' import {Organization, OrganizationApp, OrganizationSource} from '../../../models/organization.js' import {describe, expect, vi, beforeEach, test} from 'vitest' @@ -108,9 +108,12 @@ function mockApp(currentVersion = '2.2.2'): AppInterface { directory: '/', configuration: { path: joinPath('/', 'shopify.app.toml'), - scopes: 'my-scope', + client_id: 'test-client-id', + access_scopes: { + scopes: 'my-scope', + }, extension_directories: ['extensions/*'], - }, + } as AppConfiguration, nodeDependencies, }) } diff --git a/packages/app/src/cli/services/app/env/show.test.ts b/packages/app/src/cli/services/app/env/show.test.ts index e434cda300d..444f7c2f8a1 100644 --- a/packages/app/src/cli/services/app/env/show.test.ts +++ b/packages/app/src/cli/services/app/env/show.test.ts @@ -1,6 +1,6 @@ import {showEnv} from './show.js' import {fetchOrganizations} from '../../dev/fetch.js' -import {AppInterface} from '../../../models/app/app.js' +import {AppInterface, AppConfiguration} from '../../../models/app/app.js' import {selectOrganizationPrompt} from '../../../prompts/dev.js' import {testApp, testOrganizationApp} from '../../../models/app/app.test-data.js' import {OrganizationSource} from '../../../models/organization.js' @@ -54,8 +54,11 @@ function mockApp(currentVersion = '2.2.2'): AppInterface { directory: '/', configuration: { path: joinPath('/', 'shopify.app.toml'), - scopes: 'my-scope', - }, + client_id: 'test-client-id', + access_scopes: { + scopes: 'my-scope', + }, + } as AppConfiguration, nodeDependencies, }) } diff --git a/packages/app/src/cli/services/app/write-app-configuration-file.test.ts b/packages/app/src/cli/services/app/write-app-configuration-file.test.ts index 2506dc3cbd8..22217b443dd 100644 --- a/packages/app/src/cli/services/app/write-app-configuration-file.test.ts +++ b/packages/app/src/cli/services/app/write-app-configuration-file.test.ts @@ -67,6 +67,7 @@ include_config_on_deploy = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = "read_products" +use_legacy_install_flow = true [auth] redirect_urls = [ diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 2a0a98d0c09..f026a5a61d6 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -78,7 +78,6 @@ const STORE1: OrganizationStore = { } const state: AppConfigurationStateLinked = { - state: 'connected-app', basicConfiguration: { ...DEFAULT_CONFIG, path: 'shopify.app.toml', @@ -88,6 +87,7 @@ const state: AppConfigurationStateLinked = { configurationPath: 'shopify.app.toml', configSource: 'flag', configurationFileName: 'shopify.app.toml', + isTemplateForm: false, } const deployOptions = (app: AppLinkedInterface, reset = false, force = false): DeployOptions => { diff --git a/packages/app/src/cli/services/context/identifiers-extensions.test.ts b/packages/app/src/cli/services/context/identifiers-extensions.test.ts index cc9a2adb30f..754ae0a7fbb 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.test.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.test.ts @@ -9,7 +9,7 @@ import { import {extensionMigrationPrompt, matchConfirmationPrompt} from './prompts.js' import {manualMatchIds} from './id-manual-matching.js' import {EnsureDeploymentIdsPresenceOptions, LocalSource, RemoteSource} from './identifiers.js' -import {AppInterface} from '../../models/app/app.js' +import {AppInterface, AppConfiguration} from '../../models/app/app.js' import { testApp, testAppConfigExtensions, @@ -108,10 +108,13 @@ const LOCAL_APP = ( directory: '/app', configuration: { path: '/shopify.app.toml', - scopes: 'read_products', + client_id: 'test-client-id', + access_scopes: { + scopes: 'read_products', + }, extension_directories: ['extensions/*'], ...(includeDeployConfig ? {build: {include_config_on_deploy: true}} : {}), - }, + } as AppConfiguration, allExtensions: [...uiExtensions, ...functionExtensions, ...configExtensions], }) } @@ -935,7 +938,8 @@ describe('deployConfirmed: handle non existent uuid managed extensions', () => { // When const CONFIG_A = await testAppConfigExtensions() - const ensureExtensionsIdsOptions = options([], [], {configExtensions: [CONFIG_A], developerPlatformClient}) + // Don't pass config extensions when includeConfigOnDeploy is false - they won't be in allExtensions + const ensureExtensionsIdsOptions = options([], [], {developerPlatformClient}) const got = await deployConfirmed(ensureExtensionsIdsOptions, [], [], { extensionsToCreate, validMatches, diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index af0af066266..d687d05301c 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -29,7 +29,7 @@ import {DevSessionStatusManager} from './dev/processes/dev-session/dev-session-s import {TunnelMode} from './dev/tunnel-mode.js' import {PortDetail, renderPortWarnings} from './dev/port-warnings.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' -import {Web, isCurrentAppSchema, getAppScopesArray, AppLinkedInterface} from '../models/app/app.js' +import {Web, getAppScopesArray, AppLinkedInterface} from '../models/app/app.js' import {Organization, OrganizationApp, OrganizationStore} from '../models/organization.js' import {getAnalyticsTunnelType} from '../utilities/analytics.js' import metadata from '../metadata.js' @@ -199,42 +199,40 @@ export async function warnIfScopesDifferBeforeDev({ developerPlatformClient, }: Pick) { if (developerPlatformClient.supportsDevSessions) return - if (isCurrentAppSchema(localApp.configuration)) { - const localAccess = localApp.configuration.access_scopes - const remoteAccess = remoteApp.configuration?.access_scopes - - const rationaliseScopes = (scopeString: string | undefined) => { - if (!scopeString) return scopeString - return scopeString - .split(',') - .map((scope) => scope.trim()) - .sort() - .join(',') - } - const localScopes = rationaliseScopes(localAccess?.scopes) - const remoteScopes = rationaliseScopes(remoteAccess?.scopes) - - if (!localAccess?.use_legacy_install_flow && localScopes !== remoteScopes) { - const nextSteps = [ - [ - 'Run', - {command: formatPackageManagerCommand(localApp.packageManager, 'shopify app deploy')}, - 'to push your scopes to the Partner Dashboard', - ], - ] - - renderWarning({ - headline: [`The scopes in your TOML don't match the scopes in your Partner Dashboard`], - body: [ - `Scopes in ${basename(localApp.configuration.path)}:`, - scopesMessage(getAppScopesArray(localApp.configuration)), - '\n', - 'Scopes in Partner Dashboard:', - scopesMessage(remoteAccess?.scopes?.split(',') ?? []), - ], - nextSteps, - }) - } + const localAccess = localApp.configuration.access_scopes + const remoteAccess = remoteApp.configuration?.access_scopes + + const rationaliseScopes = (scopeString: string | undefined) => { + if (!scopeString) return scopeString + return scopeString + .split(',') + .map((scope) => scope.trim()) + .sort() + .join(',') + } + const localScopes = rationaliseScopes(localAccess?.scopes) + const remoteScopes = rationaliseScopes(remoteAccess?.scopes) + + if (!localAccess?.use_legacy_install_flow && localScopes !== remoteScopes) { + const nextSteps = [ + [ + 'Run', + {command: formatPackageManagerCommand(localApp.packageManager, 'shopify app deploy')}, + 'to push your scopes to the Partner Dashboard', + ], + ] + + renderWarning({ + headline: [`The scopes in your TOML don't match the scopes in your Partner Dashboard`], + body: [ + `Scopes in ${basename(localApp.configuration.path)}:`, + scopesMessage(getAppScopesArray(localApp.configuration)), + '\n', + 'Scopes in Partner Dashboard:', + scopesMessage(remoteAccess?.scopes?.split(',') ?? []), + ], + nextSteps, + }) } } diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts index 7de43c69b3f..f1b7192882d 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts @@ -11,7 +11,7 @@ import { } from '../../../models/app/app.test-data.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {loadApp, reloadApp} from '../../../models/app/loader.js' -import {AppLinkedInterface} from '../../../models/app/app.js' +import {AppLinkedInterface, AppConfiguration} from '../../../models/app/app.js' import {AppAccessSpecIdentifier} from '../../../models/extensions/specifications/app_config_app_access.js' import {PosSpecIdentifier} from '../../../models/extensions/specifications/app_config_point_of_sale.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' @@ -275,7 +275,12 @@ describe('app-event-watcher', () => { // When const app = testAppLinked({ allExtensions: initialExtensions, - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) const mockManager = new MockESBuildContextManager() @@ -346,7 +351,7 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: {client_id: 'test-client-id', extension_directories: [], path: 'shopify.app.custom.toml'}, }) const generateTypesSpy = vi.spyOn(app, 'generateExtensionTypes') @@ -383,7 +388,7 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: {client_id: 'test-client-id', extension_directories: [], path: 'shopify.app.custom.toml'}, }) const mockManager = new MockESBuildContextManager() @@ -419,7 +424,7 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1, posExtension], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: {client_id: 'test-client-id', extension_directories: [], path: 'shopify.app.custom.toml'}, }) const mockManager = new MockESBuildContextManager() @@ -451,7 +456,7 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1, extension2], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: {client_id: 'test-client-id', extension_directories: [], path: 'shopify.app.custom.toml'}, }) const generateTypesSpy = vi.spyOn(app, 'generateExtensionTypes') @@ -499,7 +504,12 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent]) @@ -542,7 +552,12 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [flowExtension], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) // When @@ -576,7 +591,12 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent]) diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts index 5704a97ed6a..01d15deb10c 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts @@ -7,6 +7,7 @@ import { testFunctionExtension, testUIExtension, } from '../../../models/app/app.test-data.js' +import {AppConfiguration} from '../../../models/app/app.js' import {flushPromises} from '@shopify/cli-kit/node/promises' import {describe, expect, test, vi} from 'vitest' import chokidar from 'chokidar' @@ -232,7 +233,11 @@ describe('file-watcher events', () => { const app = testAppLinked({ allExtensions: [ext1, ext2, posExtension], directory: dir, - configuration: {path: joinPath(dir, '/shopify.app.toml'), scopes: ''}, + configuration: { + path: joinPath(dir, '/shopify.app.toml'), + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + } as AppConfiguration, }) // Add a custom gitignore file to the extension diff --git a/packages/app/src/cli/services/dev/select-app.test.ts b/packages/app/src/cli/services/dev/select-app.test.ts index d0ce4811d3b..0e72c297e9d 100644 --- a/packages/app/src/cli/services/dev/select-app.test.ts +++ b/packages/app/src/cli/services/dev/select-app.test.ts @@ -1,5 +1,5 @@ import {selectOrCreateApp} from './select-app.js' -import {AppInterface, WebType} from '../../models/app/app.js' +import {AppInterface, AppConfiguration, WebType} from '../../models/app/app.js' import {Organization, OrganizationSource} from '../../models/organization.js' import {appNamePrompt, createAsNewAppPrompt, selectAppPrompt} from '../../prompts/dev.js' import {testApp, testOrganizationApp, testDeveloperPlatformClient} from '../../models/app/app.test-data.js' @@ -9,7 +9,12 @@ vi.mock('../../prompts/dev') const LOCAL_APP: AppInterface = testApp({ directory: '', - configuration: {path: '/shopify.app.toml', scopes: 'read_products', extension_directories: ['extensions/*']}, + configuration: { + path: '/shopify.app.toml', + client_id: 'test-client-id', + access_scopes: {scopes: 'read_products'}, + extension_directories: ['extensions/*'], + } as AppConfiguration, webs: [ { directory: '', diff --git a/packages/app/src/cli/services/generate/extension.test.ts b/packages/app/src/cli/services/generate/extension.test.ts index 38becb99547..15cfa1e2e67 100644 --- a/packages/app/src/cli/services/generate/extension.test.ts +++ b/packages/app/src/cli/services/generate/extension.test.ts @@ -116,7 +116,8 @@ describe('initialize a extension', async () => { expect(vi.mocked(addNPMDependenciesIfNeeded)).toHaveBeenCalledTimes(2) const loadedApp = await loadApp({directory: tmpDir, specifications, userProvidedConfigName: undefined}) - expect(loadedApp.allExtensions.length).toEqual(2) + const realExtensions = loadedApp.allExtensions.filter((ext) => ext.specification.experience !== 'configuration') + expect(realExtensions.length).toEqual(2) }) }) @@ -580,7 +581,18 @@ async function withTemporaryApp( const webConfigurationPath = joinPath(tmpDir, blocks.web.directoryName, blocks.web.configurationName) const appConfiguration = ` name = "my_app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [access_scopes] scopes = "read_products" + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` const webConfiguration = ` type = "backend" diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index fee68b37094..223bd138e77 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -1,5 +1,5 @@ import {InfoOptions, info} from './info.js' -import {AppInterface, AppLinkedInterface} from '../models/app/app.js' +import {AppInterface, AppLinkedInterface, AppConfiguration} from '../models/app/app.js' import {OrganizationApp, OrganizationSource} from '../models/organization.js' import {selectOrganizationPrompt} from '../prompts/dev.js' import { @@ -274,9 +274,12 @@ function mockApp({ directory, configuration: { path: joinPath(directory, 'shopify.app.toml'), - scopes: 'my-scope', + client_id: 'test-client-id', + access_scopes: { + scopes: 'my-scope', + }, extension_directories: ['extensions/*'], - }, + } as AppConfiguration, nodeDependencies, ...(app ? app : {}), }) diff --git a/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts index b75607313a4..107932cc45e 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts @@ -1,11 +1,11 @@ import {PartnersClient} from './partners-client.js' import {CreateAppQuery} from '../../api/graphql/create_app.js' -import {AppInterface, WebType} from '../../models/app/app.js' +import {AppInterface, AppConfiguration, WebType} from '../../models/app/app.js' import {Organization, OrganizationSource, OrganizationStore} from '../../models/organization.js' import { testPartnersUserSession, testApp, - testAppWithLegacyConfig, + testAppWithConfig, testOrganizationApp, } from '../../models/app/app.test-data.js' import {appNamePrompt} from '../../prompts/dev.js' @@ -23,7 +23,12 @@ beforeEach(() => { const LOCAL_APP: AppInterface = testApp({ directory: '', - configuration: {path: '/shopify.app.toml', scopes: 'read_products', extension_directories: ['extensions/*']}, + configuration: { + path: '/shopify.app.toml', + client_id: 'test-client-id', + access_scopes: {scopes: 'read_products'}, + extension_directories: ['extensions/*'], + } as AppConfiguration, webs: [ { directory: '', @@ -82,7 +87,7 @@ describe('createApp', () => { test('sends request to create app with launchable defaults and returns it', async () => { // Given const partnersClient = PartnersClient.getInstance(testPartnersUserSession) - const localApp = testAppWithLegacyConfig({config: {scopes: 'write_products'}}) + const localApp = testAppWithConfig({config: {access_scopes: {scopes: 'write_products'}}}) vi.mocked(appNamePrompt).mockResolvedValue('app-name') vi.mocked(partnersRequest).mockResolvedValueOnce({appCreate: {app: APP1, userErrors: []}}) const variables = { diff --git a/packages/app/src/cli/utilities/types.ts b/packages/app/src/cli/utilities/types.ts deleted file mode 100644 index 37fd9ac655f..00000000000 --- a/packages/app/src/cli/utilities/types.ts +++ /dev/null @@ -1,5 +0,0 @@ -import {zod} from '@shopify/cli-kit/node/schema' - -export function isType(schema: T, item: unknown): item is zod.infer { - return schema.safeParse(item).success -} diff --git a/packages/features/steps/create-app.steps.ts b/packages/features/steps/create-app.steps.ts index 8b17a44a619..8712ae382a2 100644 --- a/packages/features/steps/create-app.steps.ts +++ b/packages/features/steps/create-app.steps.ts @@ -12,40 +12,6 @@ interface ExtensionConfiguration { outputPath: string } -/** - * Ensures that a minimal shopify.app.toml file exists in the app directory - */ -async function ensureAppToml(appDirectory: string) { - const tomlPath = path.join(appDirectory, 'shopify.app.toml') - if (!fs.existsSync(tomlPath)) { - // Create a minimal shopify.app.toml file with required fields - const minimalToml = ` -name = "MyExtendedApp" -scopes = "write_products" - `.trim() - await fs.writeFile(tomlPath, minimalToml) - } -} - -/** - * Ensures that a package.json file exists in the app directory - */ -async function ensurePackageJson(appDirectory: string) { - const packageJsonPath = path.join(appDirectory, 'package.json') - if (!fs.existsSync(packageJsonPath)) { - // Create a basic package.json file - const packageJson = { - name: 'my-extended-app', - version: '1.0.0', - description: 'A Shopify app', - main: 'index.js', - dependencies: {}, - packageManager: 'npm', - } - await fs.writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2)) - } -} - When( /I create a (.+) app named (.+) with (.+) as package manager/, {timeout: 5 * 60 * 1000}, @@ -85,12 +51,6 @@ When( await fs.ensureFile(npmrcPath) // we need to disable this on CI otherwise pnpm will crash complaining that there is no lockfile await fs.appendFile(npmrcPath, 'frozen-lockfile=false\n') - - // Ensure shopify.app.toml exists in the app directory - await ensureAppToml(this.appDirectory) - - // Ensure package.json exists in the app directory - await ensurePackageJson(this.appDirectory) }, )