Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ module.exports = {
transform: {
'^.+\\.ts$': ['ts-jest'],
},
globalSetup: '<rootDir>/jest.global-setup.js',
setupFilesAfterEnv: ['<rootDir>/setup-tests.ts'],
};
6 changes: 6 additions & 0 deletions jest.global-setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Pin a non-UTC time zone for the whole suite. Parsing bugs that resolve a string
// in the system zone instead of the requested one are invisible under UTC,
// which is what CI runners default to.
module.exports = () => {
process.env.TZ = 'Europe/Moscow';
};
41 changes: 41 additions & 0 deletions src/dateTime/__tests__/regexParse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,47 @@ test.each<[string, [number, number, number, number, number, number, number]]>([
]).toEqual(expected);
});

test.each<[string, [number, number, number, number, number, number, number]]>([

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI generated

🔵 nit

Would it be worth adding a few boundary cases? Several forms flipped from isValid() === false to true through the public API, and nothing names them:

['2016-05-25 09', ...],
['2016-05-25 0908', ...],
['2016-05-25 090834.123', ...],
['2016-05-25 09:08:34[Europe/Paris]', ...],

The ISO block above does exactly this for its own grammar — lines 50–60 spell out T09, T0908, T090834 — so it'd be nice for the SQL block to read the same way.

The bracketed one is the case I'd most like to see covered. grep for [Europe/, [America/, [Asia/ across src/ returns nothing, so that branch is untested for the ISO path too — and it carries a capture group that extractISOYmdTimeAndOffset's cursor arithmetic depends on. It's not an exotic shape either: Temporal accepts '2016-05-25 09:08:34[Europe/Paris]' (I checked on Node 24 with --harmony-temporal), and RFC 9557 standardises the bracket notation — so it seems worth locking in rather than leaving implicit.

Whichever way you want these to behave is fine by me; the value is in having a test that says so.

['2016-05-25 09', [2016, 4, 25, 9, 0, 0, 0]],
['2016-05-25 09:08', [2016, 4, 25, 9, 8, 0, 0]],
['2016-05-25 0908', [2016, 4, 25, 9, 8, 0, 0]],
['2016-05-25 09:08:34', [2016, 4, 25, 9, 8, 34, 0]],
['2016-05-25 090834', [2016, 4, 25, 9, 8, 34, 0]],
['2016-05-25 09:08:34.123', [2016, 4, 25, 9, 8, 34, 123]],
['2016-05-25 090834.123', [2016, 4, 25, 9, 8, 34, 123]],
['2016-05-25 09:08:34.123456', [2016, 4, 25, 9, 8, 34, 123]],
['2016-05-25 09:08:34.000000', [2016, 4, 25, 9, 8, 34, 0]],
['2016-05-25 09:08:34,123', [2016, 4, 25, 9, 8, 34, 123]],
])('DateTime from space-separated datetime (%p)', (input, expected) => {
const dt = dateTime({input});
expect([
dt.year(),
dt.month(),
dt.date(),
dt.hour(),
dt.minute(),
dt.second(),
dt.millisecond(),
]).toEqual(expected);
});

test.each<[string, [number, number, number, number, number, number, number]]>([
['2016-05-25 09:08:34.123+06:00', [2016, 4, 25, 3, 8, 34, 123]],
['2016-05-25 09:08:34.123+06', [2016, 4, 25, 3, 8, 34, 123]],
['2016-05-25 09:08:34.123Z', [2016, 4, 25, 9, 8, 34, 123]],
])('DateTime from space-separated datetime with offset (%p)', (input, expected) => {
const dt = dateTime({input}).utc();
expect([
dt.year(),
dt.month(),
dt.date(),
dt.hour(),
dt.minute(),
dt.second(),
dt.millisecond(),
]).toEqual(expected);
});

test("DateTime from ISO doesn't accept 24:23", () => {
expect(dateTime({input: '2018-05-25T24:23'}).isValid()).toBe(false);
});
Expand Down
4 changes: 4 additions & 0 deletions src/dateTime/dateTimeUtc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ describe('DateTimeUtc', () => {
['2023-12-31T01:00', '2023-12-31T01:00:00.000Z'],
['2023-12-31T01:00Z', '2023-12-31T01:00:00.000Z'],
['2023-12-31T03:00+02:00', '2023-12-31T01:00:00.000Z'],
['2023-12-31 01:00', '2023-12-31T01:00:00.000Z'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI generated

🟠 should-fix

I don't think these four cases will actually guard the regression on CI. TZ isn't pinned anywhere — jest.config.js has no globalSetup, setup-tests.ts only does import 'dayjs/locale/ru', and .github/workflows/ci.yml runs npm run test on ubuntu-latest, where the runner defaults to UTC.

I checked by reverting regexParse.ts to main and running the new tests against the unfixed parser:

TZ failures among the 12 new cases
UTC (what CI uses) 1 — only '2016-05-25 09:08:34,123'
Europe/Moscow 4

That's expected: the bug is that the native Date fallback resolves the string in the system zone, so under UTC it lands on the right answer anyway. The single case that does fail is failing because Date rejects the comma — a syntax accident rather than the timezone bug.

Would you mind pinning a non-UTC zone for the suite?

"test": "TZ=Europe/Moscow jest"

(or a globalSetup that sets process.env.TZ). I ran the full 516-test suite under Europe/Moscow and America/New_York and it stays green, so this looks safe. Pinning UTC would have the opposite effect here.

['2023-12-31 01:00:00', '2023-12-31T01:00:00.000Z'],
['2023-12-31 01:00:00.000000', '2023-12-31T01:00:00.000Z'],
['2023-12-31 03:00:00+02', '2023-12-31T01:00:00.000Z'],
])('input option (%p)', (input, expected) => {
const date = dateTimeUtc({input}).toISOString();
expect(date).toEqual(expected);
Expand Down
16 changes: 16 additions & 0 deletions src/dateTime/regexParse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ const isoOrdinalWithTimeExtensionRegex = new RegExp(
);
const isoTimeFullRegex = new RegExp(`^${isoTimeRegex.source}$`);

// ISO 8601 specifies the use of uppercase letter T to separate the date and time.
// PostgreSQL accepts that format on input, but on output it uses a space rather than T.
// In the ISO style, the time zone is always shown as a signed numeric offset from UTC.
// https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-OUTPUT
const sqlYmdRegex = /(\d{4})-(\d\d)-(\d\d)/;
const sqlTimeRegex = RegExp(`${isoTimeBaseRegex.source}(?:${offsetRegex.source})?`);
const sqlYmdWithTimeExtensionRegex = new RegExp(`^${sqlYmdRegex.source} ${sqlTimeRegex.source}$`);

// https://datatracker.ietf.org/doc/html/rfc2822#section-4.3
const obsOffsets = {
GMT: 0,
Expand Down Expand Up @@ -340,6 +348,10 @@ export function parseISODate(s: string) {
);
}

export function parseSQLDate(s: string) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// ISO 8601 specifies the use of uppercase letter T to separate the date and time. PostgreSQL accepts that format on input, but on output it uses a space rather than T
// In the ISO style, the time zone is always shown as a signed numeric offset from UTC. 
// However, PostgreSQL accepts the full time zone name as input, not enclosed in square brackets and separated from the time by a space.
// https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-OUTPUT
const sqlYmdRegex = /(\d{4})-(\d\d)-(\d\d)/;
const sqlTimeRegex = RegExp(
    `${isoTimeBaseRegex.source}(?:${offsetRegex.source}| (${ianaRegex.source}))?`,
);
const sqlYmdWithTimeExtensionRegex = new RegExp(`^${sqlYmdRegex.source} ${sqlTimeRegex.source}$`);
const sqlTimeFullRegex = new RegExp(`^${sqlTimeRegex.source}$`);

export function parseSQLDate(s: string) {
    return parse(
        s,
        [sqlYmdWithTimeExtensionRegex, extractISOYmdTimeAndOffset],
        [sqlTimeFullRegex, extractISOTimeAndOffset],
    );
}

or

// ISO 8601 specifies the use of uppercase letter T to separate the date and time. PostgreSQL accepts that format on input, but on output it uses a space rather than T
// In the ISO style, the time zone is always shown as a signed numeric offset from UTC. 
// https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-OUTPUT
const sqlYmdRegex = /(\d{4})-(\d\d)-(\d\d)/;
const sqlTimeRegex = RegExp(`${isoTimeBaseRegex.source}(?:${offsetRegex.source})?`);
const sqlYmdWithTimeExtensionRegex = new RegExp(`^${sqlYmdRegex.source} ${sqlTimeRegex.source}$`);

export function parseSQLDate(s: string) {
    return parse(s, [sqlYmdWithTimeExtensionRegex, extractISOYmdTimeAndOffset]);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

return parse(s, [sqlYmdWithTimeExtensionRegex, extractISOYmdTimeAndOffset]);
}

export function parseRFC2822Date(s: string) {
return parse(preprocessRFC2822(s), [rfc2822, extractRfc2822]);
}
Expand All @@ -362,6 +374,10 @@ export function parseDateString(input: string) {
if (obj !== null) {
return [obj, offset] as const;
}
[obj, offset] = parseSQLDate(input);
if (obj !== null) {
return [obj, offset] as const;
}
[obj, offset] = parseRFC2822Date(input);
if (obj !== null) {
return [obj, offset] as const;
Expand Down
Loading