-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
feat:add utc flag to date column #11740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat:add utc flag to date column #11740
Conversation
WalkthroughThis PR introduces UTC timezone support for date columns across the ORM. A new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ORM as ORM<br/>(Save/Load)
participant Driver
participant DateUtils
participant DB as Database
User->>ORM: Save Entity with date value
ORM->>Driver: preparePersistentValue()
Driver->>DateUtils: mixedDateToDateString(value, utc?)
alt utc = true
DateUtils->>DateUtils: Use UTC components<br/>(getUTCFullYear, etc.)
else utc = false (default)
DateUtils->>DateUtils: Use local components<br/>(getFullYear, etc.)
end
DateUtils-->>Driver: Formatted date string
Driver->>DB: Store formatted date
DB-->>Driver: Retrieve date string
Driver->>DateUtils: mixedDateToDateString(dateString, utc?)
DateUtils-->>Driver: Parsed date
Driver-->>ORM: prepareHydratedValue()
ORM-->>User: Date object
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/driver/sqlserver/SqlServerDriver.ts (1)
531-533: Bug: Persist path ignores utc for "date" (hydrates with UTC, persists without).On persist, "date" uses DateUtils.mixedDateToDate(value) without the UTC flag, while on hydrate it uses utc. This can produce off‑by‑one day issues around timezone offsets. Align persist with hydrate by passing utc.
Apply this fix:
@@ - } else if (columnMetadata.type === "date") { - return DateUtils.mixedDateToDate(value) + } else if (columnMetadata.type === "date") { + // honor per-column UTC semantics for date-only values + return DateUtils.mixedDateToDate(value, columnMetadata.utc)This matches the behavior implemented in other drivers in this PR and the intent of ColumnOptions.utc.
Also applies to: 579-581
🧹 Nitpick comments (2)
src/decorator/options/ColumnOptions.ts (1)
203-214: Clarify doc: effect differs for Date vs string inputs.If the entity property is a Date, UTC vs local components are used accordingly; if it’s already a "YYYY-MM-DD" string, it’s passed through unchanged. Suggest noting this to avoid confusion.
Apply this doc tweak:
/** * Indicates if date values should be stored and retrieved in UTC timezone * instead of local timezone. Only applies to "date" column type. * Default value is "false" (uses local timezone for backward compatibility). + * + * Note: When the entity property is a Date, TypeORM formats it using either + * UTC or local date components based on this flag. When it is already a + * "YYYY-MM-DD" string, the value is used as-is. * * @example * @Column({ type: "date", utc: true }) * birthDate: Date */ utc?: booleansrc/util/DateUtils.ts (1)
28-31: Use lowercasebooleantype annotation.The parameter uses
Boolean(the boxed object type) instead ofboolean(the primitive type). TypeScript conventions favor the primitive type for better type safety.Apply this diff:
static mixedDateToDateString( value: string | Date, - utc: Boolean = false, + utc: boolean = false, ): string {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/docs/entity/1-entities.md(3 hunks)src/decorator/options/ColumnOptions.ts(1 hunks)src/driver/aurora-mysql/AuroraMysqlDriver.ts(2 hunks)src/driver/cockroachdb/CockroachDriver.ts(2 hunks)src/driver/mysql/MysqlDriver.ts(2 hunks)src/driver/oracle/OracleDriver.ts(2 hunks)src/driver/postgres/PostgresDriver.ts(2 hunks)src/driver/react-native/ReactNativeDriver.ts(2 hunks)src/driver/sap/SapDriver.ts(2 hunks)src/driver/spanner/SpannerDriver.ts(2 hunks)src/driver/sqlite-abstract/AbstractSqliteDriver.ts(2 hunks)src/driver/sqlserver/SqlServerDriver.ts(1 hunks)src/metadata/ColumnMetadata.ts(2 hunks)src/util/DateUtils.ts(1 hunks)test/github-issues/11515/entity/Event.ts(1 hunks)test/github-issues/11515/issue-11515.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
src/driver/sap/SapDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/react-native/ReactNativeDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
test/github-issues/11515/issue-11515.ts (1)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)closeTestingConnections(487-499)reloadTestingDatabases(504-509)
src/driver/aurora-mysql/AuroraMysqlDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/oracle/OracleDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/postgres/PostgresDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/sqlserver/SqlServerDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
test/github-issues/11515/entity/Event.ts (2)
src/decorator/columns/PrimaryGeneratedColumn.ts (1)
PrimaryGeneratedColumn(55-119)src/decorator/columns/Column.ts (1)
Column(134-220)
src/driver/cockroachdb/CockroachDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/spanner/SpannerDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/sqlite-abstract/AbstractSqliteDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
src/driver/mysql/MysqlDriver.ts (1)
src/util/DateUtils.ts (1)
DateUtils(7-296)
🪛 Biome (2.1.2)
test/github-issues/11515/issue-11515.ts
[error] 26-26: Disallow duplicate setup and teardown hooks.
Disallow after duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (20)
src/driver/aurora-mysql/AuroraMysqlDriver.ts (1)
536-536: UTC threading for "date" looks good; verify Data API path also respects it.Changes align with other drivers. If formatOptions.castParameters !== false, values flow through DataApiDriver’s preparePersistentValue/prepareHydratedValue — please confirm that implementation uses columnMetadata.utc as well.
Also applies to: 595-595
src/driver/spanner/SpannerDriver.ts (1)
402-402: LGTM — UTC handling for date-only values is consistent.Also applies to: 437-437
src/driver/oracle/OracleDriver.ts (1)
533-538: LGTM — Oracle persist/hydrate paths now UTC-aware for "date".Also applies to: 571-571
src/driver/sap/SapDriver.ts (1)
545-545: LGTM — SAP driver properly threads utc for date-only columns.Also applies to: 587-587
src/driver/react-native/ReactNativeDriver.ts (2)
337-339: LGTM! UTC flag properly propagated in persistence path.The implementation correctly passes
columnMetadata.utctoDateUtils.mixedDateToDateString, enabling UTC-aware date formatting when persisting date-type columns.
409-410: LGTM! UTC flag properly propagated in hydration path.The implementation correctly passes
columnMetadata.utctoDateUtils.mixedDateToDateString, ensuring consistent UTC-aware date handling when hydrating date-type columns from the database.src/driver/mysql/MysqlDriver.ts (2)
614-615: LGTM! UTC support properly integrated in MySQL driver.The UTC flag is correctly threaded through the persistence path, consistent with the implementation across other drivers.
668-669: LGTM! UTC support properly integrated in hydration path.The UTC flag is correctly threaded through the hydration path, ensuring symmetry with persistence behavior.
docs/docs/entity/1-entities.md (1)
473-473: LGTM! Clear documentation of the UTC feature.The documentation accurately describes the new
utcoption, its purpose, applicability todatecolumn type, and default behavior for backward compatibility.src/driver/sqlite-abstract/AbstractSqliteDriver.ts (2)
333-334: LGTM! UTC support properly integrated in SQLite driver persistence.The implementation follows the established pattern across other drivers, correctly passing the UTC flag during date persistence.
408-409: LGTM! UTC support properly integrated in SQLite driver hydration.The implementation maintains consistency with the persistence path and other drivers.
src/metadata/ColumnMetadata.ts (2)
126-130: LGTM! UTC property properly declared with clear documentation.The property is correctly typed, initialized with a backward-compatible default, and documented with clear scope (applies to "date" column type only).
397-398: LGTM! UTC flag properly populated from column options.The constructor correctly reads the UTC flag from column options, following the established pattern for other column properties.
test/github-issues/11515/entity/Event.ts (1)
1-15: LGTM! Well-structured test entity for UTC validation.The entity provides good test coverage with both UTC-enabled (
utcDate) and default local timezone (localDate) date columns, allowing tests to verify the distinct behavior of each.test/github-issues/11515/issue-11515.ts (1)
30-47: LGTM! Test logic correctly validates UTC behavior.The test properly sets up a timezone-dependent scenario (America/New_York) and validates that:
utcDatewithutc: truestores as UTC date: "2025-06-01"localDatewithout UTC flag stores as local date: "2025-05-31" (20:00 EDT on May 31 = 00:00 UTC on June 1)This effectively demonstrates the UTC flag functionality.
src/util/DateUtils.ts (1)
33-41: LGTM! UTC conversion logic is correct.The implementation properly uses UTC date components (
getUTCFullYear,getUTCMonth,getUTCDate) when the UTC flag is true, maintaining consistent date representation regardless of the server's timezone.src/driver/cockroachdb/CockroachDriver.ts (2)
387-388: LGTM! UTC flag correctly propagated for date persistence.The change correctly passes
columnMetadata.utctoDateUtils.mixedDateToDateString, enabling UTC-aware date formatting when persisting date-type columns. Whenutc: true, dates will be formatted using UTC components; otherwise, the existing local timezone behavior is preserved.
447-448: LGTM! UTC flag correctly propagated for date hydration.The change correctly passes
columnMetadata.utctoDateUtils.mixedDateToDateString, enabling UTC-aware date formatting when hydrating date-type columns from the database. This mirrors the persistence logic on line 388 and ensures consistency in both directions.src/driver/postgres/PostgresDriver.ts (2)
653-654: LGTM! UTC flag correctly applied for date persistence.The implementation correctly passes
columnMetadata.utctoDateUtils.mixedDateToDateString()when persisting date values. Whenutc: true, this ensures UTC date components are extracted from Date objects, maintaining consistent date values across timezones.
752-753: LGTM! UTC flag symmetrically applied for date hydration.The hydration path correctly mirrors the persistence path by passing
columnMetadata.utctoDateUtils.mixedDateToDateString(). This maintains consistency between reads and writes. Note that PostgreSQL typically returns DATE columns as strings, so the UTC flag primarily ensures correctness in edge cases where the driver returns Date objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/github-issues/11515/issue-11515.ts (1)
30-47: Consider clarifying the test description.The test description states "should save the date column in UTC when the utc flag is true", but the test actually validates both UTC and local timezone behavior by checking both
utcDateandlocalDatefields. A more comprehensive description would better reflect what the test covers.Consider updating the test description:
- it("should save the date column in UTC when the utc flag is true", () => + it("should save date columns in UTC when utc flag is true and in local timezone when false", () =>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/driver/sqlserver/SqlServerDriver.ts(2 hunks)src/util/DateUtils.ts(1 hunks)test/github-issues/11515/issue-11515.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/util/DateUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/driver/sqlserver/SqlServerDriver.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/github-issues/11515/issue-11515.ts (1)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)closeTestingConnections(487-499)reloadTestingDatabases(504-509)
| process.env.TZ = originalTZ | ||
| await closeTestingConnections(connections) | ||
| }) | ||
| beforeEach(() => reloadTestingDatabases(connections)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix async handling in beforeEach hook.
The reloadTestingDatabases function returns a Promise, but the beforeEach callback neither awaits it nor returns the promise. This can cause race conditions where tests execute before the database is fully reloaded.
Apply this diff to fix the issue:
- beforeEach(() => reloadTestingDatabases(connections))
+ beforeEach(() => reloadTestingDatabases(connections))Wait, that's the same. Let me correct that - you need to return the promise:
- beforeEach(() => reloadTestingDatabases(connections))
+ beforeEach(async () => await reloadTestingDatabases(connections))Or more simply, pass the function directly since it already returns a promise:
- beforeEach(() => reloadTestingDatabases(connections))
+ beforeEach(async () => {
+ await reloadTestingDatabases(connections)
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach(() => reloadTestingDatabases(connections)) | |
| beforeEach(async () => { | |
| await reloadTestingDatabases(connections) | |
| }) |
🤖 Prompt for AI Agents
In test/github-issues/11515/issue-11515.ts around line 28, the beforeEach hook
must return the Promise from reloadTestingDatabases to ensure async completion;
change the hook so it returns the promise (e.g., beforeEach(() => { return
reloadTestingDatabases(connections); }) ) or make it async and await the call
(e.g., beforeEach(async () => await reloadTestingDatabases(connections)));
either approach ensures the test runner waits for the DB reload to finish.
commit: |
gioboa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your help @CHOIJEWON
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/functional/columns/date-utc/date-utc.ts (1)
28-28: Fix async handling inbeforeEachhook.The
reloadTestingDatabasesfunction returns a Promise, but the callback neither awaits it nor returns the promise. This can cause race conditions where tests execute before the database is fully reloaded.Apply this diff to fix the async handling:
- beforeEach(() => reloadTestingDatabases(connections)) + beforeEach(async () => { + await reloadTestingDatabases(connections) + })Or more concisely, since the function signature matches:
- beforeEach(() => reloadTestingDatabases(connections)) + beforeEach(() => reloadTestingDatabases(connections))Wait, that's identical. The simplest fix is:
- beforeEach(() => reloadTestingDatabases(connections)) + beforeEach(async () => await reloadTestingDatabases(connections))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/functional/columns/date-utc/date-utc.ts(1 hunks)test/functional/columns/date-utc/entity/Event.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/functional/columns/date-utc/entity/Event.ts (2)
src/decorator/columns/PrimaryGeneratedColumn.ts (1)
PrimaryGeneratedColumn(55-119)src/decorator/columns/Column.ts (1)
Column(134-220)
test/functional/columns/date-utc/date-utc.ts (1)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)closeTestingConnections(487-499)reloadTestingDatabases(504-509)
🔇 Additional comments (6)
test/functional/columns/date-utc/entity/Event.ts (3)
1-3: LGTM!The imports are correct and necessary for defining the test entity.
5-10: LGTM!The entity definition and primary key configuration follow TypeORM conventions correctly.
12-16: LGTM!The date column definitions correctly demonstrate the new UTC flag feature, with
localDateusing default local timezone behavior andutcDateexplicitly enabling UTC storage withutc: true.test/functional/columns/date-utc/date-utc.ts (3)
1-9: LGTM!All imports are necessary and correctly configured for the functional test.
15-21: LGTM!The
beforehook properly sets up the test environment by saving the original timezone, setting it to America/New_York for testing, and creating database connections with proper async handling.
23-26: LGTM!The
afterhook properly cleans up by restoring the original timezone and closing database connections with appropriate async handling.
| it("should save date columns in UTC when utc flag is true and in local timezone when false", () => | ||
| Promise.all( | ||
| connections.map(async (connection) => { | ||
| const event = new Event() | ||
| const testDate = new Date(Date.UTC(2025, 5, 1)) // 2025-06-01 in UTC | ||
|
|
||
| event.localDate = testDate | ||
| event.utcDate = testDate | ||
|
|
||
| const savedEvent = await connection.manager.save(event) | ||
| const result = await connection.manager.findOneBy(Event, { | ||
| id: savedEvent.id, | ||
| }) | ||
|
|
||
| // UTC flag true: should save as 2025-06-01 (UTC date) | ||
| expect(result!.utcDate).to.equal("2025-06-01") | ||
| // UTC flag false (default): should save as 2025-05-31 (local timezone) | ||
| expect(result!.localDate).to.equal("2025-05-31") | ||
| }), | ||
| )) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify type consistency in date assertions.
The test logic correctly validates the UTC vs local timezone behavior. However, there's a potential type mismatch:
- Lines 45 and 47 compare
result!.utcDateandresult!.localDate(typed asDatein the entity) directly to string literals"2025-06-01"and"2025-05-31". - This comparison will work if the database driver returns date columns as strings, but the entity definition declares these fields as
Datetype.
Ensure that:
- The database drivers actually return date columns as strings (which appears to be the case based on the test expectations), OR
- The assertions should convert the Date objects to strings for comparison, OR
- The entity field types should be
string | Dateto reflect the actual runtime behavior.
Run the following script to check how other date column tests handle assertions:
🏁 Script executed:
#!/bin/bash
# Search for similar date column tests to verify the assertion pattern
rg -n -A3 -B3 'type.*:.*"date"' test/functional --type ts | head -50
# Check how date comparisons are typically done in tests
rg -n 'expect.*Date.*to\.equal' test/functional --type ts | head -20Length of output: 1455
Add transformer to Event entity date columns to match established pattern.
The script output reveals the root cause: The Post entity uses a transformer on its date column to convert between Date objects and strings (lines 32-34), but the Event entity at lines 12 and 15 has no transformer.
When the test retrieves the date columns and compares them to string literals ("2025-06-01" and "2025-05-31"), the comparison will fail because:
result!.utcDateandresult!.localDateare typed asDatebut have no transformer- Without the transformer, they'll be returned as Date objects (or driver-specific formats), not strings
- The equality check against string literals will fail
Fix: Add the same transformer pattern used in Post.ts to the Event entity:
@Column({
type: "date",
transformer: {
from: (value: any) => new Date(value),
to: (value?: Date) => value?.toISOString(),
}
})
localDate: Date
@Column({
type: "date",
utc: true,
transformer: {
from: (value: any) => new Date(value),
to: (value?: Date) => value?.toISOString(),
}
})
utcDate: DateThis ensures date columns are consistently handled across the codebase and the test assertions will work correctly.
🤖 Prompt for AI Agents
In test/functional/columns/date-utc/date-utc.ts around lines 30 to 49, the test
expects string dates but the Event entity date columns lack the transformer used
elsewhere, so retrieved values are Date objects (or driver-specific) causing
assertion failures; update the Event entity (lines referenced in the comment:
~12 and ~15) to add the same transformer pattern as Post.ts that converts from
DB value to a Date (from: value => new Date(value)) and from Date to DB string
(to: value => value?.toISOString()), and keep the utc: true flag on the utcDate
column so saved/retrieved values match the test string comparisons.
|
I’ve updated the test directory location, but I’m not entirely sure if it’s the correct place. Please let me know if there’s anything that needs to be adjusted! |
|
@alumni @sgarner @naorpeled @pkuczynski Hi there — sorry for tagging you when you're busy. Thank you! |
|
I'm thinking that the problem is a bit more complex - the server timezone is also important (and might be different than the one the app uses) - but I'm thinking we could let the users figure it out for now and fix it later (maybe here the docs/tsdoc for I'm also not sure which database clients (maybe mssql?) already return a Date object (and also not sure how the client decides how to create the Date object). In my case (SAP) I'm getting strings either in the server's TZ or in UTC, depending which function is called. TypeORM uses the non-UTC functions for create/update/delete date columns (so this option won't work with them). Telling it to use I would still recommend using UTC everywhere just to be safe, but at least this partially solves the problem on the application side :) |
|
@CHOIJEWON one thing: this method is called for "date" not for "timestamp"/"datetime"? What's the purpose then? |
|
@alumni Thank you for your review 🙌 Regarding the scope being limited to "date" type: This implementation specifically addresses issue #11515, which reports timezone problems with the DATE column type. I intentionally Even though DATE columns only store year-month-day without time information, the conversion still has timezone issues. When The Regarding DATETIME/TIMESTAMP complexity: As you pointed out, DATETIME/TIMESTAMP columns involve additional complexities (server timezone, database function choices like This PR keeps the solution simple and focused on DATE columns where timezone handling is purely application-side :) |
2950548 to
507e421
Compare
PR Code Suggestions ✨Latest suggestions up to 8cc1ddd
Previous suggestionsSuggestions up to commit 86c6d39
Suggestions up to commit 1b560d7
Suggestions up to commit db8a2b2
Suggestions up to commit 585ec09
Suggestions up to commit f803689
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
507e421 to
f803689
Compare
src/util/DateUtils.ts
Outdated
| static mixedDateToDateString(value: string | Date): string { | ||
| static mixedDateToDateString( | ||
| value: string | Date, | ||
| utc: boolean = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
Imo we should make this an object param so if we ever need more options here, we won't need to deal with breaking changes, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delayed reply — the day is just starting here in Korea 🙂
Your feedback is completely reasonable. I’ll apply the updates and push a new commit shortly.
naorpeled
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left a nit comment, let me know what you think about it 🙏
After that's addressed I think we'll be good to merge
- Fix SQL Server driver to pass UTC flag in preparePersistentValue
Ensures persist and hydrate operations handle UTC consistently
- Fix nested after hook in test file
Move closeTestingConnections call to top-level after hook
- Use lowercase boolean type in DateUtils
Follow TypeScript best practices for primitive types
Addresses CodeRabbit AI review comments
Move tests from github-issues to test/functional/columns/date-utc following maintainer feedback. Improve test descriptions and add explanatory comments
585ec09 to
db8a2b2
Compare
Changed the `mixedDateToDateString` function signature from using a boolean parameter (`utc: boolean = false`) to an object parameter (`options?: { utc?: boolean }`) to allow
for future extensibility without breaking changes.
This approach follows industry best practices and makes it easier to add additional options in the future while maintaining backward compatibility through optional parameters.
Updated all call sites across:
- 10 database drivers (MySQL, Postgres, Oracle, SQL Server, SQLite, etc.)
- SubjectChangedColumnsComputer for column comparison logic
- Maintained backward compatibility with test files using single-parameter calls
PR Code Suggestions ✨No code suggestions found for the PR. |
|
Thank you for your valuable feedback! I've updated the implementation as you suggested. Changes Made:I've refactored the src/util/DateUtils.ts:28-32 static mixedDateToDateString(
value: string | Date,
options?: { utc?: boolean },
): string {
const utc = options?.utc ?? falseAll database drivers (MySQL, PostgreSQL, SQLite, Oracle, SQL Server, Spanner, CockroachDB, SAP HANA, React Native) have been updated to use the new parameter format: DateUtils.mixedDateToDateString(value, { utc: columnMetadata.utc }) This approach allows for potential future options to be added without requiring API changes. Could you please review the updated implementation when you have a moment? I'd greatly appreciate any additional feedback. Thank you! |
gioboa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your help @CHOIJEWON
…/CHOIJEWON/typeorm into feat/add-utc-flag-to-date-column
|
I just wanted to kindly ask if there’s anything else I should address in this PR. |
gioboa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged 💪
closes: #11515
Description of change
This PR adds UTC timezone support for date type columns by introducing a new utc option to ColumnOptions.
Currently, date columns always use local timezone when converting dates to strings, making it difficult to maintain consistent
timezone across different environments. This can cause issues in distributed systems or when the application server timezone differs from the database timezone.
With the new utc flag, developers can explicitly store and retrieve dates in UTC timezone, ensuring data consistency regardless of the server's local timezone setting.
Example Usage:
What changed
Backward Compatibility
✅ No breaking changes - utc defaults to false, maintaining existing local timezone behavior
Pull-Request Checklist
masterbranchFixes #00000Additional Notes
This is my first contribution to TypeORM and open source in general. I've done my best to follow the project's conventions and
guidelines, but I would greatly appreciate any feedback on:
Thank you for taking the time to review this PR! I'm excited to contribute to TypeORM and look forward to learning from the
maintainers' feedback. 🙏
Summary by CodeRabbit
Release Notes
New Features
utcconfiguration option available for date column types across all supported databases.Documentation