Skip to content

refactor(datetime): refactor DateTimeFormatter #6509

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WWRS
Copy link
Contributor

@WWRS WWRS commented Mar 25, 2025

  • Fix an issue where fractional seconds are not properly formatted
  • Pull static functions out to allow for testing
  • Add support for aa-aaaaa and MMM-MMMMM to get closer to parity with Intl (though the months aren't fully functional)
  • Add value validation to "fractionalSecond"
  • In formatToParts > "year"
    • Make it stricter: Reject years that are not exactly 2 or 4 digits
    • For 2-digit years, produce the 20XX value here instead of in partsToDate
  • Rewrite partsToDate to collect all the parts and generate the final date in one step
    • Avoid doing any sorting or searching the array, which were used to handle short months: When generating a full date, short months are handled automatically
    • I think new Date(...) will be better for handling time zones
  • Remove almost all as strings and handle errors caused by the cases where they're not strings
  • Rewrite errors to match style guide
  • Pull symbolToFormatPart out to make control flow in formatToFormatParts easier to understand
  • Add a ton of tests, bringing coverage up to 100%
  • Other minor refactors

TODO:

  • Later: Add support for time zones
  • Later: Add support for eras and months

Copy link

codecov bot commented Mar 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.45%. Comparing base (96116ce) to head (1ad8d30).
Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6509      +/-   ##
==========================================
- Coverage   95.50%   95.45%   -0.05%     
==========================================
  Files         575      576       +1     
  Lines       43279    43389     +110     
  Branches     6472     6542      +70     
==========================================
+ Hits        41335    41419      +84     
- Misses       1905     1929      +24     
- Partials       39       41       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@WWRS WWRS force-pushed the refactor-datetimeformatter branch from 5dad993 to 8e67d10 Compare March 25, 2025 02:56
@WWRS WWRS force-pushed the refactor-datetimeformatter branch from 8e67d10 to 1ad8d30 Compare March 25, 2025 02:58
@timreichen
Copy link
Contributor

There are lots of different changes in this draft.
Would it be possible to split it up into multiple PRs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants