chore(deps): upgrade date-fns to v4.1.0 (attempt no. 2) #5332
Conversation
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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.
There was a problem hiding this comment.
PullRequest Breakdown
Reviewable lines of change
+ 111
- 105
69% TypeScript
13% JavaScript
7% JSON
6% TSX (tests)
5% TypeScript (tests)Generated lines of change
+ 5
- 5
Type of change
Feature - These changes are adding a new feature or improvement to existing code.
paolostyle
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5332 +/- ##
==========================================
- Coverage 96.92% 96.87% -0.06%
==========================================
Files 30 30
Lines 3416 3358 -58
Branches 1427 1431 +4
==========================================
- Hits 3311 3253 -58
+ Misses 105 103 -2
- Partials 0 2 +2 ☔ View full report in Codecov by Sentry. |
| "react-dom": "ReactDOM", | ||
| clsx: "clsx", | ||
| "date-fns": "dateFns", | ||
| "@floating-ui/react": "FloatingUIReact", |
There was a problem hiding this comment.
These seem new, are these required?
There was a problem hiding this comment.
They make the UMD build work in non-bundler environments. So not strictly necessary in the context of this PR but overall I think they should be kept
2 participants
Description
Linked issue: #5177, #5128, #5326
Problem
This PR is a second attempt after #5326 to update
date-fnsto 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 (this time for real, I triple checked).Changes
date-fnsversion in package.jsonbrowserfield entirely and added more modernexportsfielddate-fnsdirectly, also in tests for consistencyTo reviewers
So it turns out that the main cause of the docs page failing after all was the
browserfield in package.json. It was pointing to the UMD build, which just wouldn't work withdate-fnsafter the update. It pointsbrowserto an ESM file exporting all the other ESM files with individual functions. ESM and UMD are not compatible with each other (oh well), so there was just no way it would work.What I did is I simply removed the
browserfield. I honestly think it was just used incorrectly until now. We could keep it (pointing to the ESM build) but ifbrowserfield is missing bundlers usemodulefield anyway, and some (namelyesbuild) have additional behavior where they can usemaininstead if imported withrequire. From what I've checked most frontend libraries nowadays tend to omit this field, but if you REALLY want to keep it, I can - just need to change it to the ESM build.I kept the changes from previous PR, namely using only main
date-fnsexport as an import source. This is part of the "fixing UMD bundle" effort which I decided to do as soon as I found out... it doesn't work at all. Or I guess it could work but it would require an enormous amount of effort. After extending theglobalsfield in Rollup config it's now doable to use it with only CDN-imported dependencies.I also added the
exportsfield which I initially thought was relevant to this. It is not, however I think it's still valuable to keep it. If you'd prefer to do it in a separate PR (I will open one tomorrow re/ docs site), I can do that, too.I think that's all.
Contribution checklist