From 427d0469f98c16a5361e869a7a649227897ab70f Mon Sep 17 00:00:00 2001 From: Sagarika Dasgupta Date: Thu, 3 Sep 2026 13:57:03 -0500 Subject: [PATCH] Support target-level input query variables on functions Input queries have always been per-target (`input_query` on each `[[targeting]]` entry) while their variables were per-function (`[input.variables]`). shop/world PR 1015868 closes that asymmetry in Core by adding `input_query_variables` to `Execution::FunctionTarget`; this is the CLI half. - Accepts `input_variables` on a `[[targeting]]` entry and forwards it to the deploy payload as `input_query_variables.single_json_metafield`, the same shape already used at the extension level. - Rejects mixing extension-level `[input.variables]` with target-level `input_variables`, reported per offending target so the error points at the line to change rather than at the top of the TOML. The suggested remedy is always to move to target-level variables, since the extension-level field is being deprecated. --- .../function-target-level-input-variables.md | 5 + .../specifications/function.test.ts | 120 +++++++++++++++++- .../extensions/specifications/function.ts | 50 ++++++-- 3 files changed, 163 insertions(+), 12 deletions(-) create mode 100644 .changeset/function-target-level-input-variables.md diff --git a/.changeset/function-target-level-input-variables.md b/.changeset/function-target-level-input-variables.md new file mode 100644 index 00000000000..fa9f3ef9c50 --- /dev/null +++ b/.changeset/function-target-level-input-variables.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': minor +--- + +Support `input_variables` on function targets, alongside the extension-level `[input.variables]` diff --git a/packages/app/src/cli/models/extensions/specifications/function.test.ts b/packages/app/src/cli/models/extensions/specifications/function.test.ts index 341cff94c8e..f26e74558b5 100644 --- a/packages/app/src/cli/models/extensions/specifications/function.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/function.test.ts @@ -1,4 +1,4 @@ -import {FunctionConfigType} from './function.js' +import {FunctionConfigType, FunctionExtensionSchema} from './function.js' import {placeholderAppConfiguration, testFunctionExtension} from '../../app/app.test-data.js' import {ExtensionInstance} from '../extension-instance.js' import {inTemporaryDirectory, mkdir, touchFile, writeFile} from '@shopify/cli-kit/node/fs' @@ -6,6 +6,7 @@ import {joinPath} from '@shopify/cli-kit/node/path' import {AbortError} from '@shopify/cli-kit/node/error' import {beforeEach, describe, expect, test} from 'vitest' import {getPathValue} from '@shopify/cli-kit/common/object' +import {zod} from '@shopify/cli-kit/node/schema' describe('functionConfiguration', () => { let extension: ExtensionInstance @@ -144,6 +145,36 @@ describe('functionConfiguration', () => { }) }) + test('maps target-level input variables to the deploy payload', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + extension.directory = tmpDir + extension.configuration.input = undefined + extension.configuration.targeting = [ + {target: 'some.api.target1', input_variables: {namespace: 'target-namespace', key: 'target-key'}}, + {target: 'some.api.target2', export: 'run_target2'}, + ] + + // When + const got = await extension.deployConfig({ + apiKey, + appConfiguration: placeholderAppConfiguration, + }) + + // Then + expect(getPathValue(got!, 'targets')).toEqual([ + { + handle: 'some.api.target1', + input_query_variables: { + single_json_metafield: {namespace: 'target-namespace', key: 'target-key'}, + }, + }, + {handle: 'some.api.target2', export: 'run_target2'}, + ]) + expect(getPathValue(got!, 'input_query_variables')).toBeUndefined() + }) + }) + test('aborts when an target input query file is missing', async () => { // Given extension.configuration.targeting = [{target: 'some.api.target1', input_query: 'this-is-not-a-file.graphql'}] @@ -357,3 +388,90 @@ describe('functionConfiguration', () => { expect(extension.outputPath).toBe(joinPath('/function', 'dist', 'index.wasm')) }) }) + +describe('input variables placement', () => { + const baseConfig = {name: 'function', type: 'function', api_version: '2022-07'} + const extensionVariables = {namespace: 'namespace', key: 'key'} + const targetVariables = {namespace: 'target-namespace', key: 'target-key'} + + const parseIssues = (config: object): zod.ZodIssue[] => { + const result = FunctionExtensionSchema.safeParse(config) + return result.success ? [] : result.error.issues + } + + test('rejects a target declaring input variables when the extension declares them too', () => { + // Given + const config = { + ...baseConfig, + input: {variables: extensionVariables}, + targeting: [{target: 'some.api.target1', input_variables: targetVariables}], + } + + // When + const got = parseIssues(config) + + // Then + expect(got).toEqual([ + expect.objectContaining({ + path: ['targeting', 0, 'input_variables'], + message: + 'Input variables must be defined either at the extension level or on a target, not both. ' + + 'Remove `[input.variables]` from your extension configuration and declare `input_variables` on each target that needs them.', + }), + ]) + }) + + test('reports every offending target and leaves the others alone', () => { + // Given + const config = { + ...baseConfig, + input: {variables: extensionVariables}, + targeting: [ + {target: 'some.api.target1', input_variables: targetVariables}, + {target: 'some.api.target2', export: 'run_target2'}, + {target: 'some.api.target3', input_variables: targetVariables}, + ], + } + + // When + const got = parseIssues(config) + + // Then + expect(got.map((issue) => issue.path)).toEqual([ + ['targeting', 0, 'input_variables'], + ['targeting', 2, 'input_variables'], + ]) + }) + + test('accepts input variables declared only on targets', () => { + // Given + const config = { + ...baseConfig, + targeting: [ + {target: 'some.api.target1', input_variables: targetVariables}, + {target: 'some.api.target2', export: 'run_target2'}, + ], + } + + // When + const got = parseIssues(config) + + // Then + expect(got).toEqual([]) + }) + + test('accepts extension-level input variables when no target declares them', () => { + // Given + const config = { + ...baseConfig, + input: {variables: extensionVariables}, + targeting: [{target: 'some.api.target1', input_query: 'target1.graphql'}], + } + + // When + const got = parseIssues(config) + + // Then + expect(got).toEqual([]) + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/function.ts b/packages/app/src/cli/models/extensions/specifications/function.ts index 2d73e819d01..7bff8affe32 100644 --- a/packages/app/src/cli/models/extensions/specifications/function.ts +++ b/packages/app/src/cli/models/extensions/specifications/function.ts @@ -17,8 +17,19 @@ interface UI { ui_extension_handle?: string } +// Only offers the target-level remedy, even though removing either side would resolve the conflict: +// extension-level `[input.variables]` is on its way out, so the fix we suggest is the one that lasts. +const mixedInputVariablesMessage = + 'Input variables must be defined either at the extension level or on a target, not both. ' + + 'Remove `[input.variables]` from your extension configuration and declare `input_variables` on each target that needs them.' + +const InputVariablesSchema = zod.object({ + namespace: zod.string(), + key: zod.string(), +}) + export type FunctionConfigType = zod.infer -const FunctionExtensionSchema = BaseSchema.extend({ +export const FunctionExtensionSchema = BaseSchema.extend({ build: zod .object({ command: zod @@ -52,12 +63,7 @@ const FunctionExtensionSchema = BaseSchema.extend({ api_version: zod.string(), input: zod .object({ - variables: zod - .object({ - namespace: zod.string(), - key: zod.string(), - }) - .optional(), + variables: InputVariablesSchema.optional(), }) .optional(), targeting: zod @@ -65,10 +71,25 @@ const FunctionExtensionSchema = BaseSchema.extend({ zod.object({ target: zod.string(), input_query: zod.string().optional(), + input_variables: InputVariablesSchema.optional(), export: zod.string().optional(), }), ) .optional(), +}).superRefine((config, ctx) => { + if (!config.input?.variables) return + + // Reported per offending target so the error points at the line to change, rather than at the + // top of the TOML. + config.targeting?.forEach((targeting, index) => { + if (!targeting.input_variables) return + + ctx.addIssue({ + code: zod.ZodIssueCode.custom, + path: ['targeting', index, 'input_variables'], + message: mixedInputVariablesMessage, + }) + }) }) const functionSpec = createExtensionSpecification({ @@ -118,14 +139,21 @@ const functionSpec = createExtensionSpecification({ const targets = config.targeting && (await Promise.all( - config.targeting.map(async (config) => { + config.targeting.map(async (targeting) => { let inputQuery - if (config.input_query) { - inputQuery = await readInputQuery(joinPath(directory, config.input_query)) + if (targeting.input_query) { + inputQuery = await readInputQuery(joinPath(directory, targeting.input_query)) } - return {handle: config.target, export: config.export, input_query: inputQuery} + return { + handle: targeting.target, + export: targeting.export, + input_query: inputQuery, + input_query_variables: targeting.input_variables + ? {single_json_metafield: targeting.input_variables} + : undefined, + } }), ))