Replace moment.js with date-fns v3#87
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 (2)
📝 WalkthroughWalkthroughThis PR upgrades date-fns from v2 to v3, removes moment.js dependency, and provides custom shim implementations for date utilities and calendar modal functionality via TypeScript path aliases to eliminate external calendar library dependencies. Changesmoment.js to date-fns v3 Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60bc0714bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function parseISO(value: string): Date { | ||
| return new Date(value); |
There was a problem hiding this comment.
Parse date-only ISO strings without UTC shift
parseISO now returns new Date(value), which treats 'YYYY-MM-DD' inputs as UTC midnight; in negative-offset timezones (e.g. US), formatting that Date with local getters produces the previous calendar day. This breaks the many flows that do format(parseISO(...), 'yyyy-MM-dd') for filters/forms and can send or display off-by-one dates for users west of UTC.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/util/calendar-shim.ts`:
- Around line 157-159: toDate currently calls new Date(value) which treats
date-only strings (yyyy-MM-dd from ion-datetime) as UTC midnight and causes
timezone drift; update toDate (used by toResult) to detect date-only strings
(match /^\d{4}-\d{2}-\d{2}$/) and parse them into a local Date via the Date
constructor with numeric components (new Date(year, monthIndex, day)) so the
resulting Date represents local midnight; for non-date-only strings keep the
existing fallback of new Date(value).
In `@src/app/util/date-fns-shim.ts`:
- Around line 3-5: The parseISO implementation incorrectly uses new Date(value)
which treats date-only strings as UTC; update parseISO to detect date-only ISO
strings (e.g., via /^\d{4}-\d{2}-\d{2}$/) and for those parse out year, month,
day and return new Date(year, monthIndex, day) so the resulting Date is
constructed in local time; for all other ISO inputs (datetime with time or
timezone) fall back to new Date(value) to preserve existing behavior. Ensure you
reference and modify the parseISO function accordingly and compute monthIndex as
month - 1.
- Around line 64-66: The isSameMonth function only compares the month number and
omits the year, causing dates like Dec 2025 and Dec 2026 to be considered equal;
modify isSameMonth (and use toDate) to also compare the full year (e.g., compare
toDate(left).getFullYear() === toDate(right).getFullYear() in addition to
getMonth()) so both year and month must match to return true.
🪄 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: 8126ae93-7f7b-486f-9144-d8af6719b9ad
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/app/pages/logged-in/transfer/transfer-form/transfer-form.page.tssrc/app/util/calendar-shim.tssrc/app/util/date-fns-shim.tstsconfig.json
|
Addressed the CodeRabbit review feedback in commit 62b5a12:\n\n- date-only strings are now parsed into local Date values in both shims to avoid UTC date drift\n- isSameMonth now compares both year and month\n- reran |
|
Additional verification for the linked-issue check:
|
Summary
Verification
Closes #31
Summary by CodeRabbit
Dependencies
Refactoring