Skip to content

fix: preserve full calendar dates for expanded ISO years - #898

Open
xiehuanyi wants to merge 2 commits into
fastify:mainfrom
xiehuanyi:fix/date-expanded-year-r3-20261013
Open

xiehuanyi wants to merge 2 commits into
fastify:mainfrom
xiehuanyi:fix/date-expanded-year-r3-20261013

Conversation

@xiehuanyi

Copy link
Copy Markdown

Serializing a valid JavaScript Date in year 10000 with format: "date" currently returns "+010000-02", dropping the day. Negative expanded years are truncated in the same way. Trim the fixed ISO time suffix so the complete calendar date is preserved, while four-digit-year output stays the same.

Validation: two added expanded-year regressions fail on the original implementation; two ordinary-year controls pass. The patched full suite passes 590 unit tests with 100% coverage, 15 TypeScript assertions, and ESLint.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 18:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gurgunday gurgunday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Same bug in asTime lib/serializer.js:122 should be fixed
  2. Tests belong in test/date.test.js

@xiehuanyi

Copy link
Copy Markdown
Author

Addressed both requests in 24c39ea: asTime now extracts the time relative to the end of the ISO string, and the date/time regression cases are in test/date.test.js. Local validation: npm test passes (594 unit tests, 100% coverage, and 15 TypeScript assertions), and npm run lint passes.

This branch has not been deployed

No deployments
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.

3 participants