Warn when schema.graphql api_version mismatches extension TOML#7098
Warn when schema.graphql api_version mismatches extension TOML#7098davejcameron wants to merge 1 commit intomainfrom
Conversation
Coverage report
Test suite run success3932 tests passing in 1513 suites. Report generated by 🧪jest coverage report action from c185d02 |
shivamkaravadra
force-pushed
the
dc.warn-schema-version-mismatch
branch
from
c185d02 to
ff8c1cf
Compare
shivamkaravadra
force-pushed
the
dc.warn-schema-version-mismatch
branch
4 times, most recently
from
39dd7b8 to
e993f17
Compare
shivamkaravadra
force-pushed
the
dc.warn-schema-version-mismatch
branch
from
e993f17 to
a876b52
Compare
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
There was a problem hiding this comment.
Pull request overview
Adds a pre-build warning to help developers notice when a function’s local schema.graphql was generated for a different API version than the extension’s api_version, avoiding confusing typegen/build errors caused by stale schemas.
Changes:
- Adds a schema
@apiVersion(version: "...")stamp check at the start ofbuildFunctionExtensionand warns on mismatch. - Introduces unit tests covering match/mismatch/no-directive/missing-file scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/app/src/cli/services/build/extension.ts | Reads schema.graphql to detect stamped API version and emits a warning before building when mismatched. |
| packages/app/src/cli/services/build/extension.test.ts | Adds tests validating the new warning behavior under several schema-file conditions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| content = await readFile(schemaPath) | ||
| } catch (error) { | ||
| if (error instanceof Error && 'code' in error) return |
There was a problem hiding this comment.
The error handler in warnIfSchemaMismatch returns for any filesystem error that happens to have a code property (e.g. EACCES/EPERM), which can hide real problems reading schema.graphql and lead to more cryptic failures later in the build. Only ignore the expected “file not found” case (e.g. code === 'ENOENT') and rethrow other read errors.
| if (error instanceof Error && 'code' in error) return | |
| if (error instanceof Error && 'code' in error && error.code === 'ENOENT') return |
| async function warnIfSchemaMismatch(extension: ExtensionInstance<FunctionConfigType>) { | ||
| const schemaPath = joinPath(extension.directory, 'schema.graphql') | ||
| let content: string | ||
| try { | ||
| content = await readFile(schemaPath) | ||
| } catch (error) { |
There was a problem hiding this comment.
This check currently reads the entire schema.graphql into memory via readFile(schemaPath). The PR description calls out reading only the first ~512 bytes; doing that would keep the check lightweight even if schemas become large. Consider reading a small prefix (e.g. open + read / or readFile then slice) before applying the regex.
| return runCommand(extension.buildCommand, extension, options) | ||
| } | ||
|
|
||
| const API_VERSION_DIRECTIVE_RE = /@apiVersion\(version:\s*"([^"]+)"\)/ |
There was a problem hiding this comment.
API_VERSION_DIRECTIVE_RE requires version: with no whitespace before the colon. If the server ever formats this as version : "YYYY-MM" (still valid GraphQL), the warning will be skipped. Making the regex tolerant of optional whitespace around the colon would make the check more robust.
| const API_VERSION_DIRECTIVE_RE = /@apiVersion\(version:\s*"([^"]+)"\)/ | |
| const API_VERSION_DIRECTIVE_RE = /@apiVersion\(version\s*:\s*"([^"]+)"\)/ |
| function mockSchemaNotFound() { | ||
| const err = new Error('ENOENT') as NodeJS.ErrnoException | ||
| err.code = 'ENOENT' | ||
| vi.mocked(readFile).mockRejectedValueOnce(err) | ||
| } |
There was a problem hiding this comment.
The new tests cover ENOENT, match, mismatch, and “no directive”, but there’s no assertion for non-ENOENT read failures. Adding a test that readFile rejects with something like EACCES/EPERM and ensuring the error is propagated would prevent regressions (especially if warnIfSchemaMismatch is tightened to only ignore ENOENT).
|
It seems as though the failing test may be flaky as it is timing out and another PR was merged with the same issue. |
WHY are these changes introduced?
When a developer changes
api_versionin theirshopify.extension.toml, the localschema.graphqlbecomes stale. The build then fails with cryptic type errors from typegen, with no indication that the schema simply needs to be re-fetched. This is a common failure case when working with Shopify Functions.WHAT is this pull request doing?
Adds a lightweight check at the start of
buildFunctionExtensionthat reads the first 512 bytes ofschema.graphqllooking for the server-stamped@apiVersiondirective (e.g.schema @apiVersion(version: "2025-04") {). If present and it doesn't match the TOML'sapi_version, a warning is printed before the build runs:If the directive isn't present (legacy schema files, or server hasn't adopted stamping yet), the check is silently skipped — no false positives.
Depends on https://github.com/shop/world/pull/543227 for the server to stamp schemas with the
@apiVersiondirective.How to test your changes?
api_version: "2025-01"shopify app function schemato fetch the schema (requires server-side directive stamping to be live, or manually addschema @apiVersion(version: "2025-01") {to the top ofschema.graphql)api_versionto"2025-04"inshopify.extension.tomlshopify app function buildMeasuring impact
How do we know this change was effective? Please choose one:
Checklist