fix(fastify): fix issue with mercurius #53
Conversation
qlaffont
requested review from
kibertoad,
rupert-mckay and
turkerdev
as code owners
|
@kibertoad ? the repo is dead ? |
|
sorry for the delay, will review today |
src/index.ts
Outdated
| }; | ||
| } | ||
|
|
||
| return ajv.compile(schema); |
There was a problem hiding this comment.
I don't think this is correct. You are basically assuming that sometimes you are going to get an ajv schema, and hardcode falling back to using it if there are no zod-specific methods found. This is brittle, and too much hardcoding.
I don't mind a more universal approach that would allow users to pass their own fallback compilers for when zod one is not found, but it shouldn't be hardcoded
There was a problem hiding this comment.
Agree with you, but I will keep the default if no fallback is precised. Because by default ajv is used on fastify.
src/index.ts
Outdated
| import type { z, ZodAny, ZodTypeAny } from 'zod'; | ||
| import type { z, ZodAny, ZodError, ZodTypeAny } from 'zod'; | ||
| import { zodToJsonSchema } from 'zod-to-json-schema'; | ||
| import { fromZodError } from 'zod-validation-error'; |
There was a problem hiding this comment.
this change might be useful, but it seems to be unrelated to the issue with mercurius. Can you create a separate PR for that?
|
@kibertoad ready for me |
|
@kibertoad any update ? |
|
@kibertoad updated ! |
|
@kibertoad any update ? |
1 similar comment
|
@kibertoad any update ? |
5 similar comments
2 participants
ref #52