Skip to content

fix: update timestamp definition if date parser is string#244

Open
gdaybrice wants to merge 1 commit intoRobinBlomberg:masterfrom
gdaybrice:fix/timestamp-as-string
Open

fix: update timestamp definition if date parser is string#244
gdaybrice wants to merge 1 commit intoRobinBlomberg:masterfrom
gdaybrice:fix/timestamp-as-string

Conversation

@gdaybrice
Copy link

@gdaybrice gdaybrice commented Mar 6, 2025

When using --date-parser string, only the date scalar type is updated to use string, while the Timestamp type definition still includes Date. This results in inconsistent type generation where timestamp columns still show Date | string instead of just string.

Current Behavior

With --date-parser string, the generated types show:

export type Timestamp = ColumnType<string, Date | string, Date | string>;

Expected Behavior

With --date-parser string, the generated types should show:

export type Timestamp = ColumnType<string, string, string>;

Proposed Fix

In src/generator/dialects/postgres/postgres-adapter.ts, update the constructor to handle both date scalar and Timestamp definition:

constructor(options?: PostgresAdapterOptions) {
  super();
  if (options?.dateParser === DateParser.STRING) {
    // Update date scalar
    this.scalars.date = new IdentifierNode('string');
    
    // Update Timestamp definition
    this.definitions.Timestamp = new ColumnTypeNode(
      new IdentifierNode('string'),
      new IdentifierNode('string'),
      new IdentifierNode('string')
    );
  } else {
    this.scalars.date = new IdentifierNode('Timestamp');
  }
  // ... rest of constructor
}

Additional Context

  • The current implementation only updates the date scalar type but not the Timestamp definition
  • This causes inconsistency in type generation when using --date-parser string
  • The fix ensures both date-related types are properly updated to use string-only types

Fixes #243

@brianmcd
Copy link
Contributor

brianmcd commented Mar 7, 2025

I added the date-parser flag, so I'll chime in with my reasoning on why it works the way it does.

Dates and timestamps are fundamentally different concepts. Timestamps can accurately be represented in a JS Date object, but dates can't. There's some good discussion on it here: brianc/node-pg-types#50

I think a lot of people want date columns as JS strings, but timestamp columns as JS Dates, and this PR would make that combination impossible.

@gdaybrice
Copy link
Author

Thanks @brianmcd - so probably need another flag to target timestamps separately?

@kyle-mcguire
Copy link

Just wanted to voice my support for this feature addition. A new flag to define timestamps as strings would be great.

jsonArrayFrom and jsonObjectFromwill always return dates as strings since it is JSON even tho the typescript definitions says they are dates. In the past, Koskimas has suggested mapping dates to strings at the driver level to smooth over type errors and make it consistent. Adding this new flag will make that experience much more seamless.

@gdaybrice
Copy link
Author

@kyle-mcguire this is exactly why I want to enforce strings, because of jsonArrayFrom and jsonObjectFrom

@colin-h
Copy link

colin-h commented Apr 21, 2025

Just adding another +1 for a solution here (happy to have a separate flag if needed)! It would be great to be able to switch everything to strings at the driver level and have the ability to generate the equivalent types. Today I have to wrap the jsonArrayFrom and jsonObjectFrom with a DateToString utility. Added example below if anyone else finds it useful.

export type DateToString<T> = {
  [P in keyof T]: T[P] extends Date ? string : T[P]
}

export function jsonArrayFrom<O>(expr: Expression<O>) {
  const s = sql<
    Simplify<O>[]
  >`(select coalesce(json_agg(agg), '[]') from ${expr} as agg)`
  return s as RawBuilder<Simplify<DateToString<O>>[]>
}

@chrislambe
Copy link

chrislambe commented May 9, 2025

I would also love a flag to indicate that timestamp and timestamptz should be typed as strings. I ran into the same problem with jsonArrayFrom and jsonObjectFrom and chose to disable the pg-types date parsing since my schema parser library already trivially handles string <-> date conversions. As it stands I'll be manually editing the Timestamp type utility as type Timestamp = string;.

@uap-dev
Copy link

uap-dev commented May 16, 2025

Ty for this change. would love to see this merged! Eager for this fix :).

@codygordon
Copy link

codygordon commented Sep 19, 2025

FYI folks following this, looks like the custom type def feature just merged takes care of this. Confirming this works and can be achieved with this flag set:

kysely-codegen --type-mapping='{"timestamp":"string","timestamptz":"string","date":"string"}'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Date parser doesn't change the type

7 participants