fix: align HTTP channel parameter handling with the other protocols - #456
Open
jonaslagoni wants to merge 1 commit into
Open
fix: align HTTP channel parameter handling with the other protocols#456jonaslagoni wants to merge 1 commit into
jonaslagoni wants to merge 1 commit into
Conversation
The HTTP protocols diverged from every other channel protocol in how they
render and document parameters. Four fixes, all inside protocols/http/:
Generated client helpers now take object parameters. Ten module-private
helpers in the client's common types (applyAuth, buildUrlWithParameters,
executeWithRetry, handleHttpError, shouldRetry, ...) used positional
signatures, which the rest of the generated code -- including the HTTP
server's own common types -- does not. They are not exported from the
generated module, so this is invisible to users.
The parameter model is referenced through `type` everywhere. The context
interface rendered the type from `model.type` while the instanceof guard,
the `fromUrl` call and the reported `parameterType` used `model.name`.
Real Modelina object models have `type === name`, so nothing was broken,
but `parameterType` is a type and every other protocol reads `.type`.
HTTP functions document their parameters. Both renderers called
renderChannelJSDoc without a `parameters` array, so the generated
functions documented nothing while a NATS function documents each
argument. The single `context` object is documented as `@param
context.<field>`.
The `parameters` field is optional when every parameter is optional. An
OpenAPI operation declaring only optional query parameters forced callers
to write `{parameters: {}}`. The context and the field are now optional in
that case, defaulted through a named local so the model constructor never
receives `undefined` and the instanceof guard still narrows. AsyncAPI
channel parameters are always required, so that path is unchanged. A new
`contextOptional` field on the render carries the optionality to the
client class wrapper so it matches the function it delegates to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
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.
What
The HTTP protocols diverged from every other channel protocol in how they render and document parameters. This brings them back in line. All source changes are inside
src/codegen/generators/typescript/channels/protocols/http/.The
parameterstyping was already in sync — HTTP widens toInterface | Classand normalizes through the same sharedparameterUnionType/renderParameterNormalizationhelpers as the brokers. It was the surrounding function shape and the generated helpers that had drifted.1. Generated client helpers use object parameters
Ten helpers in the client's common types used positional signatures, which the rest of the generated code — including the HTTP server's own common types (
sendResponse({...}),handleHandlerError({...})) — does not.Also
applyQueryParams,applyTypedHeaders,calculateBackoffDelay,shouldRetry,executeWithRetry,handleOAuth2TokenFlow,handleTokenRefresh. Single-arg helpers (sleep,extractHeaders,readOptionalJsonBody,validateOAuth2Config) stay positional per the rule's exception.Not a breaking change — these are all module-private in the generated file; only the operation functions are exported.
2. The parameter model is referenced through
typeeverywhereThe context interface rendered its type from
model.typewhile theinstanceofguard, thefromUrlcall and the reportedparameterTypeusedmodel.name. Real Modelina object models havetype === nameso nothing was broken, butparameterTypeis documented as a type, the NATS client generator consumes it as one, and every other protocol reads.type.3. HTTP functions document their parameters
Both renderers called
renderChannelJSDocwithout aparametersarray, so a generated HTTP function documented nothing while a NATS function documents every argument. The singlecontextobject is documented as@param context.<field>:4.
parametersis optional when every parameter is optionalAn OpenAPI operation declaring only optional query parameters forced callers to write
{parameters: {}}. The field and the context are now optional in that case, sosearch()works. The optional path defaults through a named local (const parameterInput = context.parameters ?? {}) because the model constructor reads fields off its input and throws onundefined— and a local is also what lets TypeScript narrow theinstanceofguard.AsyncAPI is unaffected: its channel parameters are always required. A new
contextOptionalfield on the render carries the optionality to the client-class wrapper so it declares the same signature as the function it delegates to.Verification
npm run build,npm run lint:fix(incl.typecheck:test) — cleanopenapi-http+openapi-http-server— generated output type-checksgenerate:assetsprepare:prsteps were run individually rather than via the script, since itsnpm cistep trips on pre-existing runtime lockfile drift. Broker-backed runtime suites (NATS/Kafka/MQTT/AMQP) were not run — they need Docker and nothing here touches those protocols.Note for reviewers
While fixing (2) I found the
channels.spec.tsfixtures build parameter models asnew ConstrainedObjectModel('FindPetsByStatusAndCategoryParameters', undefined, {}, 'Parameter', {...})— atypethat differs from thename, which real Modelina output never produces. That artifact already shows up across the NATS/Kafka/AMQP snapshots (import {TestParameter}alongsideparameters: ParameterInterface | Parameter). I updated the one affected assertion and left a comment rather than churning every snapshot; making the fixtures realistic would be a good separate cleanup.Also out of scope here, from the same review: the NATS client class narrows
parametersto the bare class (client/protocols/nats.tsforwardsfunc.parameterTypestraight into the wrapper) while the channel function it delegates to accepts theInterface | Classunion. HTTP gets this right by reusing the context interface. Left alone at @jonaslagoni's request since it moves NATS snapshots.🤖 Generated with Claude Code