fix(client): response types, path filtering and query hooks - #43
Merged
Conversation
docs: pnpm.overrides for postcss ^8.5.18, sharp ^0.35.0, esbuild ^0.28.1. Overrides rather than 'pnpm update' because all three were transitive and pinned below the fix by their parents' ranges - postcss in particular had a vulnerable 8.4.31 alongside a patched 8.5.25. shell: dompurify ^3.4.12 (17 alerts), vite 5.4.21 -> 6.4.3, esbuild -> 0.25.x, react-router-dom 6.30.4 -> 7.18.2. vite and esbuild also needed overrides - vitest's vite-node held vite 5 even after the direct bump. Two majors here, so verified rather than assumed: 127 tests across 11 files pass and 'pnpm build' succeeds on both. react-router 7 needed no source changes - all 18 react-router-dom imports still resolve through its v7 re-export shim. The dashboard's tracked dist/ is deliberately untouched.
The status code was never read. `code` was initialised to zero and the branch meant to parse it was empty, so every response — including 200 — fell through to DefaultError and Endpoint.Responses was always empty. Nothing failed loudly. The TypeScript generator found no 2xx response, correctly typed every method `Promise<void>`, and produced a client that compiled cleanly while discarding every response body. A client whose calls all resolve to void is worse than no client: it typechecks, so nothing tells you the data is gone. Exact codes are applied before class wildcards, because map iteration order is random and a single pass would let a "2XX" overwrite an explicit "200" on some runs and not others. A key that is neither a code, a wildcard, nor "default" is now dropped rather than filed as DefaultError — treating an unparseable status as the error shape is how a typo silently becomes an endpoint's error type.
package.json is assembled from a format string so its keys keep a fixed order, which means every interpolated value has to arrive escaped. None of them did. A specification whose info.description spanned more than one line — the normal case for an API that documents itself — wrote raw newlines inside a JSON string, and npm refused to parse the manifest at all. The generated client could not be installed, let alone built. The description is also reduced to its first paragraph with line breaks collapsed. npm renders this field as a single line, so the full API description belongs in the README the generator already writes.
From TypeScript 5.7 the DOM lib parameterises ArrayBufferView, and BodyInit accepts only ArrayBufferView<ArrayBuffer>. The generated cast to a bare `ArrayBuffer | ArrayBufferView` therefore widened to include SharedArrayBuffer and stopped being assignable, so every generated client failed to typecheck against a current lib.dom. BodyInit is what the field is declared as, and the branch has already proven the value is a valid body through the bodyTag test and ArrayBuffer.isView. Casting to the declared type is also version- agnostic, where naming ArrayBufferView<ArrayBuffer> would break on older TypeScript.
The comments claimed comma-separated values and repeatable occurrences. The code delivered neither: the value was wrapped whole, so `--flag a,b` was a single element named "a,b", and each occurrence replaced the last, so `--flag a --flag b` kept only b. Both forms now work and compose. Empty segments are dropped so that a trailing comma does not become an argument that silently matches nothing — which is the failure this whole class of bug produces, an option that appears to have been applied and was not.
A specification is usually larger than the API any one consumer talks to. A service that mounts an auth engine, an admin dashboard and its own domain routes publishes all three from one document, and a client generated over the whole thing buries the endpoints a caller wants under the ones it must never touch. Patterns accept a path prefix, a glob, or a trailing "/**". Prefixes match on a segment boundary, so "/identity" covers "/identity/login" and not "/identity-provider". Plain path.Match alone would not do: its "*" never crosses a separator, so "/api/*" would miss "/api/v1/models", which is the pattern everyone writes first. Pruning unreachable component schemas is the half that makes this worth having. Schemas generate a type each, so filtering endpoints alone leaves a types file that is mostly unreachable from the client's own surface — the endpoints look filtered while the types plainly are not. Reachability follows $ref through properties, items, the polymorphic combinators, additionalProperties and discriminator mappings, and stops on revisit so a self-referential schema terminates. A filter matching nothing is an error rather than an empty client. A mistyped pattern otherwise yields a package that builds, publishes and calls nothing.
Both are repeatable and accept comma-separated lists. Flags win over .forge-client.yaml, which gains include and exclude keys so the split lives with the package it generates rather than in a shell history. The chosen patterns are echoed before generation. A client quietly missing half its endpoints looks identical to one whose server never had them, and the difference should not have to be discovered by calling something that is not there.
…orts Two things stopped a generated package from being usable inside a workspace. The client config was resolved from the Forge project root rather than the working directory. LoadClientConfig already walks upward, so starting at the working directory finds a config beside the package being generated and one at the project root; starting at the root finds only the root's, which in a workspace is the one place the file usually is not. A package carrying its own .forge-client.yaml was silently generated with defaults. Types are now imported with `import type`. Under verbatimModuleSyntax — on by default in a strict project, and not something a generated package can ask its consumer to turn off — a type imported as a value is a compile error, because the emitter is forbidden from guessing which imports to elide. The generated client could not be typechecked inside such a project at all.
Query and header parameters are emitted optional, so required ones are grouped first to avoid an optional-before-required signature. That ordering was expressed only inside the code that renders a signature, which left anything generating a *call* to these methods to reimplement it. Two implementations would drift, and the drift is silent: passing a limit where an id is expected still compiles when both are strings. Output is byte-identical — the generator suite passes unchanged.
A layer, not a second client. Each hook calls the method rest.go already produced, so the API is described once and a change to it lands in one place. Deriving the surface twice is how a hook and the method it wraps come to disagree about a parameter — which no compiler catches, because both sides were generated from a spec that never changed. Every parameter a method accepts is part of its cache key. A key that omits one serves one request's cached answer to a different request, and where an API is versioned along two time axes that failure is silent and plausible: a key carrying only the valid time returns what is true now to a caller that asked what was known then. There is a test for it. Reads become useQuery and writes become useMutation. A mutation is not keyed, because caching a write would serve a stale answer to a request whose whole purpose was to change something. The client is a parameter rather than a module singleton or a context this file invents: a generated file should not decide how an application provides its dependencies, and an explicit argument keeps the hooks usable from a test with no provider tree. react-query is a peer dependency so the hooks share the QueryClient the application already made — a second copy in the tree is a second cache no invalidation reaches — and it is only declared when hooks are actually generated.
Optional properties were declared `?: T`, and the generated methods assign possibly-undefined values into them (`signal: options?.signal`). Under exactOptionalPropertyTypes those are different types, so a client generated for a strict project produced thirty errors the moment that project compiled it — in a workspace where the package is consumed from source, which is the normal monorepo arrangement, that is every build. Optional properties are now `?: T | undefined`, and the fetch call spreads its body rather than assigning `undefined` to a field RequestInit declares optional. The alternative is every consumer turning the check off, which is how a generated client quietly stops being typechecked at all.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Conventional Commits ValidationPR Title: valid |
Contributor
Conventional Commits ValidationPR Title: valid |
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.
Six fixes and two features in the client generator, found by generating a client from a real service and then trying to use it.
Also carries
bed49bb0 fix(deps): clear the remaining npm advisories, which was already on this branch and predates the work below.The bug that motivated the rest
spec_parser.gonever read the response status code:codestayed0, so every response — including200— was filed intoDefaultErrorandEndpoint.Responseswas always empty. The TypeScript generator behaved correctly on empty input: no 2xx found, so every method was typedPromise<void>.Nothing failed. The generated client compiled, published, and discarded every response body. This affected every language the CLI generates, not just TypeScript.
Fixes
640a5af3voide46b3e5epackage.jsonwas invalid JSON wheninfo.descriptionspanned lines;npm installrefused it20de17b5tscagainst a currentlib.dom(BodyInitnarrowed in TS 5.7)239ca5235a718db1.forge-client.yamlwas ignored in a workspace; types imported as values brokeverbatimModuleSyntax2cca6a1eexactOptionalPropertyTypes— optional properties declared?: Twhile methods assign possibly-undefined values into themThree of these share a signature: a comment describing behaviour the code did not implement. None produced an error; all produced plausible output that was wrong.
Features
Path filtering (
82cdebcc,61ade033) —--include/--exclude, repeatable and comma-separated, also readable from.forge-client.yaml. Patterns accept a path prefix, a glob, or a trailing/**; prefixes match on segment boundaries, so/identitycovers/identity/loginbut not/identity-provider.Unreachable component schemas are pruned transitively, following
$refthrough properties, items, the polymorphic combinators,additionalPropertiesand discriminator mappings. That half matters as much: schemas generate a type each, so filtering endpoints alone leaves a types file that is mostly unreachable from the client's own surface.A filter matching nothing is an error rather than an empty client.
TanStack Query hooks (
41816538) — opt-in via--react-query. Each hook calls the methodrest.goalready produced rather than issuing its own request, so the API is described once. Cache keys carry every parameter; a key that omits one serves one request's cached answer to a different request, and where an API is versioned along two time axes that failure is silent and plausible. Reads becomeuseQuery, writesuseMutation; a mutation is not keyed.@tanstack/react-queryis a peer dependency, declared only when hooks are generated.78172870extracts the method parameter ordering so the hooks derive their argument order from the same place the signature does — output is byte-identical.Verification
gofmtclean--include /api/v1→ 25 methods, 180 schemas → 41 types,Promise<void>0,tsc0 errors,npm run buildESM + CJS + DTS