From 73807fd1d3da42c291a4298f162a172c7c97322e Mon Sep 17 00:00:00 2001 From: Sagarika Dasgupta Date: Tue, 10 Nov 2026 12:17:21 -0600 Subject: [PATCH 1/2] Fail the build when a prepare target's variables don't match its run query Add typegen.js, which Shopify CLI runs instead of graphql-code-generator when this package has it (Shopify/cli#8829). It generates the function's types with the codegen plugins this package depends on, whatever the app's package manager hoists. It then exits non-zero, failing the build, when a prepare target returns variables its run query doesn't accept. The check is moved here from Shopify/cli#8829: it follows what the prepare export returns, ignoring type annotations and casts, and validates it against the run query's variable definitions. --- .changeset/prepare-variables.md | 5 + prepare-variables.js | 342 +++++++++++++++++++++++++++++++ tests/prepare-variables.test.cjs | 199 ++++++++++++++++++ tests/typegen.test.cjs | 115 +++++++++++ typegen.js | 58 ++++++ 5 files changed, 719 insertions(+) create mode 100644 .changeset/prepare-variables.md create mode 100644 prepare-variables.js create mode 100644 tests/prepare-variables.test.cjs create mode 100644 tests/typegen.test.cjs create mode 100644 typegen.js diff --git a/.changeset/prepare-variables.md b/.changeset/prepare-variables.md new file mode 100644 index 0000000..a252920 --- /dev/null +++ b/.changeset/prepare-variables.md @@ -0,0 +1,5 @@ +--- +"@shopify/shopify_function": minor +--- + +Fail the build when a JavaScript function's prepare target returns variables its run query doesn't accept, and generate its types with this package's codegen plugins. Needs a Shopify CLI that runs this package's `typegen.js`. diff --git a/prepare-variables.js b/prepare-variables.js new file mode 100644 index 0000000..47e4376 --- /dev/null +++ b/prepare-variables.js @@ -0,0 +1,342 @@ +// Checks what each prepare target's export returns against the variables its run target's input query declares. +// Type annotations and casts are ignored, values are followed back to where they're created, and anything that can't +// be followed is an error rather than a pass. +const fs = require("node:fs"); +const path = require("node:path"); +const ts = require("typescript"); +const { + GraphQLInputObjectType, + GraphQLNonNull, + Kind, + buildSchema, + getNamedType, + isEnumType, + isInputObjectType, + isListType, + isNonNullType, + parse, + typeFromAST, +} = require("graphql"); + +const builtinScalars = { Int: "number", Float: "number", String: "string", Boolean: "boolean" }; + +/** The prepare targets whose run target has an input query. */ +function preparePairs(targets) { + return targets.flatMap(({ target, export: name }) => { + const run = targets.find((other) => other.target === target.replace(/\.prepare$/, ".run")); + return target.endsWith(".prepare") && name && run?.inputQuery ? [{ target, name, query: run.inputQuery }] : []; + }); +} + +/** + * Returns `file:line:column message` for each problem, relative to `directory`. `scalars` and `defaultScalarType` are + * codegen's, such as `{JSON: "unknown"}`. + */ +function prepareVariablesErrors({ directory, entry: entryPath, schema: schemaPath, targets, scalars = {}, defaultScalarType = "unknown" }) { + const pairs = preparePairs(targets); + if (pairs.length === 0) return []; + if (!entryPath) return [`The function has no entry file, such as src/index.js, to check its prepare targets in`]; + const entry = path.resolve(directory, entryPath); + + // The TypeScript type codegen gives a scalar, so `unknown` ones (like `JSON`) accept anything. + const scalarKind = (named) => { + const scalar = scalars[named.name]; + const type = typeof scalar === "object" && scalar !== null ? scalar.input : scalar; + return builtinScalars[named.name] ?? (typeof type === "string" ? type : defaultScalarType); + }; + + const schema = buildSchema(fs.readFileSync(path.resolve(directory, schemaPath), "utf8")); + const options = { + allowJs: true, + strict: true, + noEmit: true, + skipLibCheck: true, + types: [], + lib: ["lib.es2022.d.ts"], + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + }; + const host = ts.createCompilerHost(options, true); + // Annotations and casts are claims nothing checks against the value (`@ts-ignore` silences the compiler), so they're + // blanked out of the function's own files, leaving types to come from the values. Parameter types are kept. Blanking + // keeps positions, so errors point at the original source. + const { getSourceFile } = host; + host.getSourceFile = (fileName, language, ...rest) => { + const file = getSourceFile(fileName, language, ...rest); + if (!file || file.isDeclarationFile || fileName.includes("/node_modules/")) return file; + const text = file.text.split(""); + const blank = (start, end) => { + for (let index = start; index < end; index++) if (text[index] !== "\n" && text[index] !== "\r") text[index] = " "; + }; + const visit = (node) => { + for (const tag of ts.getJSDocTags(node)) { + const typing = + ts.isJSDocTypeTag(tag) || ts.isJSDocReturnTag(tag) || ts.isJSDocSatisfiesTag(tag) || ts.isJSDocOverloadTag(tag); + if (typing && !ts.isParameter(tag.parent.parent)) blank(tag.pos, tag.end); + } + if (ts.isAsExpression(node) || ts.isSatisfiesExpression(node) || ts.isNonNullExpression(node)) { + blank(node.expression.end, node.end); + } else if (ts.isTypeAssertionExpression(node)) { + blank(node.type.pos - 1, node.expression.pos); + } else if ((ts.isCallExpression(node) || ts.isNewExpression(node)) && node.typeArguments) { + blank(node.typeArguments.pos - 1, node.typeArguments.end + 1); + } else if ( + (ts.isVariableDeclaration(node) || + ts.isPropertyDeclaration(node) || + ts.isFunctionDeclaration(node) || + ts.isFunctionExpression(node) || + ts.isArrowFunction(node) || + ts.isMethodDeclaration(node) || + ts.isGetAccessorDeclaration(node)) && + node.type + ) { + blank(node.type.pos - 1, node.type.end); + } + ts.forEachChild(node, visit); + }; + visit(file); + return ts.createSourceFile(fileName, text.join(""), language); + }; + const program = ts.createProgram([entry], options, host); + const checker = program.getTypeChecker(); + const entryFile = program.getSourceFile(entry); + const entrySymbol = entryFile && checker.getSymbolAtLocation(entryFile); + const errors = new Set(); + + const report = (node, message) => { + const file = node.getSourceFile(); + const { line, character } = file.getLineAndCharacterOfPosition(node.getStart()); + errors.add(`${path.relative(directory, file.fileName)}:${line + 1}:${character + 1} ${message}`); + }; + const join = (base, key) => (base ? `${base}.${key}` : key); + const subject = (at) => (at ? `\`${at}\`` : "The result"); + const unchecked = (type) => Boolean(type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)); + + const unwrap = (node) => (ts.isParenthesizedExpression(node) ? unwrap(node.expression) : node); + + const functionOf = (node) => { + if (!node) return; + if (ts.isVariableDeclaration(node)) return node.initializer && functionOf(unwrap(node.initializer)); + const local = + !node.getSourceFile().isDeclarationFile && !program.isSourceFileFromExternalLibrary(node.getSourceFile()); + if (local && (ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node) || ts.isArrowFunction(node))) { + return node.body && node; + } + }; + + const symbolOf = (node) => + ts.isShorthandPropertyAssignment(node.parent) && node.parent.name === node + ? checker.getShorthandAssignmentValueSymbol(node.parent) + : checker.getSymbolAtLocation(node); + + // A reference that could change the value after it's created: assigning to it or a member, deleting or + // incrementing a member, calling a method on it, or passing, aliasing or reassigning the binding itself. + const changes = (reference) => { + let top = reference; + while ( + (ts.isPropertyAccessExpression(top.parent) || ts.isElementAccessExpression(top.parent)) && + top.parent.expression === top + ) { + top = top.parent; + } + const { parent } = top; + if (top === reference) { + return !(ts.isSpreadAssignment(parent) || ts.isSpreadElement(parent) || ts.isTypeOfExpression(parent)); + } + return ( + (ts.isBinaryExpression(parent) && + parent.left === top && + parent.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + parent.operatorToken.kind <= ts.SyntaxKind.LastAssignment) || + ts.isDeleteExpression(parent) || + ((ts.isPrefixUnaryExpression(parent) || ts.isPostfixUnaryExpression(parent)) && + (parent.operator === ts.SyntaxKind.PlusPlusToken || parent.operator === ts.SyntaxKind.MinusMinusToken)) || + (ts.isCallExpression(parent) && parent.expression === top) + ); + }; + + for (const { name, query } of pairs) { + const queryPath = path.resolve(directory, query); + const queryName = path.relative(directory, queryPath); + const operation = parse(fs.readFileSync(queryPath, "utf8")).definitions.find( + (definition) => definition.kind === Kind.OPERATION_DEFINITION, + ); + const variables = new GraphQLInputObjectType({ + name: "Variables", + fields: Object.fromEntries( + (operation?.variableDefinitions ?? []).map((definition) => [ + definition.variable.name.value, + { type: typeFromAST(schema, definition.type), defaultValue: definition.defaultValue }, + ]), + ), + }); + const result = new GraphQLInputObjectType({ + name: "PrepareResult", + fields: { variables: { type: new GraphQLNonNull(variables) } }, + }); + const owner = (type) => (type === variables ? `declared by ${queryName}` : `a field of ${type.name}`); + const followed = new Map(); + const checkedReferences = new Set(); + const visiting = new Set(); + + const scalarMatches = (type, named) => { + if (isEnumType(named)) return type.isStringLiteral() && named.getValue(type.value) !== undefined; + switch (scalarKind(named)) { + case "string": + return Boolean(type.flags & ts.TypeFlags.StringLike); + case "number": + return ( + Boolean(type.flags & ts.TypeFlags.NumberLike) && + !(named.name === "Int" && type.isNumberLiteral() && !Number.isInteger(type.value)) + ); + case "boolean": + return Boolean(type.flags & ts.TypeFlags.BooleanLike); + default: + return false; + } + }; + + const typed = (type, expected, at, omittable, node) => { + const named = getNamedType(expected); + if (!isEnumType(named) && !isInputObjectType(named) && scalarKind(named) === "unknown") return; + if (unchecked(type)) return report(node, `${subject(at)} is typed ${checker.typeToString(type)}, so it can't be checked`); + if (type.isUnion()) return type.types.forEach((member) => typed(member, expected, at, omittable, node)); + if (type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Void)) { + if (isNonNullType(expected) && !omittable) report(node, `${subject(at)} can be undefined, but ${expected} has no default`); + return; + } + if (type.flags & ts.TypeFlags.Null) { + if (isNonNullType(expected)) report(node, `${subject(at)} can be null, but ${expected} can't`); + return; + } + const inner = isNonNullType(expected) ? expected.ofType : expected; + if (isListType(inner)) { + const element = + (checker.isArrayType(type) || checker.isTupleType(type)) && checker.getIndexTypeOfType(type, ts.IndexKind.Number); + return element ? typed(element, inner.ofType, `${at}[]`, false, node) : typed(type, inner.ofType, at, false, node); + } + if (isInputObjectType(inner) && type.flags & ts.TypeFlags.Object && !checker.isArrayType(type)) { + if (checker.getIndexInfosOfType(type).length > 0) { + return report(node, `${subject(at)} can have any keys, so it can't be checked`); + } + const fields = inner.getFields(); + for (const property of checker.getPropertiesOfType(type)) { + const field = fields[property.name]; + const key = join(at, property.name); + // A property written in an object literal is followed to its value, keeping literal types (like an enum + // value) that the object's type widens + const declaration = property.valueDeclaration; + const assignment = + declaration && (ts.isPropertyAssignment(declaration) || ts.isShorthandPropertyAssignment(declaration)) + ? declaration + : undefined; + if (!field) { + report(assignment ?? node, `${subject(key)} isn't ${owner(inner)}`); + } else if (assignment) { + const binding = assignment.parent.parent; + if (ts.isVariableDeclaration(binding) && ts.isIdentifier(binding.name)) followed.set(binding, at); + const initializer = ts.isPropertyAssignment(assignment) ? assignment.initializer : assignment.name; + value(initializer, field.type, key, field.defaultValue !== undefined); + } else { + typed(checker.getTypeOfSymbol(property), field.type, key, field.defaultValue !== undefined, node); + } + } + for (const [key, field] of Object.entries(fields)) { + if (isNonNullType(field.type) && field.defaultValue === undefined && !checker.getPropertyOfType(type, key)) { + report(node, `${subject(join(at, key))} is missing, and ${field.type} has no default`); + } + } + return; + } + if (isInputObjectType(inner) || !scalarMatches(type, inner)) { + report(node, `${subject(at)} must be ${expected}, not ${checker.typeToString(type)}`); + } + }; + + const returns = (fn, expected, at, omittable) => { + const body = fn.body; + if (!body) return; + if (!ts.isBlock(body)) return value(body, expected, at, omittable); + const visit = (node) => { + if (ts.isReturnStatement(node)) { + if (node.expression) value(node.expression, expected, at, omittable); + else typed(checker.getUndefinedType(), expected, at, omittable, node); + } else if (!ts.isFunctionLike(node)) { + ts.forEachChild(node, visit); + } + }; + ts.forEachChild(body, visit); + }; + + const value = (node, expected, at, omittable) => { + const expression = unwrap(node); + const inner = isNonNullType(expected) ? expected.ofType : expected; + + if (ts.isIdentifier(expression)) { + const declaration = symbolOf(expression)?.valueDeclaration; + if (declaration && ts.isVariableDeclaration(declaration) && declaration.initializer) { + followed.set(declaration, at); + checkedReferences.add(expression); + return value(declaration.initializer, expected, at, omittable); + } + } else if (ts.isConditionalExpression(expression)) { + value(expression.whenTrue, expected, at, omittable); + return value(expression.whenFalse, expected, at, omittable); + } else if ( + ts.isBinaryExpression(expression) && + (expression.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken || + expression.operatorToken.kind === ts.SyntaxKind.BarBarToken) + ) { + const left = checker.getNonNullableType(checker.getTypeAtLocation(expression.left)); + typed(left, expected, at, omittable, expression.left); + return value(expression.right, expected, at, omittable); + } else if (ts.isCallExpression(expression)) { + const fn = functionOf(checker.getResolvedSignature(expression)?.declaration); + if (fn && !visiting.has(fn)) { + visiting.add(fn); + returns(fn, expected, at, omittable); + visiting.delete(fn); + return; + } + } else if (ts.isArrayLiteralExpression(expression) && isListType(inner)) { + for (const element of expression.elements) { + if (ts.isSpreadElement(element)) typed(checker.getTypeAtLocation(element), expected, at, omittable, element); + else value(element, inner.ofType, `${at}[]`, false); + } + return; + } + typed(checker.getTypeAtLocation(expression), expected, at, omittable, expression); + }; + + const exported = entrySymbol && checker.getExportsOfModule(entrySymbol).find((symbol) => symbol.name === name); + const resolved = exported && (exported.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(exported) : exported); + const fn = functionOf(resolved?.valueDeclaration); + if (!fn) { + errors.add(`${path.relative(directory, entry)} has no \`${name}\` function to check`); + continue; + } + returns(fn, new GraphQLNonNull(result), "", false); + + for (const [declaration, at] of followed) { + const symbol = checker.getSymbolAtLocation(declaration.name); + const visit = (node) => { + if ( + ts.isIdentifier(node) && + node !== declaration.name && + !checkedReferences.has(node) && + symbolOf(node) === symbol && + changes(node) + ) { + report(node, `${subject(at)} is changed or passed on after it's created, so it can't be checked`); + } + ts.forEachChild(node, visit); + }; + visit(declaration.getSourceFile()); + } + } + + return [...errors]; +} + +module.exports = { prepareVariablesErrors }; diff --git a/tests/prepare-variables.test.cjs b/tests/prepare-variables.test.cjs new file mode 100644 index 0000000..84cc5af --- /dev/null +++ b/tests/prepare-variables.test.cjs @@ -0,0 +1,199 @@ +const { describe, test } = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { prepareVariablesErrors } = require("../prepare-variables.js"); + +const schema = ` + scalar JSON + enum Mode { FAST SLOW } + input Rule { handle: String!, weight: Int } + type Query { shop: String } +`; +const runQuery = "query Run($handle: String!, $limit: Int = 5, $mode: Mode, $rules: [Rule!], $data: JSON) { shop }"; + +const paired = [ + { target: "cart.validations.generate.prepare", export: "cartValidationsGeneratePrepare" }, + { target: "cart.validations.generate.run", inputQuery: "src/run.graphql", export: "cartValidationsGenerateRun" }, +]; + +// Writes a function with `files` in src/ and returns its errors, without their locations. +function check(files, targets = paired) { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "prepare-")); + try { + fs.mkdirSync(path.join(directory, "src")); + fs.writeFileSync(path.join(directory, "schema.graphql"), schema); + fs.writeFileSync(path.join(directory, "src/run.graphql"), runQuery); + for (const [file, content] of Object.entries(files)) fs.writeFileSync(path.join(directory, "src", file), content); + return prepareVariablesErrors({ directory, entry: "src/index.js", schema: "schema.graphql", targets }).map((error) => + error.replace(/^\S+:\d+:\d+ /, ""), + ); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } +} + +const prepare = (body) => ({ "index.js": `export function cartValidationsGeneratePrepare(input) {\n${body}\n}` }); + +describe("prepareVariablesErrors", () => { + test("accepts variables the run query declares", () => { + assert.deepEqual( + check( + prepare(` + const region = String(input.region) + return { + variables: { + handle: region ? \`rule-\${region.toLowerCase()}\` : 'default', + limit: undefined, + mode: region === 'eu' ? 'FAST' : null, + rules: [{handle: 'a', weight: 1}], + data: JSON.parse(input.data), + }, + }`), + ), + [], + ); + }); + + test("ignores functions without a prepare target", () => { + assert.deepEqual( + check({}, [{ target: "cart.validations.generate.run", inputQuery: "src/run.graphql", export: "run" }]), + [], + ); + }); + + test("rejects keys the run query does not declare", () => { + assert.deepEqual(check(prepare(`return {variables: {handel: 'a'}}`)), [ + "`variables.handel` isn't declared by src/run.graphql", + "`variables.handle` is missing, and String! has no default", + ]); + }); + + test("rejects values of the wrong type", () => { + assert.deepEqual( + check(prepare(`return {variables: {handle: 1, limit: 1.5, mode: 'MEDIUM', rules: [{handle: 'a', weight: '1'}]}}`)), + [ + "`variables.handle` must be String!, not 1", + "`variables.limit` must be Int, not 1.5", + '`variables.mode` must be Mode, not "MEDIUM"', + '`variables.rules[].weight` must be Int, not "1"', + ], + ); + // Each branch is checked on its own, since TypeScript widens 'FAST' and 'MEDIUM' to string and merges the branches + assert.deepEqual( + check( + prepare(`return input.a ? {variables: {handle: 'a', mode: 'FAST'}} : {variables: {handle: 'a', mode: 'MEDIUM'}}`), + ), + ['`variables.mode` must be Mode, not "MEDIUM"'], + ); + }); + + test("rejects null and undefined where the variable needs a value", () => { + assert.deepEqual(check(prepare(`return input.a ? {variables: {handle: null}} : {variables: {handle: undefined}}`)), [ + "`variables.handle` can be null, but String! can't", + "`variables.handle` can be undefined, but String! has no default", + ]); + assert.deepEqual(check(prepare(`return {}`)), ["`variables` is missing, and Variables! has no default"]); + }); + + test("checks what is built, not what the return type declares", () => { + assert.deepEqual( + check({ + "index.js": ` + /** @returns {{variables: {handle: string}}} */ + export function cartValidationsGeneratePrepare() { + const variables = {handle: 'a', region: 'eu'} + return {variables} + }`, + }), + ["`variables.region` isn't declared by src/run.graphql"], + ); + }); + + test("sees through casts, @ts-ignore and helpers", () => { + assert.deepEqual( + check({ + "index.js": ` + export function cartValidationsGeneratePrepare(input) { + // @ts-ignore + if (input.a) return {variables: /** @type {{handle: string}} */ (JSON.parse(input.raw))} + return {variables: build()} + } + function build() { + return {handle: 2} + }`, + }), + ["`variables` is typed any, so it can't be checked", "`variables.handle` must be String!, not 2"], + ); + }); + + test("rejects variables changed or passed on after they are created", () => { + assert.deepEqual( + check( + prepare(` + const variables = {handle: 'a'} + variables.limit = 'many' + tweak(variables) + return {variables}`), + ), + [ + "`variables` is changed or passed on after it's created, so it can't be checked", + "`variables` is changed or passed on after it's created, so it can't be checked", + ], + ); + assert.deepEqual( + check( + prepare(` + const base = {handle: 'a'} + base.handle = 1 + return {variables: {...base}}`), + ), + ["`variables` is changed or passed on after it's created, so it can't be checked"], + ); + }); + + test("ignores what annotations and casts claim", () => { + assert.deepEqual( + check( + prepare(` + const raw = /** @type {{handle: string}} */ (JSON.parse(input.raw)) + const {limit} = /** @type {{limit: number}} */ (JSON.parse(input.raw)) + return {variables: {handle: raw.handle, limit}}`), + ), + ["`variables.handle` is typed any, so it can't be checked", "`variables.limit` is typed any, so it can't be checked"], + ); + assert.deepEqual( + check({ + "index.js": `export * from './prepare.js'`, + "prepare.ts": ` + type Variables = {handle: string} + export function cartValidationsGeneratePrepare(input: {name?: string; mode: string}): {variables: Variables} { + if (input.mode) return {variables: {handle: input.name!, mode: <'FAST'>input.mode}} + return {variables: new Map().get(input.mode)! as Variables} + }`, + }), + [ + "`variables.handle` can be undefined, but String! has no default", + "`variables.mode` must be Mode, not string", + "`variables` is typed any, so it can't be checked", + ], + ); + }); + + test("follows re-exports", () => { + assert.deepEqual( + check({ + "index.js": `export * from './prepare.js'`, + "prepare.js": `export const cartValidationsGeneratePrepare = () => ({variables: {handle: 1}})`, + }), + ["`variables.handle` must be String!, not 1"], + ); + }); + + test("rejects a missing prepare export", () => { + assert.deepEqual(check({ "index.js": "export function other() {}" }), [ + "src/index.js has no `cartValidationsGeneratePrepare` function to check", + ]); + }); +}); diff --git a/tests/typegen.test.cjs b/tests/typegen.test.cjs new file mode 100644 index 0000000..04e8275 --- /dev/null +++ b/tests/typegen.test.cjs @@ -0,0 +1,115 @@ +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const { spawnSync } = require("node:child_process"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { generate } = require("@graphql-codegen/cli"); + +const typegen = path.join(__dirname, "..", "typegen.js"); + +const schema = ` +schema { query: Input, mutation: MutationRoot } +scalar JSON +scalar Void +input LineSelector @oneOf { id: ID, handle: String } +type CartLine { id: ID! } +type Input { id: ID!, lines(selector: LineSelector!, first: Int): [CartLine!]! } +input FunctionTargetPrepareResult { variables: JSON! } +input FunctionTargetRunResult { lineCount: Int! } +type MutationRoot { + "Handles the Function result for the test.target.prepare target." + targetPrepare(result: FunctionTargetPrepareResult!): Void! + "Handles the Function result for the test.target.run target." + targetRun(result: FunctionTargetRunResult!): Void! +} +`; + +const queries = { + "target_prepare.graphql": "query TargetPrepareInput { id }", + "target_run.graphql": "query TargetRunInput($selector: LineSelector!, $first: Int) { lines(selector: $selector, first: $first) { id } }", +}; + +const targets = [ + { target: "test.target.prepare", export: "targetPrepare", inputQuery: "src/target_prepare.graphql" }, + { target: "test.target.run", export: "targetRun", inputQuery: "src/target_run.graphql" }, +]; + +const prepare = (variables) => ({ + "index.js": 'export * from "./prepare.js";\n', + "prepare.js": `export function targetPrepare() {\n return { variables: ${variables} };\n}\n`, +}); + +// The templates' codegen config, with Shopify CLI's defaults merged in. +const codegenConfig = (config) => ({ + schema: "schema.graphql", + documents: "src/*.graphql", + generates: { "./generated/api.ts": { plugins: ["typescript", "typescript-operations"] } }, + config: { defaultScalarType: "unknown", scalars: { ID: "string", JSON: "unknown", Void: "null" }, omitOperationSuffix: true, ...config }, +}); + +// Runs typegen.js as Shopify CLI does. `modules` are written to the function's node_modules. +function runTypegen(t, { files, fnTargets = targets, config = {}, queryFiles = queries, modules = {} }) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "function-")); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "typegen-")); + t.after(() => [dir, tmp].forEach((d) => fs.rmSync(d, { recursive: true, force: true }))); + fs.writeFileSync(path.join(dir, "schema.graphql"), schema); + fs.mkdirSync(path.join(dir, "src")); + for (const [name, contents] of Object.entries({ ...queryFiles, ...files })) fs.writeFileSync(path.join(dir, "src", name), contents); + for (const [name, contents] of Object.entries(modules)) { + fs.mkdirSync(path.join(dir, "node_modules", name), { recursive: true }); + fs.writeFileSync(path.join(dir, "node_modules", name, "index.js"), contents); + } + fs.writeFileSync(path.join(tmp, "codegen.json"), JSON.stringify(codegenConfig(config))); + const fn = { handle: "prepare-test", directory: dir, entry: "src/index.js", schema: "schema.graphql", targets: fnTargets }; + fs.writeFileSync(path.join(tmp, "function.json"), JSON.stringify(fn)); + const result = spawnSync( + process.execPath, + [typegen, "--config", path.join(tmp, "codegen.json"), "--function", path.join(tmp, "function.json")], + { cwd: dir, encoding: "utf8" }, + ); + const api = path.join(dir, "generated", "api.ts"); + return { dir, status: result.status, stderr: result.stderr, types: fs.existsSync(api) ? fs.readFileSync(api, "utf8") : undefined }; +} + +test("passes variables the run query accepts, generating the types with this package's plugins rather than the function's", (t) => { + // Plugins that work, as another version installed in the function would, but mark the output + const other = 'exports.plugin = () => "// the function\'s plugin";'; + const fn = runTypegen(t, { + files: prepare('{ selector: { id: "gid://shopify/CartLine/1" }, first: 5 }'), + modules: { "@graphql-codegen/typescript": other, "@graphql-codegen/typescript-operations": other }, + }); + assert.equal(fn.status, 0, fn.stderr); + assert.doesNotMatch(fn.types, /the function's plugin/); + assert.match(fn.types, /export type TargetRunInputVariables = /); +}); + +test("fails when a prepare target returns variables its run query doesn't accept", (t) => { + const fn = runTypegen(t, { files: prepare('{ selectr: { id: "gid://shopify/CartLine/1" }, first: "5" }') }); + assert.equal(fn.status, 1); + assert.equal( + fn.stderr, + [ + `The function extension "prepare-test" returns prepare variables its run query doesn't accept`, + "src/prepare.js:2:25 `variables.selectr` isn't declared by src/target_run.graphql", + 'src/prepare.js:2:77 `variables.first` must be Int, not "5"', + "src/prepare.js:2:23 `variables.selector` is missing, and LineSelector! has no default", + "", + ].join("\n"), + ); + // The types are still generated, for editors + assert.match(fn.types, /export type TargetRunInputVariables = /); +}); + +test("without prepare targets, generates the same types as the typescript and typescript-operations plugins", async (t) => { + const fn = runTypegen(t, { files: prepare("{}"), fnTargets: [targets[1]] }); + assert.equal(fn.status, 0, fn.stderr); + const [plain] = await generate({ ...codegenConfig({}), cwd: fn.dir, silent: true, pluginLoader: require }, false); + assert.equal(fn.types, plain.content); +}); + +test("fails when codegen fails", (t) => { + const fn = runTypegen(t, { files: prepare("{}"), queryFiles: { ...queries, "target_run.graphql": "query TargetRunInput { missingField }" } }); + assert.equal(fn.status, 1); + assert.match(fn.stderr, /missingField/); +}); diff --git a/typegen.js b/typegen.js new file mode 100644 index 0000000..edb978f --- /dev/null +++ b/typegen.js @@ -0,0 +1,58 @@ +#!/usr/bin/env node +// Shopify CLI runs this instead of graphql-code-generator when the package has it: +// +// node typegen.js --config --function +// +// The function JSON is {handle, directory, entry, schema, targets: [{target, export, inputQuery}]}, where `export` is +// the JavaScript function the CLI bundles. It exits non-zero when codegen fails or a prepare target returns variables +// its run query doesn't accept. +const fs = require("node:fs"); +const path = require("node:path"); +const { createRequire } = require("node:module"); +const { parseArgs } = require("node:util"); +const { createContext, generate } = require("@graphql-codegen/cli"); +const { prepareVariablesErrors } = require("./prepare-variables.js"); + +async function main(args) { + const { values } = parseArgs({ args, options: { config: { type: "string" }, function: { type: "string" } } }); + if (!values.config || !values.function) throw new Error("Usage: typegen.js --config --function "); + const fn = { schema: "schema.graphql", targets: [], ...JSON.parse(fs.readFileSync(values.function, "utf8")) }; + + const context = await createContext({ config: values.config }); + const config = context.getConfig(); + context.updateConfig({ + // Errors are reported on stderr, so codegen's progress output would only repeat them + silent: true, + pluginLoader: pluginLoader(fn.directory), + }); + await generate(context); + + const errors = prepareVariablesErrors({ + ...fn, + scalars: config.config?.scalars, + defaultScalarType: config.config?.defaultScalarType, + }); + if (errors.length > 0) { + const name = fn.handle ? `The function extension "${fn.handle}"` : "The function"; + throw new Error(`${name} returns prepare variables its run query doesn't accept\n${errors.join("\n")}`); + } +} + +// Loads plugins from this package first, so the versions it pins run, whatever the package manager hoists. +function pluginLoader(directory) { + const fromPackage = createRequire(__filename); + const fromFunction = createRequire(path.join(directory, "package.json")); + return (name) => { + try { + return fromPackage(name); + } catch (error) { + if (error.code !== "MODULE_NOT_FOUND") throw error; + } + return fromFunction(name); + }; +} + +main(process.argv.slice(2)).catch((error) => { + console.error(error.message); + process.exitCode = 1; +}); From 106e3870b2bbd094c3b569f9abc02dabaf295544 Mon Sep 17 00:00:00 2001 From: Sagarika Dasgupta Date: Tue, 10 Nov 2026 12:17:56 -0600 Subject: [PATCH 2/2] Type a prepare result's variables with its run query's variables type Add @shopify/shopify_function/codegen, a plugin to use instead of typescript, with the same output. It takes customScalarOverrides to give a custom scalar field of an input object another type. typegen.js uses it in place of typescript, and gives each prepare result's variables its run query's variables type, so editors report mismatches. --- .changeset/prepare-variables-types.md | 5 ++ codegen.js | 92 +++++++++++++++++++++++++++ prepare-variables.js | 2 +- tests/typegen.test.cjs | 43 ++++++++++++- typegen.js | 55 +++++++++++++++- 5 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 .changeset/prepare-variables-types.md create mode 100644 codegen.js diff --git a/.changeset/prepare-variables-types.md b/.changeset/prepare-variables-types.md new file mode 100644 index 0000000..eae7389 --- /dev/null +++ b/.changeset/prepare-variables-types.md @@ -0,0 +1,5 @@ +--- +"@shopify/shopify_function": minor +--- + +Give a prepare result's `variables` its run query's variables type, so editors report mismatches. Also adds `@shopify/shopify_function/codegen`, a plugin to use instead of `typescript` when running codegen yourself. diff --git a/codegen.js b/codegen.js new file mode 100644 index 0000000..20475e2 --- /dev/null +++ b/codegen.js @@ -0,0 +1,92 @@ +// A GraphQL Codegen plugin to use instead of `typescript`, with the same output. `customScalarOverrides` maps +// "InputObject.field" to a TypeScript type to use instead of the field's custom scalar, keeping lists and nullability. +const { plugin: typescriptPlugin } = require("@graphql-codegen/typescript"); +const { + Kind, + buildASTSchema, + getNamedType, + isInputObjectType, + isScalarType, + isSpecifiedScalarType, + parse, + printSchema, + visit, +} = require("graphql"); + +exports.plugin = (schema, documents, config) => { + const overrides = parseOverrides(schema, config.customScalarOverrides); + if (overrides.length === 0) { + return typescriptPlugin(schema, documents, config); + } + + // Each overridden field gets a scalar of its own, mapped to the override's type. + const taken = new Set([...Object.keys(schema.getTypeMap()), ...Object.keys(config.scalars ?? {})]); + const scalars = { ...config.scalars }; + const fieldScalars = new Map(); + for (const { typeName, fieldName, type } of overrides) { + let name = `${typeName}${fieldName[0].toUpperCase()}${fieldName.slice(1)}`; + while (taken.has(name)) name += "Override"; + taken.add(name); + scalars[name] = type; + fieldScalars.set(`${typeName}.${fieldName}`, name); + } + return typescriptPlugin(withFieldScalars(schema, fieldScalars), documents, { ...config, scalars }); +}; + +exports.validate = (_schema, _documents, _config, _outputFile, plugins) => { + if (plugins.some((plugin) => Object.hasOwn(plugin, "typescript"))) { + throw new Error( + "@shopify/shopify_function/codegen generates the same types as the typescript plugin. Use it instead of typescript, not with it.", + ); + } +}; + +function parseOverrides(schema, overrides = {}) { + const parsed = []; + const errors = []; + for (const [path, type] of Object.entries(overrides)) { + const [typeName, fieldName, ...rest] = path.split("."); + const inputObject = schema.getType(typeName); + const field = rest.length === 0 && isInputObjectType(inputObject) ? inputObject.getFields()[fieldName] : undefined; + const scalar = field && getNamedType(field.type); + if (!field) { + errors.push(`"${path}" isn't a field of an input object.`); + } else if (!isScalarType(scalar) || isSpecifiedScalarType(scalar)) { + errors.push(`"${path}" is ${field.type}, not a custom scalar.`); + } else { + parsed.push({ typeName, fieldName, type }); + } + } + if (errors.length > 0) { + throw new Error(`customScalarOverrides:\n${errors.map((error) => ` ${error}`).join("\n")}`); + } + return parsed; +} + +function withFieldScalars(schema, fieldScalars) { + const retype = (node) => ({ + ...node, + fields: node.fields?.map((field) => { + const name = fieldScalars.get(`${node.name.value}.${field.name.value}`); + return name ? { ...field, type: renameNamedType(field.type, name) } : field; + }), + }); + const document = visit(parse(printSchema(schema)), { + InputObjectTypeDefinition: retype, + InputObjectTypeExtension: retype, + }); + const scalarDefinitions = [...fieldScalars.values()].map((name) => ({ + kind: Kind.SCALAR_TYPE_DEFINITION, + name: { kind: Kind.NAME, value: name }, + })); + return buildASTSchema( + { ...document, definitions: [...document.definitions, ...scalarDefinitions] }, + { assumeValidSDL: true }, + ); +} + +function renameNamedType(type, name) { + return type.kind === Kind.NAMED_TYPE + ? { ...type, name: { kind: Kind.NAME, value: name } } + : { ...type, type: renameNamedType(type.type, name) }; +} diff --git a/prepare-variables.js b/prepare-variables.js index 47e4376..077b2da 100644 --- a/prepare-variables.js +++ b/prepare-variables.js @@ -339,4 +339,4 @@ function prepareVariablesErrors({ directory, entry: entryPath, schema: schemaPat return [...errors]; } -module.exports = { prepareVariablesErrors }; +module.exports = { preparePairs, prepareVariablesErrors }; diff --git a/tests/typegen.test.cjs b/tests/typegen.test.cjs index 04e8275..7776154 100644 --- a/tests/typegen.test.cjs +++ b/tests/typegen.test.cjs @@ -17,6 +17,9 @@ type CartLine { id: ID! } type Input { id: ID!, lines(selector: LineSelector!, first: Int): [CartLine!]! } input FunctionTargetPrepareResult { variables: JSON! } input FunctionTargetRunResult { lineCount: Int! } +input OptionalVariables { variables: JSON } +input VariablesList { variables: [JSON!]! } +input VariablesChoice @oneOf { variables: JSON, none: Boolean } type MutationRoot { "Handles the Function result for the test.target.prepare target." targetPrepare(result: FunctionTargetPrepareResult!): Void! @@ -72,7 +75,7 @@ function runTypegen(t, { files, fnTargets = targets, config = {}, queryFiles = q return { dir, status: result.status, stderr: result.stderr, types: fs.existsSync(api) ? fs.readFileSync(api, "utf8") : undefined }; } -test("passes variables the run query accepts, generating the types with this package's plugins rather than the function's", (t) => { +test("passes variables the run query accepts, typing them with this package's plugins rather than the function's", (t) => { // Plugins that work, as another version installed in the function would, but mark the output const other = 'exports.plugin = () => "// the function\'s plugin";'; const fn = runTypegen(t, { @@ -81,6 +84,8 @@ test("passes variables the run query accepts, generating the types with this pac }); assert.equal(fn.status, 0, fn.stderr); assert.doesNotMatch(fn.types, /the function's plugin/); + assert.match(fn.types, /variables: Scalars\['FunctionTargetPrepareResultVariables'\]\['input'\]/); + assert.match(fn.types, /FunctionTargetPrepareResultVariables: \{ input: TargetRunInputVariables;/); assert.match(fn.types, /export type TargetRunInputVariables = /); }); @@ -98,7 +103,7 @@ test("fails when a prepare target returns variables its run query doesn't accept ].join("\n"), ); // The types are still generated, for editors - assert.match(fn.types, /export type TargetRunInputVariables = /); + assert.match(fn.types, /FunctionTargetPrepareResultVariables/); }); test("without prepare targets, generates the same types as the typescript and typescript-operations plugins", async (t) => { @@ -108,6 +113,40 @@ test("without prepare targets, generates the same types as the typescript and ty assert.equal(fn.types, plain.content); }); +test("applies customScalarOverrides from the config, keeping lists, nullability and oneOf variants", (t) => { + const fn = runTypegen(t, { + files: prepare('{ selector: { id: "gid://shopify/CartLine/1" } }'), + config: { + customScalarOverrides: { + "FunctionTargetPrepareResult.variables": "Record", + "OptionalVariables.variables": "TargetRunInputVariables", + "VariablesList.variables": "TargetRunInputVariables", + "VariablesChoice.variables": "TargetRunInputVariables", + }, + }, + }); + assert.equal(fn.status, 0, fn.stderr); + for (const line of [ + "FunctionTargetPrepareResultVariables: { input: Record;", + "variables?: InputMaybe;", + "variables: Array;", + "{ none?: never; variables: Scalars['VariablesChoiceVariables']['input']; }", + ]) { + assert.ok(fn.types.includes(line), line); + } +}); + +test("fails when an override isn't a custom scalar field of an input object", (t) => { + const fn = runTypegen(t, { + files: prepare("{}"), + fnTargets: [targets[1]], + config: { customScalarOverrides: { "Input.id": "TargetRunInputVariables", "FunctionTargetRunResult.lineCount": "TargetRunInputVariables" } }, + }); + assert.equal(fn.status, 1); + assert.match(fn.stderr, /"Input\.id" isn't a field of an input object\./); + assert.match(fn.stderr, /"FunctionTargetRunResult\.lineCount" is Int!, not a custom scalar\./); +}); + test("fails when codegen fails", (t) => { const fn = runTypegen(t, { files: prepare("{}"), queryFiles: { ...queries, "target_run.graphql": "query TargetRunInput { missingField }" } }); assert.equal(fn.status, 1); diff --git a/typegen.js b/typegen.js index edb978f..645890d 100644 --- a/typegen.js +++ b/typegen.js @@ -11,7 +11,10 @@ const path = require("node:path"); const { createRequire } = require("node:module"); const { parseArgs } = require("node:util"); const { createContext, generate } = require("@graphql-codegen/cli"); -const { prepareVariablesErrors } = require("./prepare-variables.js"); +const { TypeScriptDocumentsVisitor } = require("@graphql-codegen/typescript-operations"); +const { Kind, buildSchema, getNamedType, isInputObjectType, isScalarType, isSpecifiedScalarType, parse } = require("graphql"); +const codegenPlugin = require("./codegen.js"); +const { preparePairs, prepareVariablesErrors } = require("./prepare-variables.js"); async function main(args) { const { values } = parseArgs({ args, options: { config: { type: "string" }, function: { type: "string" } } }); @@ -24,6 +27,7 @@ async function main(args) { // Errors are reported on stderr, so codegen's progress output would only repeat them silent: true, pluginLoader: pluginLoader(fn.directory), + generates: withPrepareVariableTypes(config, fn), }); await generate(context); @@ -42,7 +46,11 @@ async function main(args) { function pluginLoader(directory) { const fromPackage = createRequire(__filename); const fromFunction = createRequire(path.join(directory, "package.json")); + // As `typescript`, it replaces that plugin, so it skips the check against using both. + const asTypescript = { plugin: codegenPlugin.plugin }; return (name) => { + if (name === "@graphql-codegen/typescript") return asTypescript; + if (name === "@shopify/shopify_function/codegen") return codegenPlugin; try { return fromPackage(name); } catch (error) { @@ -52,6 +60,51 @@ function pluginLoader(directory) { }; } +// Gives each prepare result's `variables` its run query's variables type, in outputs with both plugins. Overrides in +// the config win. +function withPrepareVariableTypes(config, fn) { + const schema = buildSchema(fs.readFileSync(path.resolve(fn.directory, fn.schema), "utf8")); + const entries = Object.entries(config.generates ?? {}).map(([file, output]) => { + const { plugins = [], config: outputConfig = {}, ...rest } = Array.isArray(output) ? { plugins: output } : output; + const names = plugins.map((plugin) => (typeof plugin === "string" ? plugin : Object.keys(plugin)[0])); + if (!names.includes("typescript-operations") || !names.some((name) => name === "typescript" || name === "@shopify/shopify_function/codegen")) { + return [file, output]; + } + const overrides = { + ...prepareVariableTypes(schema, fn, { ...config.config, ...outputConfig }), + ...config.config?.customScalarOverrides, + ...outputConfig.customScalarOverrides, + }; + return [file, { ...rest, plugins, config: { ...outputConfig, customScalarOverrides: overrides } }]; + }); + return Object.fromEntries(entries); +} + +function prepareVariableTypes(schema, fn, config) { + const visitor = new TypeScriptDocumentsVisitor(schema, config, []); + const overrides = {}; + for (const { target, query } of preparePairs(fn.targets)) { + // The schema describes the field the result is passed to as handling the target's result + const field = Object.values(schema.getMutationType()?.getFields() ?? {}).find((candidate) => + candidate.description?.includes(` ${target} `), + ); + const result = field?.args[0] && getNamedType(field.args[0].type); + const variables = isInputObjectType(result) ? result.getFields().variables : undefined; + const scalar = variables && getNamedType(variables.type); + if (!isScalarType(scalar) || isSpecifiedScalarType(scalar)) continue; + + const operation = parse(fs.readFileSync(path.resolve(fn.directory, query), "utf8")).definitions.find( + (definition) => definition.kind === Kind.OPERATION_DEFINITION, + ); + if (!operation?.name) continue; + const name = operation.name.value; + overrides[`${result.name}.variables`] = visitor.convertName(name, { + suffix: `${visitor.getOperationSuffix(name, "Query")}Variables`, + }); + } + return overrides; +} + main(process.argv.slice(2)).catch((error) => { console.error(error.message); process.exitCode = 1;