[Mysql]: (Feature) Added utc to datetime config. #1082
Conversation
New configuration option added for Mysql datetime type, for date conversion to Date JS object. drizzle-team#920
There was a problem hiding this comment.
Let's just always use UTC for now and remove the utc flag from the config. We'll convert the values to UTC on insert and construct Date objects using UTC on select.
We discussed this internally and decided to do it in two steps - first, fix the ongoing issue with timezone mismatch, and then add an offset parameter that would specify the timezone in which the dates are/should be stored in the DB. The second step is outside the scope of this PR.
…UTC as the defaul config for Datetime Closes drizzle-team#920 by setting UTC as the default way to tranform the Date from sql to JS.
|
The requested changes were applied.
|
| export interface MySqlDatetimeConfig<TMode extends 'date' | 'string' = 'date' | 'string'> { | ||
| mode?: TMode; | ||
| fsp?: DatetimeFsp; | ||
| // If user specifies mode: 'date'. the utc option will be required |
There was a problem hiding this comment.
Comment left from the previous implementation
| mode?: TMode; | ||
| fsp?: DatetimeFsp; | ||
| // If user specifies mode: 'date'. the utc option will be required | ||
| export type MySqlDatetimeConfig<TMode extends "date" | "string" = "date" | "string"> = { |
There was a problem hiding this comment.
Why did you make this change?
| @@ -89,7 +92,7 @@ export class MySqlDateTimeStringBuilder<T extends ColumnBuilderBaseConfig<'strin | |||
| } | |||
| } | |||
|
|
|||
| export class MySqlDateTimeString<T extends ColumnBaseConfig<'string', 'MySqlDateTimeString'>> extends MySqlColumn<T> { | |||
| export class MySqlDateTimeString<T extends ColumnBaseConfig<'string', 'MySqlDateTimeString'>> extends MySqlColumn<T, MySqlDatetimeConfig> { | |||
There was a problem hiding this comment.
Why did you make this change?
| @@ -1967,3 +1967,46 @@ test.serial('update undefined', async (t) => { | |||
|
|
|||
| await db.execute(sql`drop table ${users}`); | |||
| }); | |||
|
|
|||
| test.serial('utc config for datetime', async (t) => { | |||
There was a problem hiding this comment.
Could you add a check to this test that verifies the actual value that's put into the DB with mode: "date"? You could use a raw select query to get the data bypassing the mapping.
| ) | ||
| `, | ||
| ); | ||
| const datesTable2 = mysqlTable('datestable', { |
There was a problem hiding this comment.
Please use dprint to format all the files you've changed.
| static readonly [entityKind]: string = 'MySqlDateTimeBuilder'; | ||
|
|
||
| constructor(name: T['name'], config: MySqlDatetimeConfig | undefined) { | ||
| constructor(name: T['name'], config: MySqlDatetimeConfig) { |
There was a problem hiding this comment.
Why did you make this change?
| @@ -35,7 +34,7 @@ export class MySqlDateTimeBuilder<T extends ColumnBuilderBaseConfig<'date', 'MyS | |||
| } | |||
| } | |||
|
|
|||
| export class MySqlDateTime<T extends ColumnBaseConfig<'date', 'MySqlDateTime'>> extends MySqlColumn<T> { | |||
| export class MySqlDateTime<T extends ColumnBaseConfig<'date', 'MySqlDateTime'>> extends MySqlColumn<T, MySqlDatetimeConfig> { | |||
There was a problem hiding this comment.
Why did you make this change?
This PR aims to address #920
utcto set how drizzle-orm should treat theDateyou pass and receive from the database.In my opinion we should evaluate to add this option to both timestamp and date data types.
Any feedback would be appreciated.