fix(fastify): omit schema keys which the spec does not define - #304
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
toSchema() always emitted querystring/params/body/headers, so keys absent from the spec were set to undefined. Fastify checks the keys with Object.hasOwn() and warns with FSTWRN001 for each undefined one, which made every route defined through toRoutes() log warnings for the parts it does not use.
This branch has not been deployed
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.
This PR proposes omitting the Fastify schema keys that a spec does not define, so
toRoutes()no longer makes Fastify logFSTWRN001for every route (fixes #167). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/447. You can sign in with your GitHub ID to claim ownership of the project.The defect
toSchema()copies all four request parts unconditionally, so a spec that defines onlyquerystill produces{ querystring, params: undefined, body: undefined, headers: undefined, response }. Fastify does not test those keys for truthiness — inlib/validation.jsit falls through toObject.hasOwn(schema, 'body')and warns withFSTWRN001for each key that is present butundefined. Every route defined throughtoRoutes()therefore logs one warning per request part it does not use, which is exactly what #167 reports.Reproduced on
mainatd816230, with the/usersGET spec fromexamples/misc/spec/zod.tsregistered on a Fastify 5.8.5 instance (toRoutes(pathMap)→fastify.route({ ...routes["/users"]["get"], handler })→fastify.ready()):The change
toSchema()now starts from{ response }and addsquerystring/params/body/headersonly when the spec actually defines them, as suggested in the issue. The same script on this branch printsschema keys emitted by toRoutes(): ["response","querystring"]andFastifyWarning count: 0, and the same holds through the built package (dist/fastify/index.js).The exported
FastifySchematype is deliberately left as it is, so this is a runtime-only change: code that readsroutes[path][method].schema.querystringkeeps compiling, and thefastify-type-provider-zodinference used inexamples/misc/fastify/zod/fastify.tsis untouched. A spec key explicitly written asundefinedis treated as not defined, which is the same behaviour Fastify would want for it.Verification
src/fastify/index.test.tsis new and covers three things: the key set thattoSchema()produces for a partial spec and for a spec that defines all four parts, that noFSTWRN001is emitted while the generated routes are registered (asserted through a spy onprocess.emitWarning), and — viafastify.inject()— that the parts a spec does define are still compiled and enforced (GET /users?page=1→ 200,GET /users→ 400). Against the unchangedtoSchema()two of these fail, includingAssertionError: expected [ 'FSTWRN001', 'FSTWRN001', …(7) ] to not include 'FSTWRN001'; with the change all four pass.npm run build -w pkgs/typed-api-specandnpm test -w pkgs/typed-api-specwere run onmainbefore the change and on this branch after it. Baseline: 39 tests in 8 files passing, withtest:lint,test:formatandtest:type-checkclean. After: 43 tests in 9 files passing, the same three checks clean, and the build unchanged — no new failures.How this was managed
We tracked this work as the story for #167 on a board imported from this repository's own issues, pull requests and milestones (303 stories), and moved it through in-progress to finished as the fix was written and verified.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com