Skip to content
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

chore(deps): upgrade date-fns to v4.1.0 #5326

Merged
merged 2 commits into from
Jan 20, 2025

Conversation

paolostyle
Copy link
Contributor

@paolostyle paolostyle commented Jan 20, 2025

Description

Linked issue: #5177, #5128

Problem
This PR update date-fns to the latest version, v4.1.0. Dependabot update breaks the docs website (which is a separate topic in itself, the build system over there is extremely outdated - happy to help with that), my changes fix these issues so the website is still functional.

Changes

  • bumped version in package.json
  • removed the logic in Rollup that marks date-fns exports as external
  • adjusted imports to use date-fns directly, also in tests for consistency

To reviewers

This might seem like a bad decision, after all previous contributors were using direct date-fns exports to reduce the bundle size! This isn't actually correct after v4.1.0. Since v4, date-fns is marked as type: "module", so really any somewhat up-to-date bundle will handle tree shaking correctly (in fact this might've been the case even earlier as date-fns was providing ESM exports back in v3, too). I have a hard time imagining that people stuck on super old bundlers that don't work with ESM are keeping this library up to date.

On another note, if this was an issue, someone would likely report it already, because there was a direct date-fns import in src/calendar.tsx for a while now, so for CJS builds this would've had an impact already anyway.

As a small positive side effect, the final bundle size is slightly smaller for every produced target.

After this is merged I believe both linked issues can be closed; one is the dependabot's PR, the other contains a subset of my changes.

Contribution checklist

  • I have followed the contributing guidelines.
  • I have added sufficient test coverage for my changes.
  • I have formatted my code with Prettier and checked for linting issues with ESLint for code readability.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ This pull request was sent to the PullRequest network for review. Expert reviewers are now being matched to your request based on the code's requirements. Stay tuned!

What to expect from this code review:
  • Comments posted to any areas of potential concern or improvement.
  • Detailed feedback or actions needed to resolve issues that are found.
  • Turnaround times vary, but we aim to be swift.

@paolostyle you can click here to see the review status or cancel the code review job.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

PullRequest Breakdown

Reviewable lines of change

+ 92
- 101

76% TypeScript
10% JavaScript
7% TSX (tests)
6% TypeScript (tests)
1% JSON

Generated lines of change

+ 5
- 5

Type of change

Feature - These changes are adding a new feature or improvement to existing code.

Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.87%. Comparing base (4804646) to head (8256af8).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5326      +/-   ##
==========================================
- Coverage   96.92%   96.87%   -0.06%     
==========================================
  Files          30       30              
  Lines        3416     3358      -58     
  Branches     1414     1431      +17     
==========================================
- Hits         3311     3253      -58     
+ Misses        105      103       -2     
- Partials        0        2       +2     

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

@martijnrusschen
Copy link
Member

@paolostyle Let's give this a try. Thanks for pushing this along. The added context in the description will be helpful for future references. If you're up with helping to upgrade outdated logic feel (such as the build system) free to open a PR.

@martijnrusschen martijnrusschen merged commit dfcefaf into Hacker0x01:main Jan 20, 2025
6 checks passed
@martijnrusschen
Copy link
Member

OK, it seems we're not fully functional: https://reactdatepicker.com/

@paolostyle
Copy link
Contributor Author

paolostyle commented Jan 20, 2025

Okay I must've messed up something during my local development as it indeed does not work after I did a clean set up, more specifically the UMD build used by the docs page does not work. I will revisit this and get back to you.

That being said I wonder if the UMD build is even needed nowadays. ESM "just works" which I verified by replacing browser field in package.json with the path to ESM file. I will try to make it work, though.

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.

2 participants