Skip to content

Remove legacy app schema #6612

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
gonzaloriestra wants to merge 1 commit into
base: main
Choose a base branch
from
+664 −1,009
Draft

Remove legacy app schema #6612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions packages/app/src/cli/commands/app/release.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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({
Expand Down
31 changes: 7 additions & 24 deletions packages/app/src/cli/models/app/app.test-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import {
App,
AppConfiguration,
AppConfigurationSchema,
AppSchema,
AppConfigurationWithoutPath,
AppInterface,
AppLinkedInterface,
CurrentAppConfiguration,
LegacyAppConfiguration,
WebType,
getAppVersionedSchema,
} from './app.js'
Expand Down Expand Up @@ -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<AppInterface> = {}, 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({
Expand All @@ -126,7 +121,7 @@ export function testApp(app: Partial<AppInterface> = {}, 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,
Expand All @@ -150,20 +145,6 @@ interface TestAppWithConfigOptions {
config: object
}

export function testAppWithLegacyConfig({
app = {},
config = {},
}: TestAppWithConfigOptions): AppInterface<LegacyAppConfiguration> {
const configuration: AppConfiguration = {
path: '',
scopes: '',
name: 'name',
extension_directories: [],
...config,
}
return testApp({...app, configuration}) as AppInterface<LegacyAppConfiguration>
}

export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinkedInterface {
const app = testAppLinked(options?.app)
app.configuration = {
Expand Down Expand Up @@ -207,7 +188,9 @@ export function testOrganizationApp(app: Partial<OrganizationApp> = {}): Organiz
return {...defaultApp, ...app}
}

export const placeholderAppConfiguration: AppConfigurationWithoutPath = {scopes: ''}
export const placeholderAppConfiguration: AppConfigurationWithoutPath = {
client_id: '',
}

export async function testUIExtension(
uiExtension: Omit<Partial<ExtensionInstance>, 'configuration'> & {
Expand Down
200 changes: 24 additions & 176 deletions packages/app/src/cli/models/app/app.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
import {
AppSchema,
CurrentAppConfiguration,
LegacyAppConfiguration,
TemplateConfigSchema,
getAppScopes,
getAppScopesArray,
getTemplateScopesArray,
getUIExtensionRendererVersion,
isCurrentAppSchema,
isLegacyAppSchema,
validateExtensionsHandlesInCollection,
validateFunctionExtensionsWithUiHandle,
} from './app.js'
Expand Down Expand Up @@ -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'])
})
})

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 = {
Expand Down
Loading
Loading