m1212e / prismabox

typebox schema generator for prisma
MIT License
38 stars 7 forks source link

Support fastify-compatible dates #14

Closed Dan-Shields closed 2 months ago

Dan-Shields commented 4 months ago

By default prismabox uses the Type.Date schema for dates. This type is not serializable by Fastify when used in a response/request schema as described in this article.

Using useJsonTypes = true gets closer to a solution, as now Dates are generated as Type.String({ format: 'date-time' }) by prismabox. Fastify can serialize this. Now it becomes a TS problem, with the following error being raised by tsc when you try to return a Date object in a response:

Types of property 'createdAt' are incompatible.
      Type 'Date' is not assignable to type 'string'.

@sinclairzx81 explains in https://github.com/fastify/fastify/discussions/3357#discussioncomment-4241449 how using Type.Unsafe<Date>({ type: 'string', format: 'date' }) is the recommended workaround.

Could this be added as a separate config option, something like fastifyDateCompat = true? Happy to work on a PR if acceptable.

m1212e commented 4 months ago

Hi, thanks for your issue. I see your point, the idea behind https://github.com/m1212e/prismabox/pull/11 was to do that more ore less :D Do you see a way to unify these? Would it make sense to implement your suggested behavior when useJsonTypes is enabled instead of the current behavior?

Dan-Shields commented 4 months ago

Ah I see, I didn't realise the idea behind useJsonTypes was for fastify support in the first place. The only reason I can think of why the creator of that PR didn't run into this issue is if they are not using typescript in their application?

Regardless, I think integrating with useJsonTypes makes sense. The problem I'm having is properly parsing the options you can pass to DateTime. In both the current master version and my version, /// @prismabox.options{format: 'date'} should set the JSON schema to use the "date" format, but there's no options merging occurring, so the output looks like:

Type.String({
    format: "date-time",
    format: "date",
    additionalProperties: false,
})

Which is invalid. Any thoughts on how to approach this? I saw @brancobruyneel tried to do some JSON parsing, but you reverted that as the options syntax isn't JSON.

brancobruyneel commented 4 months ago

Hi, the https://github.com/m1212e/prismabox/pull/11 PR I opened couple of weeks ago wasn't actually finished & well tested in my personal project @m1212e. My apologies for not letting you know sooner.

Regarding the issue @Dan-Shields mentioned with TypeScript, I encountered the same problem. In my personal project, I resolved it by using Unsafe Types to satisfy the typescript compiler. However, I'm not sure if this approach is appropriate to be used in prismabox.

 Type.Unsafe<Date>({ additionalProperties: false, type: "string", format: "date-time" }),
Dan-Shields commented 4 months ago

Thanks for your input @brancobruyneel. I've gone ahead and opened a new PR to move to using the Unsafe schemas as discussed. I don't see any reason this would cause issues with other packages that rely on useJsonTypes as it is still using a string json schema under the hood.

I've tested it locally on my WIP project and it looks to be working great.

Once we've figured out the best way to support custom formats as described above it should be good to go.

m1212e commented 2 months ago

I'll close this for now, feel free to open again if the issue persists, you have any questions or you continue to work on the PR!

anotheri commented 1 week ago

The alternative and hopefully better solution suggested in https://github.com/m1212e/prismabox/issues/25