Skip to content

Commit cba504e

Browse files
authored
Merge pull request #7005 from Shopify/fix_uid_strategy_matching
fix: infer uidStrategy from backend typename
2 parents bdd4def + 9371afa commit cba504e

18 files changed

Lines changed: 121 additions & 29 deletions

packages/app/src/cli/api/graphql/app-management/generated/specifications.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ export type FetchSpecificationsQuery = {
1414
externalIdentifier: string
1515
features: string[]
1616
uidStrategy:
17-
| {appModuleLimit: number; isClientProvided: boolean}
18-
| {appModuleLimit: number; isClientProvided: boolean}
19-
| {appModuleLimit: number; isClientProvided: boolean}
17+
| {__typename: 'UidStrategiesClientProvided'; appModuleLimit: number; isClientProvided: boolean}
18+
| {__typename: 'UidStrategiesDynamic'; appModuleLimit: number; isClientProvided: boolean}
19+
| {__typename: 'UidStrategiesStatic'; appModuleLimit: number; isClientProvided: boolean}
2020
validationSchema?: {jsonSchema: string} | null
2121
}[]
2222
}
@@ -61,9 +61,9 @@ export const FetchSpecifications = {
6161
selectionSet: {
6262
kind: 'SelectionSet',
6363
selections: [
64+
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
6465
{kind: 'Field', name: {kind: 'Name', value: 'appModuleLimit'}},
6566
{kind: 'Field', name: {kind: 'Name', value: 'isClientProvided'}},
66-
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
6767
],
6868
},
6969
},

packages/app/src/cli/api/graphql/app-management/queries/specifications.graphql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ query fetchSpecifications($organizationId: ID!) {
55
externalIdentifier
66
features
77
uidStrategy {
8+
__typename
89
appModuleLimit
910
isClientProvided
1011
}

packages/app/src/cli/api/graphql/extension_specifications.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {gql} from 'graphql-request'
22

3+
// eslint-disable-next-line @shopify/cli/no-inline-graphql
34
export const ExtensionSpecificationsQuery = gql`
45
query fetchSpecifications($apiKey: String!) {
56
extensionSpecifications(apiKey: $apiKey) {
@@ -40,6 +41,7 @@ export interface RemoteSpecification {
4041
managementExperience: 'cli' | 'custom' | 'dashboard'
4142
registrationLimit: number
4243
uidIsClientProvided: boolean
44+
uidStrategy?: 'single' | 'dynamic' | 'uuid'
4345
}
4446
features?: {
4547
argo?: {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ import {SchemaDefinitionByTargetQueryVariables} from '../../api/graphql/function
7474
import {SchemaDefinitionByApiTypeQueryVariables} from '../../api/graphql/functions/generated/schema-definition-by-api-type.js'
7575
import {AppHomeSpecIdentifier} from '../extensions/specifications/app_config_app_home.js'
7676
import {AppProxySpecIdentifier} from '../extensions/specifications/app_config_app_proxy.js'
77-
import {ExtensionSpecification} from '../extensions/specification.js'
77+
import {ExtensionSpecification, isAppConfigSpecification} from '../extensions/specification.js'
7878
import {AppLogsOptions} from '../../services/app-logs/utils.js'
7979
import {AppLogsSubscribeMutationVariables} from '../../api/graphql/app-management/generated/app-logs-subscribe.js'
8080
import {Session} from '@shopify/cli-kit/node/session'
@@ -1526,5 +1526,5 @@ export async function buildVersionedAppSchema() {
15261526
}
15271527

15281528
export async function configurationSpecifications() {
1529-
return (await loadLocalExtensionsSpecifications()).filter((spec) => spec.uidStrategy === 'single')
1529+
return (await loadLocalExtensionsSpecifications()).filter(isAppConfigSpecification)
15301530
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('load', () => {
6767

6868
// Helper to get only real extensions (not configuration extensions)
6969
function getRealExtensions(app: AppInterface) {
70-
return app.allExtensions.filter((ext) => ext.specification.experience !== 'configuration')
70+
return app.allExtensions.filter((ext) => !ext.isAppConfigExtension)
7171
}
7272

7373
/**

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@ import {configurationFileNames, dotEnvFileNames} from '../../constants.js'
2020
import metadata from '../../metadata.js'
2121
import {ExtensionInstance} from '../extensions/extension-instance.js'
2222
import {ExtensionsArraySchema, UnifiedSchema} from '../extensions/schemas.js'
23-
import {ExtensionSpecification, RemoteAwareExtensionSpecification} from '../extensions/specification.js'
23+
import {
24+
ExtensionSpecification,
25+
RemoteAwareExtensionSpecification,
26+
isAppConfigSpecification,
27+
} from '../extensions/specification.js'
2428
import {getCachedAppInfo} from '../../services/local-storage.js'
2529
import use from '../../services/app/config/use.js'
2630
import {CreateAppOptions, Flag} from '../../utilities/developer-platform-client.js'
@@ -714,7 +718,8 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
714718
const configPath = this.loadedConfiguration.configPath
715719
const extensionInstancesWithKeys = await Promise.all(
716720
this.specifications
717-
.filter((specification) => specification.uidStrategy === 'single')
721+
.filter((specification) => isAppConfigSpecification(specification))
722+
.filter((specification) => specification.identifier !== WebhookSubscriptionSpecIdentifier)
718723
.map(async (specification) => {
719724
const specConfiguration = parseConfigurationObjectAgainstSpecification(
720725
specification,

packages/app/src/cli/models/extensions/extension-instance.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ import {outputDebug} from '@shopify/cli-kit/node/output'
3434
import {extractJSImports, extractImportPathsRecursively} from '@shopify/cli-kit/node/import-extractor'
3535
import {uniq} from '@shopify/cli-kit/common/array'
3636

37+
// DEPRECATED. We should get the experience from the specification instead of hardcoding it based on the identifier.
38+
// This is a temporary solution to avoid breaking changes while we update the API and the specifications query.
3739
export const CONFIG_EXTENSION_IDS: string[] = [
3840
AppAccessSpecIdentifier,
3941
AppHomeSpecIdentifier,
@@ -44,6 +46,10 @@ export const CONFIG_EXTENSION_IDS: string[] = [
4446
WebhookSubscriptionSpecIdentifier,
4547
WebhooksSpecIdentifier,
4648
EventsSpecIdentifier,
49+
50+
// Hardcoded identifiers that don't exist locally.
51+
'data',
52+
'admin',
4753
]
4854

4955
/**
@@ -121,7 +127,7 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
121127
}
122128

123129
get isAppConfigExtension() {
124-
return ['single', 'dynamic'].includes(this.specification.uidStrategy)
130+
return this.specification.experience === 'configuration'
125131
}
126132

127133
get isFlow() {

packages/app/src/cli/models/extensions/specification.integration.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ describe('createContractBasedModuleSpecification', () => {
3333
// When
3434
const got = createContractBasedModuleSpecification({
3535
identifier: 'test',
36+
uidStrategy: 'uuid',
3637
appModuleFeatures: () => ['localization'],
3738
})
3839

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ export interface CustomTransformationConfig {
3232
}
3333

3434
type ExtensionExperience = 'extension' | 'configuration'
35+
36+
export function isAppConfigSpecification(spec: {experience: string}): boolean {
37+
return spec.experience === 'configuration'
38+
}
3539
type UidStrategy = 'single' | 'dynamic' | 'uuid'
3640

3741
export enum AssetIdentifier {
@@ -210,7 +214,13 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
210214
return {
211215
...merged,
212216
contributeToAppConfigurationSchema: (appConfigSchema: ZodSchemaType<unknown>) => {
213-
if (merged.uidStrategy !== 'single') {
217+
const isConfig = isAppConfigSpecification(merged)
218+
const hasSchema = merged.schema._def.shape !== undefined
219+
// This filters out webhook subscription specifications from contributing to the app configuration schema
220+
const hasSingleUidStrategy = merged.uidStrategy === 'single'
221+
222+
const canContribute = isConfig && hasSchema && hasSingleUidStrategy
223+
if (!canContribute) {
214224
// no change
215225
return appConfigSchema
216226
}
@@ -268,13 +278,17 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
268278
}
269279

270280
export function createContractBasedModuleSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(
271-
spec: Pick<CreateExtensionSpecType<TConfiguration>, 'identifier' | 'appModuleFeatures' | 'buildConfig'>,
281+
spec: Pick<
282+
CreateExtensionSpecType<TConfiguration>,
283+
'identifier' | 'appModuleFeatures' | 'buildConfig' | 'uidStrategy'
284+
>,
272285
) {
273286
return createExtensionSpecification({
274287
identifier: spec.identifier,
275288
schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
276289
appModuleFeatures: spec.appModuleFeatures,
277290
buildConfig: spec.buildConfig ?? {mode: 'none'},
291+
uidStrategy: spec.uidStrategy,
278292
deployConfig: async (config, directory) => {
279293
let parsedConfig = configWithoutFirstClassFields(config)
280294
if (spec.appModuleFeatures().includes('localization')) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ describe('localAppContext', () => {
435435
})
436436

437437
// Then
438-
const realExtensions = result.allExtensions.filter((ext) => ext.specification.experience !== 'configuration')
438+
const realExtensions = result.allExtensions.filter((ext) => !ext.isAppConfigExtension)
439439
expect(realExtensions).toHaveLength(1)
440440
expect(realExtensions[0]).toEqual(
441441
expect.objectContaining({

0 commit comments

Comments
 (0)