-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: replaced moment with dayjs #6724
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughReplaced Moment.js with a centralized Day.js setup across the app; updated imports and date/time calls (format, diff, calendar, isAfter/isBefore/isSame, add/subtract, utcOffset). Added configured Day.js module and renamed Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🔇 Additional comments (2)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/lib/methods/loadNextMessages.ts (1)
34-34
: Convert Dayjs object to Date for type safety.The
ts
field expects aDate
type (as defined inILoadNextMessages
at line 17), butdayjs().add()
returns a Dayjs object. While the original Moment.js code had the same pattern (and presumably worked due to implicit handling), explicit conversion is safer.Apply this diff to add explicit Date conversion:
- ts: dayjs(lastMessage.ts).add(1, 'millisecond'), + ts: dayjs(lastMessage.ts).add(1, 'millisecond').toDate(),This matches the pattern used in
app/containers/Passcode/utils.ts
(line 9) and ensures type correctness.app/lib/dayjs/index.ts (1)
8-32
: Consider lazy-loading locales to maximize bundle size savings.Loading all 24 locales upfront increases the bundle size unnecessarily. Since the PR objective is to reduce bundle size (246 KB reported), you can achieve further savings by loading locales on demand.
Based on learnings.
Recommended approach:
- Remove all locale imports from this file.
- Dynamically import locales when
toDayJsLocale
is called inapp/i18n/index.ts
.Example refactor for
app/i18n/index.ts
:import dayjs from '../lib/dayjs'; import { toDayJsLocale } from './dayjs'; const loadLocale = async (locale: string) => { const dayjsLocale = toDayJsLocale(locale); try { await import(`dayjs/locale/${dayjsLocale}`); dayjs.locale(dayjsLocale); } catch (error) { console.warn(`Failed to load locale: ${dayjsLocale}`); } };Then call
loadLocale(locale)
in yoursetLanguage
function.This ensures only the active locale is bundled, further reducing your app size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (27)
app/containers/MessageActions/index.tsx
(3 hunks)app/containers/MessageComposer/components/Quotes/Quote.tsx
(2 hunks)app/containers/MessageSeparator.tsx
(2 hunks)app/containers/Passcode/utils.ts
(1 hunks)app/containers/SupportedVersions/useSupportedVersionMessage.ts
(2 hunks)app/containers/UIKit/DatePicker.tsx
(2 hunks)app/containers/message/Components/Attachments/Reply.tsx
(2 hunks)app/containers/message/Time.tsx
(2 hunks)app/i18n/dayjs.ts
(1 hunks)app/i18n/index.ts
(2 hunks)app/lib/dayjs/index.ts
(1 hunks)app/lib/methods/AudioManager.ts
(2 hunks)app/lib/methods/checkSupportedVersions.ts
(2 hunks)app/lib/methods/getServerInfo.ts
(2 hunks)app/lib/methods/helpers/localAuthentication.ts
(2 hunks)app/lib/methods/helpers/normalizeMessage.ts
(2 hunks)app/lib/methods/helpers/room.ts
(2 hunks)app/lib/methods/loadMessagesForRoom.ts
(2 hunks)app/lib/methods/loadNextMessages.ts
(2 hunks)app/lib/methods/loadSurroundingMessages.ts
(3 hunks)app/sagas/login.js
(2 hunks)app/views/DiscussionsView/Item.tsx
(2 hunks)app/views/InviteUsersView/index.tsx
(2 hunks)app/views/ReadReceiptView/index.tsx
(2 hunks)app/views/RoomInfoView/Timezone.tsx
(2 hunks)app/views/RoomView/index.tsx
(2 hunks)package.json
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/views/ReadReceiptView/index.tsx (1)
ios/RocketChat Watch App/Formatters/MessageFormatter.swift (1)
time
(76-87)
app/containers/MessageSeparator.tsx (1)
ios/RocketChat Watch App/Formatters/MessageFormatter.swift (1)
date
(63-74)
app/i18n/index.ts (1)
app/i18n/dayjs.ts (1)
toDayJsLocale
(21-21)
app/lib/methods/getServerInfo.ts (1)
app/sagas/login.js (1)
serverRecord
(56-56)
app/containers/message/Time.tsx (1)
ios/RocketChat Watch App/Formatters/MessageFormatter.swift (1)
time
(76-87)
app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
ios/RocketChat Watch App/Formatters/MessageFormatter.swift (1)
time
(76-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Run iOS / ios-test
🔇 Additional comments (26)
package.json (1)
65-65
: LGTM! Dependency change aligns with migration goals.The addition of
dayjs@^1.11.18
is correct and aligns with the PR's objective to replace moment.js with a lighter alternative. According to the learnings, version 1.11.18 is the latest stable release with no breaking changes.app/sagas/login.js (1)
7-7
: LGTM! Date difference calculation correctly migrated.The replacement of
moment
withdayjs
for the time difference calculation is correct. The.diff()
method has the same API in both libraries, and the 12-hour threshold logic remains unchanged.Also applies to: 58-58
app/lib/methods/helpers/normalizeMessage.ts (2)
17-17
: LGTM! Timestamp conversion correctly migrated.The replacement of
moment(att.ts).toDate()
withdayjs(att.ts).toDate()
is correct. Both libraries provide identical.toDate()
methods for converting to native JavaScript Date objects.
1-1
: All dayjs imports resolve to app/lib/dayjs; no action needed.app/lib/methods/loadMessagesForRoom.ts (1)
1-1
: LGTM! Timestamp manipulation correctly migrated.The replacement of
moment(lastMessage.ts).subtract(1, 'millisecond')
withdayjs(lastMessage.ts).subtract(1, 'millisecond')
is correct. The.subtract()
method has identical API in both libraries and doesn't require additional plugins.Also applies to: 48-48
app/containers/UIKit/DatePicker.tsx (1)
7-7
: LGTM! Date formatting correctly migrated.The replacement of
moment(newDate).format('YYYY-MM-DD')
withdayjs(newDate).format('YYYY-MM-DD')
is correct. The basic format string 'YYYY-MM-DD' is supported by both libraries without requiring additional plugins.Also applies to: 57-57
app/containers/MessageSeparator.tsx (1)
4-4
: LGTM! Date formatting correctly migrated.The replacement of
moment(ts).format('LL')
withdayjs(ts).format('LL')
is correct. Note that 'LL' is a localized format token that requires thelocalizedFormat
plugin to be loaded in the dayjs wrapper (verification requested in other file comments).Also applies to: 39-39
app/views/DiscussionsView/Item.tsx (1)
5-5
: LGTM! Time formatting correctly migrated.The replacement of
moment(item.ts).format('LT')
withdayjs(item.ts).format('LT')
is correct. Note that 'LT' is a localized format token that requires thelocalizedFormat
plugin to be loaded in the dayjs wrapper (verification requested in other file comments).Also applies to: 61-61
app/lib/methods/helpers/room.ts (1)
25-30
: Day.js calendar and localizedFormat plugins are correctly configured. The dayjs wrapper imports and extends both plugins, so calendar() and localized tokens will work as expected.app/lib/methods/loadNextMessages.ts (1)
4-4
: LGTM!The import path correctly points to the local Day.js wrapper.
app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
4-4
: LGTM!The migration from Moment.js to Day.js is correctly implemented. The
format('LT')
call for localized time formatting will work as expected if the Day.js wrapper includes thelocalizedFormat
plugin.Also applies to: 28-28
app/views/ReadReceiptView/index.tsx (1)
8-8
: LGTM!The Day.js replacement is correctly implemented. The
format()
method with the customMessage_TimeAndDateFormat
setting will work identically to Moment.js.Also applies to: 115-115
app/containers/message/Components/Attachments/Reply.tsx (1)
19-19
: LGTM!The Day.js migration is correctly implemented. The timestamp formatting logic remains functionally equivalent to the original Moment.js implementation.
Also applies to: 106-106
app/containers/Passcode/utils.ts (1)
3-3
: LGTM! Excellent use of explicit type conversion.The Day.js migration correctly uses
.toDate()
to convert the Dayjs object to a native Date object. This ensures type safety and is the recommended pattern for this migration.Also applies to: 9-9
app/containers/SupportedVersions/useSupportedVersionMessage.ts (1)
1-1
: LGTM!The Day.js replacement is correctly implemented. The
diff()
method works identically to Moment.js for calculating the difference in days.Also applies to: 27-27
app/containers/message/Time.tsx (1)
4-4
: LGTM!The Day.js migration is correctly implemented. The timestamp formatting remains functionally equivalent to the original Moment.js code.
Also applies to: 18-18
app/views/InviteUsersView/index.tsx (1)
5-5
: LGTM!The Day.js migration is correctly implemented across both expiration date formatting calls. The logic remains functionally equivalent to the original Moment.js implementation.
Also applies to: 65-65, 70-70
app/lib/methods/AudioManager.ts (1)
4-4
: LGTM!The replacement of
moment(msg.ts).valueOf()
withdayjs(msg.ts).valueOf()
is correct. Both libraries return milliseconds since epoch fromvalueOf()
, ensuring identical behavior in the timestamp comparison.Also applies to: 123-123
app/lib/methods/helpers/localAuthentication.ts (1)
6-6
: LGTM!The migration from
moment(...).diff(...)
todayjs(...).diff(...)
is correct. Thediff()
method with the 'seconds' unit behaves identically in both libraries, preserving the auto-lock timing logic.Also applies to: 149-149
app/lib/methods/loadSurroundingMessages.ts (1)
4-4
: LGTM!The migration is correct. The
subtract()
,add()
, andtoDate()
methods are fully compatible between Moment.js and Day.js, maintaining the same behavior for creating load-more timestamps.Also applies to: 30-30, 45-45
app/lib/methods/getServerInfo.ts (1)
3-3
: LGTM!The replacement of
moment(...).diff(...)
withdayjs(...).diff(...)
is correct. Thediff()
method with the 'hours' unit behaves identically in both libraries.Also applies to: 78-78
app/containers/MessageActions/index.tsx (1)
6-6
: LGTM!The migration from Moment.js to Day.js in the edit and delete permission time-diff checks is correct. The
diff()
method with the 'minutes' unit behaves identically, and the defensive null checks are preserved.Also applies to: 151-157, 182-188
app/lib/methods/checkSupportedVersions.ts (1)
4-4
: LGTM!The migration from Moment.js to Day.js in the expiration calculations is correct. The
diff()
method with 'days' and 'hours' units behaves identically in both libraries.Also applies to: 15-15, 19-19
app/views/RoomInfoView/Timezone.tsx (1)
3-3
: Approve: UTC plugin is correctly loaded in Day.js wrapperapp/i18n/dayjs.ts (1)
21-21
: LGTM! Good fallback for unmapped locales.The fallback
|| locale
ensures that if a locale key is not in the mapping, the original locale string is returned instead ofundefined
. This prevents potential runtime errors.app/views/RoomView/index.tsx (1)
1301-1309
: LGTM! Proper null handling with short-circuit evaluation.The logic correctly guards against
null
values oflastOpen
with thelastOpen &&
condition on line 1302, ensuring that date comparisons are only performed whenlastOpen
is truthy. The fallback tofalse
is appropriate.The day-level comparison on line 1306 is also correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (6)
app/lib/methods/loadMessagesForRoom.ts (1)
1-1
: Prefer.toISOString()
for consistency.The migration is correct. However, consider using
.toISOString()
instead of.toString()
for the timestamp to ensure consistency with the API format used elsewhere in the file (see line 16).Apply this diff to use
.toISOString()
:- ts: dayjs(lastMessage.ts).subtract(1, 'millisecond').toString(), + ts: dayjs(lastMessage.ts).subtract(1, 'millisecond').toISOString(),Also applies to: 48-48
app/lib/methods/checkSupportedVersions.ts (1)
15-20
: Use a single “now” to avoid micro drift between diffs (optional).Capture now once to ensure consistent comparisons.
- if (!messages?.length || !expiration || dayjs(expiration).diff(new Date(), 'days') < 0) { + const now = dayjs(); + if (!messages?.length || !expiration || dayjs(expiration).diff(now, 'days') < 0) { return; } const sortedMessages = messages.sort((a, b) => a.remainingDays - b.remainingDays); - return sortedMessages.find(({ remainingDays }) => dayjs(expiration).diff(new Date(), 'hours') <= remainingDays * 24); + return sortedMessages.find(({ remainingDays }) => dayjs(expiration).diff(now, 'hours') <= remainingDays * 24);app/views/RoomInfoView/Timezone.tsx (1)
14-15
: Verify utcOffset support and units; optional display tweak.
- Ensure the Day.js wrapper extends the utc plugin; utcOffset expects minutes.
- Optional: render offset as UTC ±HH:mm for readability (e.g., +05:30) instead of raw number.
app/i18n/index.ts (1)
5-7
: Day.js wrapper plugins and locales are correctly registered
- The wrapper imports and extends utc, timezone, calendar, relativeTime and localizedFormat, and preloads all locales.
- Only add advancedFormat or customParseFormat plugins if you’re using their specific tokens (e.g. “Do”, “Qo”, custom‐parse formats).
- Optional: unify imports to
../lib/dayjs
(omit/index
) for consistency.app/views/RoomView/index.tsx (1)
1297-1309
: Consider using the isSameOrAfter plugin for better readability.The timestamp comparison logic is functionally correct. However, the verbose pattern
dayjs(item.ts).isSame(lastOpen) || dayjs(item.ts).isAfter(lastOpen)
on lines 1303-1304 could be simplified using Day.js'sisSameOrAfter
plugin, which would make the code more concise and readable.If you choose to add the plugin, first extend Day.js in
app/lib/dayjs/index.ts
:import isSameOrAfter from 'dayjs/plugin/isSameOrAfter'; dayjs.extend(isSameOrAfter);Then simplify this code:
- showUnreadSeparator = - (lastOpen && - (dayjs(item.ts).isSame(lastOpen) || dayjs(item.ts).isAfter(lastOpen)) && - dayjs(previousItem.ts).isBefore(lastOpen)) ?? - false; + showUnreadSeparator = + (lastOpen && + dayjs(item.ts).isSameOrAfter(lastOpen) && + dayjs(previousItem.ts).isBefore(lastOpen)) ?? + false;app/lib/dayjs/index.ts (1)
1-6
: Consider adding comparison plugins for API completeness.The plugin selection is solid for basic date/time operations. However, consider adding optional comparison plugins to improve code readability across the codebase:
isSameOrAfter
/isSameOrBefore
- simplify compound comparisonsisBetween
- useful for range checkscustomParseFormat
- if parsing non-standard date formatsIf you'd like to add these, apply this diff:
import dayjs from 'dayjs'; import utc from 'dayjs/plugin/utc'; import timezone from 'dayjs/plugin/timezone'; import calendar from 'dayjs/plugin/calendar'; import relativeTime from 'dayjs/plugin/relativeTime'; import localizedFormat from 'dayjs/plugin/localizedFormat'; +import isSameOrAfter from 'dayjs/plugin/isSameOrAfter'; +import isSameOrBefore from 'dayjs/plugin/isSameOrBefore'; +import isBetween from 'dayjs/plugin/isBetween'; // ... locale imports ... dayjs.extend(utc); dayjs.extend(timezone); dayjs.extend(calendar); dayjs.extend(relativeTime); dayjs.extend(localizedFormat); +dayjs.extend(isSameOrAfter); +dayjs.extend(isSameOrBefore); +dayjs.extend(isBetween);Also applies to: 34-38
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (27)
app/containers/MessageActions/index.tsx
(3 hunks)app/containers/MessageComposer/components/Quotes/Quote.tsx
(2 hunks)app/containers/MessageSeparator.tsx
(2 hunks)app/containers/Passcode/utils.ts
(1 hunks)app/containers/SupportedVersions/useSupportedVersionMessage.ts
(2 hunks)app/containers/UIKit/DatePicker.tsx
(2 hunks)app/containers/message/Components/Attachments/Reply.tsx
(2 hunks)app/containers/message/Time.tsx
(2 hunks)app/i18n/dayjs.ts
(1 hunks)app/i18n/index.ts
(2 hunks)app/lib/dayjs/index.ts
(1 hunks)app/lib/methods/AudioManager.ts
(2 hunks)app/lib/methods/checkSupportedVersions.ts
(2 hunks)app/lib/methods/getServerInfo.ts
(2 hunks)app/lib/methods/helpers/localAuthentication.ts
(2 hunks)app/lib/methods/helpers/normalizeMessage.ts
(2 hunks)app/lib/methods/helpers/room.ts
(2 hunks)app/lib/methods/loadMessagesForRoom.ts
(2 hunks)app/lib/methods/loadNextMessages.ts
(2 hunks)app/lib/methods/loadSurroundingMessages.ts
(3 hunks)app/sagas/login.js
(2 hunks)app/views/DiscussionsView/Item.tsx
(2 hunks)app/views/InviteUsersView/index.tsx
(2 hunks)app/views/ReadReceiptView/index.tsx
(2 hunks)app/views/RoomInfoView/Timezone.tsx
(2 hunks)app/views/RoomView/index.tsx
(2 hunks)package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Run iOS / ios-test
🔇 Additional comments (22)
app/sagas/login.js (1)
7-7
: LGTM! Correct Day.js usage for time difference calculation.The
diff()
method is part of Day.js core and the usage correctly replaces the equivalent moment.js call.Also applies to: 58-58
app/lib/methods/getServerInfo.ts (1)
3-3
: LGTM! Correct Day.js usage for time difference calculation.The
diff()
method is part of Day.js core and correctly replaces the moment.js equivalent for the 12-hour update interval check.Also applies to: 78-78
app/lib/methods/helpers/normalizeMessage.ts (1)
17-17
: LGTM! Correct Day.js usage for date conversion.The
toDate()
method is part of Day.js core and correctly converts the Day.js object to a native JavaScript Date object.app/containers/SupportedVersions/useSupportedVersionMessage.ts (1)
27-27
: LGTM! Correct Day.js usage for calculating remaining days.The
diff()
method with the'days'
unit is part of Day.js core and correctly replaces the moment.js equivalent.app/lib/methods/helpers/room.ts (1)
25-30
: Day.js calendar and localizedFormat plugins are already loadedThe centralized Day.js setup (
app/lib/dayjs/index.ts
) imports and extends bothcalendar
andlocalizedFormat
, so the.calendar()
calls and locale-aware tokens work as expected.package.json (1)
65-65
: Approve dayjs upgrade; no transitive moment dependencies
Confirmed that no dependencies reference moment.js and dayjs ^1.11.18 is up-to-date.app/containers/MessageComposer/components/Quotes/Quote.tsx (1)
28-28
: No action required: localizedFormat plugin is already registered in app/lib/dayjs/index.ts.app/containers/message/Components/Attachments/Reply.tsx (1)
106-106
: Localized-format plugin is enabled; locale-aware tokens are supported
app/lib/dayjs/index.ts
imports and extends thelocalizedFormat
plugin, sodayjs().format(timeFormat)
will correctly handle tokens likeLT
,LL
,dddd
, etc.app/containers/message/Time.tsx (1)
4-4
: LGTM! Verify Day.js plugins are configured.The migration from Moment.js to Day.js is correct. However, ensure that the centralized Day.js setup at
app/lib/dayjs/index.ts
includes thelocalizedFormat
andadvancedFormat
plugins if custom or localized format strings are used intimeFormat
.Also applies to: 18-18
app/views/InviteUsersView/index.tsx (1)
5-5
: LGTM! Migration is correct.The replacement of
moment(expiration).format(timeDateFormat)
withdayjs(expiration).format(timeDateFormat)
is accurate and maintains the same behavior, assuming the Day.js wrapper includes necessary plugins for the custom format strings.Also applies to: 65-65, 70-70
app/views/ReadReceiptView/index.tsx (1)
8-8
: LGTM! Migration is correct.The replacement of Moment.js with Day.js for timestamp formatting is accurate.
Also applies to: 115-115
app/lib/methods/AudioManager.ts (1)
4-4
: LGTM! Migration is correct.The replacement of
moment(msg.ts).valueOf()
withdayjs(msg.ts).valueOf()
is accurate. Both return Unix timestamp in milliseconds, ensuring compatibility with the WatermelonDB query.Also applies to: 123-123
app/lib/methods/helpers/localAuthentication.ts (1)
6-6
: LGTM! Migration is correct.The replacement of
moment(timesync).diff(...)
withdayjs(timesync).diff(...)
is accurate. Day.js handles Unix timestamps (numbers) and Date objects correctly for diff calculations, maintaining the same behavior as Moment.js.Also applies to: 149-149
app/views/DiscussionsView/Item.tsx (1)
5-5
: LGTM! VerifylocalizedFormat
plugin is loaded.The migration is correct. However, the
'LT'
format token is a localized format that requires thelocalizedFormat
plugin in Day.js. Ensure thatapp/lib/dayjs/index.ts
imports and extends this plugin.Also applies to: 61-61
app/containers/MessageSeparator.tsx (1)
4-4
: LGTM!localizedFormat
plugin is already imported and extended in your Day.js setup.app/containers/UIKit/DatePicker.tsx (1)
57-57
: LGTM: moment → dayjs formatting kept equivalent.app/lib/methods/loadSurroundingMessages.ts (1)
30-31
: LGTM: preserves Date type with toDate() and correct 1ms boundaries.Also applies to: 45-46
app/containers/MessageActions/index.tsx (3)
6-6
: LGTM!The import of the centralized Day.js wrapper is appropriate for this migration.
178-189
: Time calculation logic is correct.The Day.js implementation correctly replicates the Moment.js behavior for calculating time differences in minutes.
148-157
: Timestamp parsing is compatible. Themessage.ts
field is declared asstring | Date
, both of which Day.js parses natively, so no further changes are required.app/views/RoomView/index.tsx (1)
11-11
: LGTM!The import of the centralized Day.js wrapper is correct and aligns with the migration strategy.
app/lib/dayjs/index.ts (1)
8-32
: LGTM!The locale imports are comprehensive and cover the major languages supported by the application. Note that these imports make the locales available for use, but the actual locale switching is handled elsewhere (likely in the i18n configuration).
ts: dayjs(lastMessage.ts).add(1, 'millisecond'), | ||
t: MessageTypeLoad.NEXT_CHUNK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ts should be a Date; missing toDate() (bug).
You’re pushing a Day.js instance instead of a Date. Align with loadSurroundingMessages and consumers’ expectations.
- ts: dayjs(lastMessage.ts).add(1, 'millisecond'),
+ ts: dayjs(lastMessage.ts).add(1, 'millisecond').toDate(),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
ts: dayjs(lastMessage.ts).add(1, 'millisecond'), | |
t: MessageTypeLoad.NEXT_CHUNK | |
ts: dayjs(lastMessage.ts).add(1, 'millisecond').toDate(), | |
t: MessageTypeLoad.NEXT_CHUNK |
🤖 Prompt for AI Agents
In app/lib/methods/loadNextMessages.ts around lines 34-35, the code is pushing a
Day.js instance for the ts field which should be a native Date; change the value
to dayjs(lastMessage.ts).add(1, 'millisecond').toDate() so ts is a Date
(matching loadSurroundingMessages and consumers' expectations).
Proposed changes
This PR migrates from moment.js to day.js because moment is no longer maintained and its they recommends using day.js instead. Additionally day.js is lightweight, which will help reduce the app size.
I also tested the bundle size after migration and found that the app size was reduced by 246 KB.

Issue(s)
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Bug Fixes
Chores