fix: unused shared schema to serializer#1496
Conversation
| } | ||
|
|
||
| Schemas.prototype.resolveRefs = function (routeSchemas) { | ||
| Schemas.prototype.resolveRefs = function (routeSchemas, dontClearId) { |
There was a problem hiding this comment.
Can you please add a bit of description of dontClearId, and why somebody should set it to true?
There was a problem hiding this comment.
That boolean needs because it resolve the shared schema when added without removing the $id that is needed by fastify-plugin.
If I remove that condition (only) this specific test fails:
fastify/test/decorator.test.js
Line 635 in cf7a096
because the $id would be removed from the object, than the same object reference would be given to children that throw a FST_ERR_SCH_MISSING_ID error.
I have tried to clone the object, but I get so many fails..
Only schemas.js should set it to true
Context:
When a shared schema is added, a pair of $id + JSON with $id is created
When a JSON schema is resolved by the validation.js during the startup, the JSON with $id is injected in the right place.
During the injection che $id field is deleted because, if a user use two or more time the same shared schema AJV will complain for "multiple equals $id".
If a user add a shared schema with a reference to another shared schema and doesn't use it, the shared shema will not resolved and the code break.
There was a problem hiding this comment.
Can you put some of this explanation in the code itself?
mcollina
left a comment
mcollina
left a comment
There was a problem hiding this comment.
LGTM, can you do a backport PR?
* fix: unused shared schema to serializer * add: details on bool check
delvedor
added
bugfix
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
3 participants
Closes #1491
This fix needs to be backported to v1.x
Checklist
npm run testandnpm run benchmark