Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 0 deletions .changeset/function-target-level-input-variables.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/app': minor
---

Support `input_variables` on function targets, alongside the extension-level `[input.variables]`
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
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'
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<FunctionConfigType>
Expand Down Expand Up @@ -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'}]
Expand Down Expand Up @@ -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([])
})
})
50 changes: 39 additions & 11 deletions packages/app/src/cli/models/extensions/specifications/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof FunctionExtensionSchema>
const FunctionExtensionSchema = BaseSchema.extend({
export const FunctionExtensionSchema = BaseSchema.extend({
build: zod
.object({
command: zod
Expand Down Expand Up @@ -52,23 +63,33 @@ 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
.array(
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({
Expand Down Expand Up @@ -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,
}
}),
))

Expand Down
Loading