Remove schema from external schemas #14
Conversation
|
Does this pass all test in Fastify too? |
Yes. |
index.js
Outdated
| @@ -10,6 +10,10 @@ function SerializerSelector () { | |||
| } | |||
|
|
|||
| function responseSchemaCompiler (fjsOpts, { schema /* method, url, httpStatus */ }) { | |||
| if (fjsOpts.schema && schema.$id) { | |||
There was a problem hiding this comment.
Has this addition a source issue?
The use case is when the same schema is added to the compiler opts and to a route's configuration
There was a problem hiding this comment.
This happens when we use fastify.getSchema when creating a new schema. fastify/fastify#4023
Here is the same code for the schema validator. https://github.com/fastify/ajv-compiler/blob/10879404784f69aa8c5488f77622b52532a28799/index.js#L80
This PR only solves part of the problem (top level $id). Ajv solves the rest. More about it here. fastify/fastify#4023 (comment)
There was a problem hiding this comment.
Ok, I get the time to focus on it.
This change removes from the externalSchemas parameter the input schema if it is already in (or a double schema error would be thrown)
added a suggestion and I would like a dedicated test that replicates the error. As said it should be necessary to use the same schema from input and as an external schema.
Then it LGTM!
index.js
Outdated
| @@ -10,6 +10,10 @@ function SerializerSelector () { | |||
| } | |||
|
|
|||
| function responseSchemaCompiler (fjsOpts, { schema /* method, url, httpStatus */ }) { | |||
| if (fjsOpts.schema && schema.$id) { | |||
There was a problem hiding this comment.
Adding a condition to avoid unuseful delete call
| if (fjsOpts.schema && schema.$id) { | |
| if (fjsOpts.schema && schema.$id && fjsOpts.schema[schema.$id]) { |
Checklist
npm run testandnpm run benchmarkand the Code of conduct
It's not needed to pass the target schema in the external schemas.