jsonArrayFrom dates have Date type but are returned as a string

[jvandenaardweg]

Hi there,

Thanks for the time and effort on this library, really enjoy working with it on my current project!

I do however have a question. I notice the Date's retrieved by using jsonArrayFrom are returned as a string instead of a Date. The types expect i should get a Date back, but in fact i'm getting a string. Is this expected behaviour?

Due to not giving away my database architecture in the open, i've reduced it down to it's most basic form below.

Typescript sees both createdAt with a Date type. But only the createdAt from the jsonArrayFrom is actually a string, which is not what we expect.

const a = await kysely
  .selectFrom('a')
  .select((eb) => [
    jsonArrayFrom(
      eb
        .selectFrom('b')
        .select([
          'b.createdAt',
        ])
        .whereRef('b.aId', '=', 'a.id'),
    ).as('b'),
  ])
  .select(['a.createdAt'])
  .where('id', '=', id)
  .executeTakeFirst();

console.log(
  'typeof a?.createdAt',
  typeof a?.createdAt,
  a?.createdAt,
);
// typeof a?.createdAt object 2023-05-02T21:57:15.466Z
// This is a Date, correct!

a?.b.forEach((b) =>
  console.log('typeof b.createdAt', typeof b.createdAt, b.createdAt),
);
// typeof b.createdAt string 2023-05-11 14:16:36.483000
// This is a string, not correct

The types:

export type A = {
  id: Generated<number>;
  createdAt: Generated<Timestamp>;
};

export type B = {
  id: Generated<number>;
  aId: number;
  createdAt: Generated<Timestamp>;
};

Some context: i'm using Kysely with the Planetscale Dialect together with prisma-kysely to have the correct types.

edit: can confirm jsonObjectFrom is also having this same behaviour.

[koskimas]

Yep, this is expected.

[jvandenaardweg]

Ok, is it something that can be fixed? Either as a fix in this library or a workaround I can use? As it's not really type-safe here, which is a bit of a bummer

[koskimas]

JSON is JSON. There's no Date type in JSON. There's nothing we can do. The word JSON is in the name of the function, so you shouldn't be that surprised.

One option is to configure the DB driver to return dates as strings and change your types. Or define the date columns as Date | string.

[jvandenaardweg]

JSON is JSON. There's no Date type in JSON. There's nothing we can do. The word JSON is in the name of the function, so you shouldn't be that surprised.

True true, kinda forgot it only returns JSON because the types were not representing that. Would have expected the types to pick that up too. If the method could return non-JSON types as string, I think that would already fix it?

[jvandenaardweg]

I've used below workaround using $castTo and that works fine for now.

const a = await kysely
  .selectFrom('a')
  .select((eb) => [
    jsonArrayFrom(
      eb
        .selectFrom('b')
        .select(['b.createdAt'])
        .$castTo<{ createdAt: string }>() // Here
        .whereRef('b.aId', '=', 'a.id')
        .as('b'),
  ])
  .select(['a.createdAt'])
  .where('id', '=', id)
  .executeTakeFirst();

Would be great if this can be done automatically, as the jsonArrayFrom and jsonObjectFrom functions are aware of returning json, so changing a Date type to string somewhere internally would be a bit nicer if possible.

Thanks again for this awesome library! 👍

[jtmarmon]

Here's the workaround I'm using in case it helps anyone

export type CastDatesToStrings<O> = {
    [K in keyof O]: O[K] extends Date ? string : O[K];
};

// https://github.com/kysely-org/kysely/issues/482
export function typesafeJsonArrayFrom<O>(
    expr: Expression<O>,
): RawBuilder<CastDatesToStrings<Simplify<O>>[]> {
    // @ts-expect-error #TS2322
    return jsonArrayFrom(expr);
}

[ptrxyz]

I also find this devilish; it makes the linter and typescript think everything's find and than -- out of nothing -- things break terribly! :smiling_imp:

Therefore I extended the casting type from above

export type JSONValue =
    | string
    | number
    | boolean
    | null
    | { [x: string]: JSONValue }
    | Array<JSONValue>

export type CastDatesToStrings<O> = {
    [K in keyof O]: O[K] extends JSONValue
        ? O[K] // it's a JSONValue, we are good
        : O[K] extends Date // it's a Date, we need to cast it to string
        ? string
        : never // otherwise, we don't know what to do, better error out.
}

export type WithTransformedFields<T> = T extends AliasedRawBuilder<infer U, infer V>
    ? AliasedRawBuilder<CastDatesToStrings<U>, V>
    : false

// usage:

const re = await db
        .selectFrom('Profile')
        .selectAll('Profile')
        .select((eb) => {
            const arr = jsonObjectFrom(
                eb
                    .selectFrom('Department')
                    .select('Department.created')
                    .whereRef('Department.id', '=', 'Profile.departmentId')
            ).as('department')
            return [arr as WithTransformedFields<typeof arr>]
        })
        .execute()

console.log('JSON', re[0].department?.created) 
// we get proper types for `created` (string) and we would get an error for types that would not be valid JSON types.
// Further, we do not have to change anything if we add or remove fields to the query.

[koskimas]

True true, kinda forgot it only returns JSON because the types were not representing that. Would have expected the types to pick that up too. If the method could return non-JSON types as string, I think that would already fix it?

Recursively traversing the output type and transforming all non-JSON properties to JSON would be terribly slow and it would be done for all outputs of those JSON functions, not just the problematic ones. We could destroy typescript performance for everyone that uses those functions.

Since the typescript compiler is super slow, we need to constantly balance between type-safety and keeping IDEs and your compilation step fast. If I fixed this for you, a bunch of other users would come here pissed off demanding the opposite fix.

If someone is able to implement the tranformation AND is able to provide proof through comprehensive tests that the performance is good, I'd merge that PR. Remember that the solution needs to be recursive! It's not enough to only convert one level of properties since the columns might be JSON as well.

[elitan]

Here's the workaround I'm using in case it helps anyone

export type CastDatesToStrings<O> = {
    [K in keyof O]: O[K] extends Date ? string : O[K];
};

// https://github.com/kysely-org/kysely/issues/482
export function typesafeJsonArrayFrom<O>(
    expr: Expression<O>,
): RawBuilder<CastDatesToStrings<Simplify<O>>[]> {
    // @ts-expect-error #TS2322
    return jsonArrayFrom(expr);
}

What's the downside of this, and why can't Kysely adopt it natively?

[thelinuxlich]

But this isn't recursive

[shoooe]

Was also surprised by this behaviour. Maybe the middleground is to provide a recursive typesafe solution by default and an unsafe version if the performance of the former sucks? In my own use cases for example I only have 1 levels deep objects so the performance decrease is extremely worth the effort. I would expect most real world scenarios to involve 1-2 levels deep objects, but maybe others can chime in.

What do you think?

Anyway, thanks for this awesome library. By far the best solution in the space. ❤️