Upgrade graphql-js to v17 alpha + ESM migration + Schema Coordinates#217
Upgrade graphql-js to v17 alpha + ESM migration + Schema Coordinates#217captbaritone wants to merge 3 commits intomainfrom
Conversation
Major changes: - Upgrade graphql-js from v16 to 17.0.0-alpha.11 - Migrate to ESM: add "type": "module" to package.json, nodenext module resolution, .js extensions on all relative imports - Replace ts-node with tsx for running TypeScript directly - Rewrite `locate` command to use graphql-js's resolveSchemaCoordinate() API, supporting schema coordinate syntax (Type, Type.field, Type.field(arg:), @directive, etc.) - Replace instanceof checks with type guard functions - Update defaultValue handling for v17's new arg.default property - Convert configSpecRaw.json import to inline TypeScript for webpack compat - Update all example projects for ESM (type: module, .js imports, graphql v17, import.meta.url for __dirname) - Update website: tsx instead of ts-node, graphql v17, webpack config for extensionAlias and Node.js fallbacks - Update all test fixture snapshots for v17 output changes - Add new test fixtures for enum value and field argument locate
✅ Deploy Preview for grats ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| } catch (e: unknown) { | ||
| return err( | ||
| `Cannot locate field \`${entity.field}\` on type \`${entity.parent}\`. Only object types, interfaces, and input objects have fields.`, | ||
| `Invalid schema coordinate: \`${coordinate}\`. ${e instanceof Error ? e.message : String(e)}`, |
There was a problem hiding this comment.
Could this include a link to our docs or the spec defining how schema coordidates work?
Question: Do our docs describe the syntax? They should! And link back to the draft spec.
There was a problem hiding this comment.
Done — added a link to the spec (https://spec.graphql.org/draft/#sec-Schema-Coordinates) in the docstring.
Re: docs — the docs don't currently describe schema coordinate syntax. That would be good to add (to the locate CLI docs page), but probably as a follow-up.
src/configSpec.ts
Outdated
| experimental: true, | ||
| }, | ||
| }, | ||
| } as any; |
There was a problem hiding this comment.
What's going on here? Having the JSON is important for the playground I think. Can we find a way to go back to having this in JSON?
There was a problem hiding this comment.
Fixed — restored JSON. Now using createRequire(import.meta.url) to load configSpecRaw.json, which works in both Node.js ESM and webpack (unlike import ... with { type: "json" } which babel-loader can't parse). The build script copies the JSON file to dist/ since TypeScript doesn't copy non-TS files.
examples/apollo-server/tsconfig.json
Outdated
| "grats": { | ||
| "nullableByDefault": false | ||
| "nullableByDefault": false, | ||
| "importModuleSpecifierEnding": ".js" |
There was a problem hiding this comment.
Looks like all packages need to add this now. Is it time to change the default here? Is this tied to using es modules?
There was a problem hiding this comment.
Changed the default to ".js". Since .js extensions work for both CJS and ESM Node.js projects, this is a better default. All example tsconfigs no longer need to set this explicitly.
The one exception is the Next.js example which uses moduleResolution: "bundler" — webpack can't resolve .js to .ts without extra config, so that example sets importModuleSpecifierEnding: "".
| path: require.resolve("path-browserify"), | ||
| url: false, | ||
| fs: false, | ||
| module: false, |
There was a problem hiding this comment.
Playground crashes with
Uncaught (in promise) TypeError: (0 , u.fileURLToPath) is not a function
at 34282 (4747.0d4c8b85.js:1:393)
at o (4747.0d4c8b85.js:6:5872997)
at 4747.0d4c8b85.js:6:5873082
at o.O (4747.0d4c8b85.js:6:5873352)
at o.x (4747.0d4c8b85.js:6:5873104)
There was a problem hiding this comment.
Fixed — gratsRoot.ts now has a runtime guard:
const gratsRoot =
typeof fileURLToPath === "function"
? join(dirname(fileURLToPath(import.meta.url)), "../..")
: ".";In the browser (playground), fileURLToPath is undefined so it falls back to ".", which is fine since the playground doesn't use file paths. The url, fs, and module fallbacks in the webpack config are set to false to avoid bundling Node.js builtins.
There was a problem hiding this comment.
Now I get:
Uncaught (in promise) TypeError: (0 , r(...).createRequire) is not a function
at 80296 (8926b418.5a329830.js:1:10040)
at r (runtime~main.d6c746fe.js:1:136)
at 2801 (8926b418.5a329830.js:1:19272)
at r (runtime~main.d6c746fe.js:1:136)
|
|
||
| ```text | ||
| * Applied fix "Add export keyword to function with @gqlField" in grats/src/tests/fixtures/field_definitions/asyncFunctionFieldNotExported.invalid.ts | ||
| * Applied fix "Add export keyword to function with @gqlField" in src/tests/fixtures/field_definitions/asyncFunctionFieldNotExported.invalid.ts |
There was a problem hiding this comment.
Lets have a conversation about these path changes dropping grats/. Curious to know more about what's causing that.
There was a problem hiding this comment.
Fixed — this was caused by the findGratsRoot() approach in gratsRoot.ts which walked up the filesystem to find package.json. When running tests via tsx from the source tree, it found the root correctly, but the relative path calculation differed from the original join(__dirname, "../..").
Reverted to the simple static approach: join(dirname(fileURLToPath(import.meta.url)), "../.."). The grats/ prefix is restored in all test snapshots.
src/gratsRoot.ts
Outdated
| } | ||
| dir = parent; | ||
| } | ||
| } |
There was a problem hiding this comment.
Curious to learn why we need this. Makes me a bit nervous. (Perhaps especially since we need to run on the web). Also fear of some weird symlink thing leading to infinite loops, or just perf in general (I think we call this in a few places and we should not have to hit the FS in a loop each time).
There was a problem hiding this comment.
Agreed — removed findGratsRoot() entirely. Replaced with the simple static approach:
const gratsRoot =
typeof fileURLToPath === "function"
? join(dirname(fileURLToPath(import.meta.url)), "../..")
: ".";No filesystem walking, no loops, no symlink concerns. The typeof guard handles the browser/playground case where fileURLToPath isn't available. The "../.." works from both src/gratsRoot.ts (via tsx) and dist/src/gratsRoot.js (compiled) since the relative depth is the same.
- Add schema coordinates spec link to Locate.ts docstring - Restore JSON config import via createRequire (instead of inline TS) - Copy configSpecRaw.json to dist in build step - Simplify gratsRoot.ts to use static import.meta.url with browser guard - Change importModuleSpecifierEnding default from "" to ".js" - Add extensionAlias to Next.js example webpack config for .js resolution - Remove explicit importModuleSpecifierEnding from example tsconfigs - Update test snapshots
| "prettier": "^3.6.2", | ||
| "process": "^0.11.10", | ||
| "ts-node": "^10.9.2" | ||
| "ts-node": "^10.9.2", |
There was a problem hiding this comment.
Can we remove this now that we have tsx?
- Reference Schema Coordinates spec - Add examples for field arguments, enum values, and directives - Update argument name from ENTITY to COORDINATE
Summary
"type": "module",nodenextmodule resolution,.jsextensions on all importsts-nodewithtsxfor running TypeScript directly under ESMlocateCLI command to use graphql-js'sresolveSchemaCoordinate()API, adding support for full schema coordinate syntax (Type,Type.field,Type.field(arg:),@directive,@directive(arg:),EnumType.VALUE)instanceofchecks with type guard functions (isObjectType,isInterfaceType, etc.)defaultValuehandling for v17's newarg.defaultpropertyBreaking Changes
require()will need to update)locatecommand now uses schema coordinate syntax (backwards compatible forTypeandType.fieldforms)Test plan
pnpm run buildpassespnpm run test— all 3 test suites passpnpm run lintpasses (eslint + prettier)pnpm run --filter="!website" -r grats— all example project schemas regeneratepnpm run --filter="!website" -r build— all example projects buildpnpm run gratsin website/) workspnpm run buildin website/) succeeds