Skip to content

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

Merged
tyxla merged 2 commits into
WordPress:trunkfrom
wojtekn:update/moment-to-fix-timezone-issues
Feb 9, 2023
Merged

Update moment and moment-timezone packages to fix timezone issues#47879
tyxla merged 2 commits into
WordPress:trunkfrom
wojtekn:update/moment-to-fix-timezone-issues

Conversation

@wojtekn

@wojtekn wojtekn commented Feb 8, 2023

Copy link
Copy Markdown
Contributor

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

gziolo commented Feb 9, 2023

Copy link
Copy Markdown
Member

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

@gziolo gziolo left a comment

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 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.

@tyxla tyxla left a comment

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.

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

gziolo commented Feb 9, 2023

Copy link
Copy Markdown
Member

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

@Mamaduka

Mamaduka commented Feb 9, 2023

Copy link
Copy Markdown
Member

@gziolo matching the library versions makes sense to me 👍

@wojtekn wojtekn deleted the update/moment-to-fix-timezone-issues branch February 9, 2023 11:45
@ntsekouras

Copy link
Copy Markdown
Contributor

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

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

Development

Successfully merging this pull request may close these issues.

DateTimePicker Displays an Incorrect Calendar if the user's timezone is vastly different from the WordPress one.

5 participants