Skip to content

Replace moment.js in Comments and EventDatesInfo components (PR 2/4) - #8250

Open
avoinea wants to merge 4 commits into
mainfrom
replace-moment-pr2
Open

Replace moment.js in Comments and EventDatesInfo components (PR 2/4)#8250
avoinea wants to merge 4 commits into
mainfrom
replace-moment-pr2

Conversation

@avoinea

@avoinea avoinea commented May 19, 2026

Copy link
Copy Markdown
Member

Part of the ongoing effort to remove moment.js from Volto core while keep it backward compatible - refs plone/aurora#94

@avoinea
avoinea requested review from davisagli and wesleybl May 19, 2026 13:18
@davisagli davisagli added this to the 19.x.x milestone May 19, 2026
Replace moment.js usage in Comments.jsx and EventDatesInfo.jsx with
native Intl formatting via formatDate/formatRelativeDate helpers and
native Date comparisons.

- Comments.jsx: moment.format('LLLL') → formatDate, moment.fromNow() →
  formatRelativeDate. Remove injectLazyLibs(['moment']) wrapper.
- EventDatesInfo.jsx When: Replace moment.isValid/isSame/format with
  native Date validation + formatDate. Remove injectLazyLibs(['moment']).
- EventDatesInfo.jsx Recurrence: Remove moment prop, keep
  injectLazyLibs(['rrule']), use formatDate with locale from useSelector.
- datesForDisplay export preserved unchanged for backward compatibility.
- Preserve original behavior: undefined end falls back to current time,
  matching moment(undefined) behavior.
- Locale is now explicit via useSelector/intl.locale instead of implicit
  moment global locale inheritance.

@avoinea
@avoinea
avoinea force-pushed the replace-moment-pr2 branch from 6eff8fd to 4d82fce Compare May 19, 2026 14:40
Comment thread packages/volto/news/6732.feature Outdated
Co-authored-by: David Glick <david@glicksoftware.com>
import { useSelector } from 'react-redux';
import { formatDate } from '@plone/volto/helpers/Utils/Date';

export const datesForDisplay = (start, end, moment) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We still have this moment here. Will it be removed in the future? Should we note this down somewhere to remember to remove it? In an issue?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@wesleybl I think @avoinea is still working on it this week.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@wesleybl I deprecated datesForDisplay as it's not used in Volto Core anymore but maybe some add-on is using it. Thus we can safely remove it in the next major release, Volto 20.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@avoinea Is this enough to remind us that removal is necessary? Wouldn't it be better to open an issue?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@wesleybl moment js is deprecated. See also #3753

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@wesleybl @davisagli

@robgietema mentioned that if we can't completely replace moment js with vanilla JS we can replace it with https://day.js.org/.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@robgietema mentioned that if we can't completely replace moment js with vanilla JS we can replace it with https://day.js.org/.

@avoinea @robgietema Could you explain that in more detail?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@wesleybl Not for this PR.

This one is clean enough and backward compatible and can safely be merged.

But it seems that I am hitting a wall in the DatetimeWidget / TimeWidget.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd also question why to use any moment-like library, per plone/Products.CMFPlone#4115 (comment), plone/Products.CMFPlone#4115 (comment), and https://momentjs.com/docs/#/-project-status/. It doesn't make sense to me to force confusion unto the end user, as we experience on every ploneconf.org website that used it. Specifically, times of events would display in the user's timezone, not in that of the location.

Sorry to belabor the point, but I have a keen interest in all things date and time related.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also remember we already have @internationalized/date as a dependency because of react-aria-components, it helps in some situations.
https://react-aria.adobe.com/internationalized/date/

avoinea and others added 2 commits May 20, 2026 11:44
datesForDisplay is no longer used internally (When_ was refactored to
native Date API) but is kept exported for addon consumers until Volto 20.
Also removes stale moment prop from Recurrence_ type declaration.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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.

5 participants