kristiandupont/kanel

`kanel-zod`: coerce date and timestamp types

kfirba opened this issue · 3 comments

kfirba commented

Hey @kristiandupont,

When kanel-zod encounters a date or a timestamp, it uses z.date() as its value. While this probably will work most of the time, there are scenarios where it will fail.
Generating z.date() makes the implementation driver dependent. Knex or Kysely might return a JS date object, but other libraries or connectors might choose not to and will return either a timestamp (integer) or an ISO string.

Another use case is when we want to use the zod structures to build a more complex type, that may represent a more "complex" query:

select 
id,
name,
(select jsonb_agg(jsonb_build_object('date', date, 'amount', amount)) from "SomeTable" s where pid = p.id) something
from p

Ideally, I would love to construct a structure to represent the response with zod:

const structure = p.pick({ id: true, name: true }).extend({
  something: z.array(someTable.pick({ date: true, amount: true }))
});

This won't work since the drivers don't know they should "deserialize" the nested date field.

I think we can overcome it by using type coercion with zod instead:

// instead of generating:
z.date();

// we can generate:
z.union([z.number(), z.string(), z.date()]).pipe(z.coerce.date())

As far as typing is concerned, the output of this expression is a date object, but it's more "tolerant" with the input.

What do you think?

Do you think this needs to be the new default? Because even though it's not documented, it's possible to change specific types with configuration.. This is my config for tsrange:

const generateZodSchemas = makeGenerateZodSchemas({
  getZodSchemaMetadata: defaultGetZodSchemaMetadata,
  getZodIdentifierMetadata: defaultGetZodIdentifierMetadata,
  zodTypeMap: {
    ...defaultZodTypeMap,
    "pg_catalog.tsrange": "z.custom<Range<Date>>(v => v)",
    "public.email": {
      name: "emailString",
      typeImports: [
        {
          name: "emailString",
          path: "narrow-types",
          isAbsolute: true,
          isDefault: false,
        },
      ],
    },
    "public.vector": "z.array(z.number())",
  },
});

// ...
/** @type {import('kanel').Config} */
module.exports = {
  // ...

  preRenderHooks: [generateZodSchemas],
};

I have only been updating the Zod library to suit my own specific needs so obviously there is a lot of room for improvement :-)

kfirba commented

I see.
It's hard to tell. I could of course override it to suit my needs, I was thinking it might be something more users would be interested in. It feels pretty "safe" to me as it will always yield a date type, and we know we are handling a date/timestamp column from PG.

I'll make the change locally and see if I bump into any weird issues

I am going to close this issue for now. If you feel strongly about it let me know and I will consider options in this direction. Cheers!