Revive the codegen typecheck harness#3092
Open
ifeanyi-ugwu wants to merge 3 commits into
Open
Conversation
codegen-test/ was dead code: its committed types.ts referenced `graphql-scalars` type exports that never existed, and nothing ran it — no script, no workflow. It now regenerates types.ts, typechecks the output (`yarn codegen && tsc --noEmit`), and runs under `yarn test` for CI coverage. The generated file is gitignored, since a stale committed copy is what rotted here before. This guards that every scalar's codegenScalarType is valid TypeScript.
generatePositionType() returned an unparenthesized union, so arrayed
usages (`${generatePositionType()}[]`) applied `[]` to only the second
member: `[number, number] | [number, number, number][]` instead of
`([number, number] | [number, number, number])[]`. The codegenScalarType
string emitted to GraphQL Codegen consumers was therefore wrong for
MultiPoint, LineString, MultiLineString, Polygon and MultiPolygon (every
geometry whose coordinates are arrays of positions).
Typechecking the generated types.ts on its own only proves the emitted type is valid TypeScript. A precedence error in a scalar's codegen type — such as an unparenthesized `Position | Position[]` before `[]` — produces a valid but wrong type that `tsc --noEmit` accepts without complaint. Assigning each of GeoJSON's valid examples to the generated type exercises it: an arrayed geometry's coordinates stop being assignable when the union isn't grouped. A negative `@ts-expect-error` case guards the reverse, failing if the type widens enough to accept a bare Position.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Revives the dormant
codegen-test/harness to regenerate the scalar types and typecheck them in CI, and adds a value-assignment fixture proving the generated GeoJSON type accepts real geometries.The harness was dead code: its committed
types.tsreferencedgraphql-scalarstype exports that never existed, and nothing ran it. It now runsgraphql-codegen+tsc --noEmitunder the roottestscript, with the generated file gitignored so a stale copy can't recur. This guards that every scalar'scodegenScalarTypeemits valid TypeScript.A plain typecheck proves the emitted types are valid TS, not that they're correct — the GeoJSON precedence bug (#3089) produced a valid-but-wrong nested type that
tscaccepted.codegen-test/fixtures/geojson.tscloses that gap: it assigns each GeoJSON geometry to the generated type, and the arrayed geometries (MultiPoint, LineString, MultiLineString, Polygon, MultiPolygon) fail to assign if the Position union isn't parenthesized.Includes #3089's one-line fix so the branch typechecks standalone; that commit rebases out once #3089 merges. Touches the root
testscript, which changes what CI runs.