Skip to content
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

Look ma, no momentjs! #2910

Merged
merged 40 commits into from
Jan 28, 2022
Merged

Look ma, no momentjs! #2910

merged 40 commits into from
Jan 28, 2022

Conversation

sneridagh
Copy link
Member

@sneridagh sneridagh commented Dec 13, 2021

Still in development, all the "easy" ones are done. The new FormattedI18nDate component does the magic.

@nzambello next are the "utility" functions in helpers and the Event/Recurrent ones. These are a bit more "tricky". Do we have proper tests for them? Specially the utility ones. If not, we could implement the tests, then the refactoring.

I also need someone to take over, kind of exhausted on this one, but I think it's worth the effort, the bundle size will thank us, any takers? :) @plone/volto-team

https://caniuse.com/?search=Intl.DateTimeFormat

image

@tiberiuichim
Copy link
Contributor

We'll not be able to completely remove momentjs, as it's a dependency of our Datetime widget. But we can make it lazyload. https://github.com/airbnb/react-dates#localization

@sneridagh
Copy link
Member Author

@tiberiuichim yeah a pity indeed. But as you said, if the component that uses react-dates is lazy loaded, then we are fine.

@tiberiuichim
Copy link
Contributor

@sneridagh I gave up on trying to remove momentjs. We can't get rid of it from the bundle. Ideally we wouldn't need it to display event date information, but in that area it gets tricky, as it uses rrule for recurrence and so on. Maybe this could be the subject of a sprint... but as you've noticed, it's tiring, more so since it's not really covered by unit tests.

So I've lazyloaded all occurances of momentjs, and for the date helper (which could be removed, except that there's a "format" param that uses momentjs formatting, but which is not used in Volto) we need to pass it an instance of momentjs, like parseDateTime(local, value, format, moment).

@sneridagh
Copy link
Member Author

I'm more than fine with the current shape of this PR!

@tiberiuichim
Copy link
Contributor

@sneridagh We should think about merging this soon, otherwise it will bit rot

@sneridagh
Copy link
Member Author

@tiberiuichim below 14 it should fail :( I think it's from 13 that it's included by default

@tiberiuichim
Copy link
Contributor

@sneridagh Confirmed, it fails one node 12. Advice on how to proceed?

@sneridagh
Copy link
Member Author

12 eol is 2022-04-30. I guess we can live with it in the meanwhile... any downside on the bundle size?

@tiberiuichim
Copy link
Contributor

@sneridagh I don't think it would make a difference. It would bring v12 in line with the rest of versions. I've watched the bundle size for v14 and it went down (due to offloading momentjs).

@sneridagh sneridagh merged commit 68114df into master Jan 28, 2022
@sneridagh sneridagh deleted the byebyemomentjs branch January 28, 2022 11:17
@sneridagh sneridagh mentioned this pull request Jan 28, 2022
sneridagh added a commit that referenced this pull request Feb 2, 2022
* master: (25 commits)
  Add projectId for Cypress dashboard (#3023)
  Docs: faster onboarding, identify running processes, improve internal proxy clarification (#3010)
  Pin pyOpenSSL to 21.1.0
  Back to development
  Release 14.7.0
  Prepare for release
  Fix changelog
  Look ma, no `momentjs`! (#2910)
  Back to development
  Release 14.6.0
  Prepare for release
  Fix ObjectWidget story (#3009)
  Use `volto.config.js` as dynamic configuration for addons. It adds up… (#3008)
  fix: enable url with 'underscore' char
  fix: fixed italian translations
  Back to development
  Release 14.5.0
  Prepare for release
  Chenges to lockfile not saved, apparently :/
  Fix `language-independent-field` CSS class styling (#3005)
  ...
sneridagh added a commit that referenced this pull request Feb 2, 2022
* master: (25 commits)
  Add projectId for Cypress dashboard (#3023)
  Docs: faster onboarding, identify running processes, improve internal proxy clarification (#3010)
  Pin pyOpenSSL to 21.1.0
  Back to development
  Release 14.7.0
  Prepare for release
  Fix changelog
  Look ma, no `momentjs`! (#2910)
  Back to development
  Release 14.6.0
  Prepare for release
  Fix ObjectWidget story (#3009)
  Use `volto.config.js` as dynamic configuration for addons. It adds up… (#3008)
  fix: enable url with 'underscore' char
  fix: fixed italian translations
  Back to development
  Release 14.5.0
  Prepare for release
  Chenges to lockfile not saved, apparently :/
  Fix `language-independent-field` CSS class styling (#3005)
  ...
sneridagh added a commit that referenced this pull request Feb 2, 2022
* master: (25 commits)
  Add projectId for Cypress dashboard (#3023)
  Docs: faster onboarding, identify running processes, improve internal proxy clarification (#3010)
  Pin pyOpenSSL to 21.1.0
  Back to development
  Release 14.7.0
  Prepare for release
  Fix changelog
  Look ma, no `momentjs`! (#2910)
  Back to development
  Release 14.6.0
  Prepare for release
  Fix ObjectWidget story (#3009)
  Use `volto.config.js` as dynamic configuration for addons. It adds up… (#3008)
  fix: enable url with 'underscore' char
  fix: fixed italian translations
  Back to development
  Release 14.5.0
  Prepare for release
  Chenges to lockfile not saved, apparently :/
  Fix `language-independent-field` CSS class styling (#3005)
  ...
sneridagh added a commit that referenced this pull request Feb 2, 2022
* plone6-docs: (25 commits)
  Add projectId for Cypress dashboard (#3023)
  Docs: faster onboarding, identify running processes, improve internal proxy clarification (#3010)
  Pin pyOpenSSL to 21.1.0
  Back to development
  Release 14.7.0
  Prepare for release
  Fix changelog
  Look ma, no `momentjs`! (#2910)
  Back to development
  Release 14.6.0
  Prepare for release
  Fix ObjectWidget story (#3009)
  Use `volto.config.js` as dynamic configuration for addons. It adds up… (#3008)
  fix: enable url with 'underscore' char
  fix: fixed italian translations
  Back to development
  Release 14.5.0
  Prepare for release
  Chenges to lockfile not saved, apparently :/
  Fix `language-independent-field` CSS class styling (#3005)
  ...
sneridagh added a commit that referenced this pull request Feb 2, 2022
* master: (60 commits)
  Fix the a11y violation of UrlWidget (#2944)
  Back to development
  Release 14.7.1
  Prepare for release
  Add CSS body class in Babel view. Improve marker for language independent fields in Babel view too. (#3027)
  Add projectId for Cypress dashboard (#3023)
  Docs: faster onboarding, identify running processes, improve internal proxy clarification (#3010)
  Pin pyOpenSSL to 21.1.0
  Back to development
  Release 14.7.0
  Prepare for release
  Fix changelog
  Look ma, no `momentjs`! (#2910)
  Back to development
  Release 14.6.0
  Prepare for release
  Fix ObjectWidget story (#3009)
  Use `volto.config.js` as dynamic configuration for addons. It adds up… (#3008)
  fix: enable url with 'underscore' char
  fix: fixed italian translations
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants