Replace moment with date-fns v3#86
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR migrates the app from moment.js and ion2-calendar to date-fns v3 with a custom Ionic calendar modal. The wrapper module centralizes date-fns utility imports, the new CalendarModal component replaces the removed ion2-calendar package, and imports across 25+ files are redirected to use the local wrapper. ChangesDate-fns v3 migration with custom calendar modal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/components/calendar-modal/calendar-modal.component.ts`:
- Around line 83-93: The confirm() handler in calendar-modal.component currently
dismisses the modal with from/to as-is, which can produce an inverted range when
fromValue > toValue; fix by converting both values using toCalendarDay (use the
resulting comparable timestamp or date) then normalize so the earlier day is
assigned to "from" and the later to "to" (swap if necessary) before calling
this.modalCtrl.dismiss({ ...from, from, to }); reference: confirm(),
isRangeMode, fromValue, toValue, toCalendarDay, modalCtrl.dismiss.
In `@src/app/util/date-fns.ts`:
- Around line 1-3: The file currently uses a CommonJS require assigned to the
dateFns variable (declare const require; const dateFns = require('date-fns');),
which prevents tree-shaking; replace that with ESM named imports for only the
utilities used (e.g., eachDayOfInterval, endOfMonth, format, getDate, getMonth,
getYear, isSameDay, isSameMonth, isSameYear, isToday, parseISO, startOfMonth) or
import those functions from their submodule paths (like 'date-fns/format') and
remove the declare/require and dateFns variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c69cba4b-622b-441c-b6d5-8aa1218a5185
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
package.jsonsrc/app/app.module.tssrc/app/components/appeal-filter/appeal-filter.component.tssrc/app/components/calendar-modal/calendar-modal.component.htmlsrc/app/components/calendar-modal/calendar-modal.component.scsssrc/app/components/calendar-modal/calendar-modal.component.tssrc/app/components/calendar-modal/calendar-modal.module.tssrc/app/components/date-dropdown/date-dropdown.component.tssrc/app/components/date-picker/date-picker.component.tssrc/app/components/date-popup/date-popup.component.tssrc/app/components/date-range-refinement-list/date-range-refinement-list.component.tssrc/app/components/request-filter/request-filter.component.tssrc/app/components/story-filter/story-filter.component.tssrc/app/components/time-picker/time-picker.component.tssrc/app/pages/logged-in/candidate-assign-form/candidate-assign-form.page.tssrc/app/pages/logged-in/candidate/candidate-assigned-history/candidate-assigned-history.page.tssrc/app/pages/logged-in/candidate/candidate-certificate-form/candidate-certificate-form.page.tssrc/app/pages/logged-in/candidate/candidate-form/candidate-form.page.tssrc/app/pages/logged-in/candidate/candidate-view/candidate-view.page.tssrc/app/pages/logged-in/candidate/candidate-work-log/log-time-manually/log-time-manually.page.tssrc/app/pages/logged-in/company/company-contract/company-contract-form/company-contract-form.page.tssrc/app/pages/logged-in/company/company-request-list/company-request-list.page.tssrc/app/pages/logged-in/company/request-interview-filter/request-interview-filter.page.tssrc/app/pages/logged-in/email-campaign/email-campaign-form/email-campaign-form.page.tssrc/app/pages/logged-in/fulltimer/fulltimer-form/fulltimer-form.page.tssrc/app/pages/logged-in/leave-request/leave-request.page.tssrc/app/pages/logged-in/leave/leave-request-form/leave-request-form.page.tssrc/app/pages/logged-in/reports/valocity/valocity.page.tssrc/app/pages/logged-in/story/job-form/job-form.page.tssrc/app/pages/logged-in/transfer/transfer-candidate-list/transfer-candidate-list.page.tssrc/app/pages/logged-in/transfer/transfer-form/transfer-form.page.tssrc/app/pages/logged-in/transfer/transfer-list-all/transfer-list-all.page.tssrc/app/util/date-fns.ts
| declare const require: any; | ||
|
|
||
| const dateFns = require('date-fns'); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For date-fns v3, do CommonJS require('date-fns') imports preserve tree-shaking in webpack/Angular builds, or are ESM named imports required for optimal bundle size?
💡 Result:
For date-fns v3, using CommonJS require imports does not preserve tree-shaking in webpack or Angular builds [1][2]. To achieve optimal bundle size, you must use ESM named imports [3][4]. In date-fns v3, the library is a dual-package that supports both CommonJS and ESM [1][2]. However, tree-shaking—the process by which bundlers like webpack remove unused code—relies on the static structure of ES modules [5][6]. When you use CommonJS require statements, the bundler cannot statically analyze the dependency graph to safely eliminate unused functions, often resulting in the entire library (or large portions of it) being included in your final bundle [7][6][3]. To ensure optimal bundle size: 1. Use ESM named imports: Import functions directly from their specific submodules [3][4]. For example, use import { format } from 'date-fns/format'; instead of import { format } from 'date-fns'; [1][2]. 2. Avoid wildcard imports: Never use import * as dateFns from 'date-fns';, as this forces the bundler to include the entire library [3]. 3. Build Configuration: Ensure your build environment (webpack, Angular CLI) is configured for production mode (e.g., mode: 'production' in webpack or ng build --configuration production in Angular), which enables the optimizations necessary for tree-shaking to take effect [8][9]. While date-fns v3 improved its internal structure to be more compatible with modern bundlers, using ESM imports remains the standard requirement for effective tree-shaking [3][2]. If you are migrating from v2, note that v3 requires named imports even when importing from submodules (e.g., import { addDays } from 'date-fns/addDays';) [1][2].
Citations:
- 1: https://github.com/date-fns/date-fns/releases/tag/v3.0.0
- 2: https://blog.date-fns.org/v3-is-out/
- 3: https://app.studyraid.com/en/read/11943/380843/importing-specific-functions-for-optimal-bundle-size
- 4: https://date-fns.org/
- 5: Enable tree shaking for date-fns date-fns/date-fns#179
- 6: https://discourse.aurelia.io/t/treeshaking-date-fns-with-webpack-4/2837
- 7: Date-fns imports all functions and locales date-fns/date-fns#1493
- 8: https://stackoverflow.com/questions/57857096/date-fns-2-cant-get-tree-shaking-to-work
- 9: treeshaking does not work with typescript (Angular CLI: 7.1.0) date-fns/date-fns#990
🏁 Script executed:
# Check the actual file to confirm the problematic pattern
cat -n src/app/util/date-fns.tsRepository: BAWES-Universe/studenthub-staff
Length of output: 1364
Switch to ESM named imports to preserve tree-shaking and achieve bundle-size goals.
Line 3 uses CommonJS require('date-fns'), which prevents webpack/Angular from tree-shaking unused code. This pulls the entire library into the bundle, directly undermining this PR's bundle-reduction objective. Replace with ESM named imports:
import { eachDayOfInterval, endOfMonth, format, getDate, getMonth, getYear, isSameDay, isSameMonth, isSameYear, isToday, parseISO, startOfMonth } from 'date-fns';
For even better tree-shaking, import from submodules (e.g., import { format } from 'date-fns/format';).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/util/date-fns.ts` around lines 1 - 3, The file currently uses a
CommonJS require assigned to the dateFns variable (declare const require; const
dateFns = require('date-fns');), which prevents tree-shaking; replace that with
ESM named imports for only the utilities used (e.g., eachDayOfInterval,
endOfMonth, format, getDate, getMonth, getYear, isSameDay, isSameMonth,
isSameYear, isToday, parseISO, startOfMonth) or import those functions from
their submodule paths (like 'date-fns/format') and remove the declare/require
and dateFns variable.
|
Addressed the range-order review item in commit c66bcb4: the calendar modal now converts both selected dates to CalendarDay and swaps them when needed so from is always the earlier date and to is always the later date. Validation:
I also tested the suggested date-fns ESM wrapper change, but it is not safe with this project stack. A named import from date-fns fails ng build because this Angular/TypeScript version cannot parse date-fns v3 export type * declarations in node_modules/date-fns/*. Submodule ESM imports hit the same declaration path, and .js submodule specifiers are not exported by date-fns. I left the compatibility wrapper unchanged to keep the build green. |
1 participant
Closes #31.
Summary
date-fnsfrom v2 to v3 and removed directmomention2-calendar, which depended onmoment, and replaced it with a local Ionic/date-fns calendar modal for the existing single-date and range-date flowsVerification
npm run buildpassesrg -n moment|ion2-calendar src package.json package-lock.jsonreturns no matchesgit diff --checkpassesNote:
npm run lintcurrently fails on 713 pre-existing template equality violations across the app, unrelated to this change.Summary by CodeRabbit
Release Notes
New Features
Chores