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

Update moment and moment-timezone packages to fix timezone issues #47879

Merged
merged 2 commits into from
Feb 9, 2023

Conversation

wojtekn
Copy link
Contributor

@wojtekn wojtekn commented Feb 8, 2023

What?

In this PR, I propose to update the moment and moment-timezone packages to match the versions used by WP core.

Why?

It should fix the timezone-related issues e.g. Automattic/wp-calypso#71529 and fix #47698

How?

moment-timezone package is being updated to catch with IANA time zone database changes. Last year some time zones were renamed e.g. Kiev to Kyiv (moment/moment-timezone#986). This update should fix issues for such timezone, and possibly it will fix other issues related to timezones.

Testing Instructions

  1. Set WordPress's timezone setting to Kyiv
  2. Edit or Add a new post.
  3. Open the DateTimePicker component by trying to schedule a post.
  4. Choose date and hour
  5. Save post
  6. Open the DateTimePicker component and confirm that hour displayed is the same as in the block
  7. Confirm that there is no "Moment Timezone has no data for Europe/Kyiv." JS error in the browser console

Testing Instructions for Keyboard

N/A

Screenshots or screencast

Incorrect Correct
Screenshot 2023-02-08 at 14 50 00 Screenshot 2023-02-08 at 14 50 07

@gziolo gziolo added [Package] Date /packages/date [Type] Code Quality Issues or PRs that relate to code quality labels Feb 9, 2023
@gziolo
Copy link
Member

gziolo commented Feb 9, 2023

moment is externalized in the context of WordPress so we don't have to worry about that part as it's pure for development purposes in the repository and for 3rd party consumes of npm packages. There is a slight increase in the bundle size for moment-timezone but that shouldn't be a blocker:

Before

Screenshot 2023-02-09 at 08 06 29

After

Screenshot 2023-02-09 at 08 08 49

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

I haven't tested the changes, but they look correct and all CI cheks still pass. I can confirm that the version of moment aligns now with WordPress core:

https://github.com/WordPress/wordpress-develop/blob/7104aa0a9c75b64ee0bb5e5390bb564519e3022d/package.json#L151

The only caveat is that in core, it's pinned and here we correctly allow newer versions to avoid locking 3rd party package consumers.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

Looks good, tests well too, thanks @wojtekn 👍

The bundle size change is unfortunate, but it's one more signal that we should be seeking ways to get rid of moment completely at some point. There is an open issue about it for anyone interested: #43533

@tyxla tyxla merged commit 4885b31 into WordPress:trunk Feb 9, 2023
@github-actions github-actions bot added this to the Gutenberg 15.2 milestone Feb 9, 2023
@gziolo
Copy link
Member

gziolo commented Feb 9, 2023

@Mamaduka or @ntsekouras – is it something you would be fine adding to the scope of the WordPress 6.2 release?

@Mamaduka
Copy link
Member

Mamaduka commented Feb 9, 2023

@gziolo matching the library versions makes sense to me 👍

@Mamaduka Mamaduka added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 9, 2023
@wojtekn wojtekn deleted the update/moment-to-fix-timezone-issues branch February 9, 2023 11:45
@ntsekouras
Copy link
Contributor

Cherry-picked this PR to the wp/6.2 branch.

@ntsekouras ntsekouras removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Date /packages/date [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
5 participants