Skip to content

Conversation

erikhfj
Copy link
Member

@erikhfj erikhfj commented Nov 4, 2026

Closes #30.

What was done

client/src/types.ts no longer exists (it was removed before M3), so the issue text was stale. The real problem was the same one it described: every result type was hand-written next to its operation, with nothing tying it to schema.graphql. That is how the GenreFacet.genre shape was wrong in M2.

  • client/codegen.ts runs the GraphQL Code Generator client preset against the repository's root schema.graphql - the same file the server reads at startup, so there is no second copy of the contract.
  • Each operation keeps its place in client/src/api/*.ts but is declared with graphql(). Result types are derived (SearchAlbumsQuery['searchAlbums'], NonNullable<AlbumDetailQuery['album']>, ...), and the component-facing names (AlbumSummary, GenreFacet, AlbumDetail) survive as aliases, so this PR does not touch components - deliberately, to keep it mergeable with Improve UI to match the original design specification #70.
  • DateTime is mapped to string; without it the generated type is unknown and the review timestamps stop typechecking.
  • Generated output is committed under client/src/generated/, and CI regenerates and fails on any diff before typecheck runs, so the message is "run pnpm codegen" rather than a type error inside generated code.
  • Test files are excluded from the document glob. Without that, CrateButton.test.tsx's fixture operation lands in the generated document map and in the production bundle.
  • ALBUM_SORTS stays a runtime array (the URL parser iterates it; the generated enum is a type union with nothing to iterate), and a new test compares it to the schema enum so a value cannot go missing.
  • pnpm build runs the generator first, which is what makes the acceptance criterion literally true.

What was tested

The acceptance criterion, run for real. Renaming averageRating to averageScore in schema.graphql:

$ pnpm build
client build: Error 0: Cannot query field "averageRating" on type "Album". Did you mean "averageScore"?
ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL @t19/client@0.0.0 build
exit 1

Restored, pnpm build succeeds: index-*.js 418.29 kB (gzip 126.37 kB), inside the 512 000-byte budget.

pnpm format:check && pnpm lint && pnpm typecheck && pnpm test passes: 108 client tests (one new), 61 server tests. pnpm codegen immediately after generation produces no diff, which is what the CI step checks.

Notes for the reviewer

  • Two new dev dependencies in client/: @graphql-codegen/cli, @graphql-codegen/client-preset, plus @graphql-typed-document-node/core which the preset's output imports and pnpm does not hoist.
  • Generated files are ignored by ESLint and Prettier. Regenerating them into Prettier's style would make every stale-file diff a formatting diff.
  • The client preset's document map is not tree-shakeable by design; the measured cost is inside the existing budget, but the bundle number above is the evidence rather than an assumption.

The client's operation types were hand-written next to each operation, so they
could drift from schema.graphql without anything failing. They already had: the
GenreFacet shape was wrong in the M2 branch.

`client/codegen.ts` runs the GraphQL Code Generator client preset against the
same root schema.graphql the server reads at startup. Each operation stays where
it is used, but is declared with `graphql()` and its result types are derived
from the generated output, so a field renamed in the contract fails typecheck.

Generated files are committed, so a checkout still builds without running the
generator; CI regenerates and fails on any difference. Two deliberate choices:
test files are excluded from the document glob (a fixture operation would land in
the production bundle), and generated files are ignored by ESLint and Prettier
because reformatting them would only make the stale-file check noisy.

`ALBUM_SORTS` stays a runtime array - it is what the URL parser validates
against, and the generated enum is a type union with nothing to iterate - so a
new test compares it with the schema enum instead.
Copy link
Member

@rachelks rachelks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Tested codegen, build and tests locally, all passing. No issues found in the review. Approved, and we can merge after #72 as planned!

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraphQL codegen for typed client operations
2 participants