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

Use moment-timezone-data-webpack-plugin to optimize timezones shipped in wp/date #51519

Merged
merged 2 commits into from Jun 30, 2023

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Jun 15, 2023

Makes the @wordpress/date script 10x smaller (from 750k to 75k) by shipping timezone data only for the 2000-2030 year range, instead of the full dataset that starts somewhere in antiquity and ends in 2400.

Calypso has already been doing for a long time. This PR has a discussion about the most recent updates we did there just a few months ago.

@jsnajdr jsnajdr self-assigned this Jun 15, 2023
@jsnajdr jsnajdr added [Type] Performance Related to performance efforts [Package] Date /packages/date [Type] Build Tooling Issues or PRs related to build tooling labels Jun 15, 2023
@jsnajdr jsnajdr requested a review from gziolo June 15, 2023 06:57
@github-actions
Copy link

github-actions bot commented Jun 15, 2023

Size Change: -22.7 kB (-2%)

Total Size: 1.42 MB

Filename Size Change
build/date/index.min.js 17.8 kB -22.7 kB (-56%) 🏆
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 955 B
build/annotations/index.min.js 2.69 kB
build/api-fetch/index.min.js 2.28 kB
build/autop/index.min.js 2.1 kB
build/blob/index.min.js 451 B
build/block-directory/index.min.js 6.99 kB
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
build/block-editor/content-rtl.css 4.25 kB
build/block-editor/content.css 4.24 kB
build/block-editor/default-editor-styles-rtl.css 381 B
build/block-editor/default-editor-styles.css 381 B
build/block-editor/index.min.js 209 kB
build/block-editor/style-rtl.css 14.6 kB
build/block-editor/style.css 14.6 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 126 B
build/block-library/blocks/audio/theme.css 126 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 584 B
build/block-library/blocks/button/editor.css 582 B
build/block-library/blocks/button/style-rtl.css 624 B
build/block-library/blocks/button/style.css 623 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 409 B
build/block-library/blocks/columns/style.css 409 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/cover/style-rtl.css 1.61 kB
build/block-library/blocks/cover/style.css 1.6 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 159 B
build/block-library/blocks/details/style.css 159 B
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 126 B
build/block-library/blocks/embed/theme.css 126 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/style-rtl.css 269 B
build/block-library/blocks/file/style.css 270 B
build/block-library/blocks/file/view.min.js 312 B
build/block-library/blocks/footnotes/style-rtl.css 183 B
build/block-library/blocks/footnotes/style.css 182 B
build/block-library/blocks/freeform/editor-rtl.css 2.58 kB
build/block-library/blocks/freeform/editor.css 2.58 kB
build/block-library/blocks/gallery/editor-rtl.css 947 B
build/block-library/blocks/gallery/editor.css 952 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 336 B
build/block-library/blocks/html/editor.css 337 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/style-rtl.css 1.42 kB
build/block-library/blocks/image/style.css 1.42 kB
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/image/view-interactivity.min.js 1.44 kB
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 507 B
build/block-library/blocks/media-text/style.css 505 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 712 B
build/block-library/blocks/navigation-link/editor.css 711 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/style-rtl.css 2.21 kB
build/block-library/blocks/navigation/style.css 2.2 kB
build/block-library/blocks/navigation/view.min.js 906 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 401 B
build/block-library/blocks/page-list/editor.css 401 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 174 B
build/block-library/blocks/paragraph/editor.css 174 B
build/block-library/blocks/paragraph/style-rtl.css 279 B
build/block-library/blocks/paragraph/style.css 281 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 508 B
build/block-library/blocks/post-comments-form/style.css 508 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 319 B
build/block-library/blocks/post-featured-image/style.css 319 B
build/block-library/blocks/post-navigation-link/style-rtl.css 153 B
build/block-library/blocks/post-navigation-link/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 314 B
build/block-library/blocks/post-template/style.css 314 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 335 B
build/block-library/blocks/pullquote/style.css 335 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 288 B
build/block-library/blocks/query-pagination/style.css 284 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 450 B
build/block-library/blocks/query/editor.css 449 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 178 B
build/block-library/blocks/search/editor.css 178 B
build/block-library/blocks/search/style-rtl.css 587 B
build/block-library/blocks/search/style.css 584 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 531 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 323 B
build/block-library/blocks/shortcode/editor.css 323 B
build/block-library/blocks/site-logo/editor-rtl.css 754 B
build/block-library/blocks/site-logo/editor.css 754 B
build/block-library/blocks/site-logo/style-rtl.css 203 B
build/block-library/blocks/site-logo/style.css 203 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.43 kB
build/block-library/blocks/social-links/style.css 1.42 kB
build/block-library/blocks/spacer/editor-rtl.css 348 B
build/block-library/blocks/spacer/editor.css 348 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 433 B
build/block-library/blocks/table/editor.css 433 B
build/block-library/blocks/table/style-rtl.css 645 B
build/block-library/blocks/table/style.css 644 B
build/block-library/blocks/table/theme-rtl.css 146 B
build/block-library/blocks/table/theme.css 146 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 403 B
build/block-library/blocks/template-part/editor.css 403 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 174 B
build/block-library/blocks/video/style.css 174 B
build/block-library/blocks/video/theme-rtl.css 126 B
build/block-library/blocks/video/theme.css 126 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.1 kB
build/block-library/common.css 1.1 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 12.1 kB
build/block-library/editor.css 12.1 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/index.min.js 201 kB
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 13.7 kB
build/block-library/style.css 13.7 kB
build/block-library/theme-rtl.css 686 B
build/block-library/theme.css 691 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/blocks/index.min.js 50.9 kB
build/commands/index.min.js 14.9 kB
build/commands/style-rtl.css 827 B
build/commands/style.css 827 B
build/components/index.min.js 240 kB
build/components/style-rtl.css 11.8 kB
build/components/style.css 11.8 kB
build/compose/index.min.js 12 kB
build/core-commands/index.min.js 2.26 kB
build/core-data/index.min.js 16.1 kB
build/customize-widgets/index.min.js 12 kB
build/customize-widgets/style-rtl.css 1.46 kB
build/customize-widgets/style.css 1.45 kB
build/data-controls/index.min.js 640 B
build/data/index.min.js 8.27 kB
build/deprecated/index.min.js 451 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.63 kB
build/edit-post/classic-rtl.css 544 B
build/edit-post/classic.css 545 B
build/edit-post/index.min.js 35.2 kB
build/edit-post/style-rtl.css 7.58 kB
build/edit-post/style.css 7.57 kB
build/edit-site/index.min.js 84.4 kB
build/edit-site/style-rtl.css 12.5 kB
build/edit-site/style.css 12.5 kB
build/edit-widgets/index.min.js 16.9 kB
build/edit-widgets/style-rtl.css 4.53 kB
build/edit-widgets/style.css 4.53 kB
build/editor/index.min.js 45.4 kB
build/editor/style-rtl.css 3.58 kB
build/editor/style.css 3.58 kB
build/element/index.min.js 4.8 kB
build/escape-html/index.min.js 537 B
build/format-library/index.min.js 7.62 kB
build/format-library/style-rtl.css 554 B
build/format-library/style.css 553 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.58 kB
build/interactivity/index.min.js 10 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.64 kB
build/keycodes/index.min.js 1.84 kB
build/list-reusable-blocks/index.min.js 2.13 kB
build/list-reusable-blocks/style-rtl.css 836 B
build/list-reusable-blocks/style.css 836 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 948 B
build/plugins/index.min.js 1.77 kB
build/preferences-persistence/index.min.js 1.84 kB
build/preferences/index.min.js 1.24 kB
build/primitives/index.min.js 943 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 943 B
build/react-i18n/index.min.js 615 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.7 kB
build/reusable-blocks/index.min.js 2.38 kB
build/reusable-blocks/style-rtl.css 243 B
build/reusable-blocks/style.css 243 B
build/rich-text/index.min.js 10.9 kB
build/router/index.min.js 1.77 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.39 kB
build/style-engine/index.min.js 1.83 kB
build/token-list/index.min.js 582 B
build/url/index.min.js 3.57 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 958 B
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.15 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.02 kB

compressed-size-action

@jsnajdr
Copy link
Member Author

jsnajdr commented Jun 15, 2023

The bot reports gzipped sizes, there the improvement is not 10x, but merely 57%. The big TZ data compress well.

@jsnajdr
Copy link
Member Author

jsnajdr commented Jun 15, 2023

Performance tests also show improvement when it comes to loading the editor:

┌──────────────────────┬──────────────────────────────────────────┬───────────────┐
│       (index)        │ 1bfeb9cd1d2d123dcc5bf168317e5a90190021b7 │     trunk     │
├──────────────────────┼──────────────────────────────────────────┼───────────────┤
│    serverResponse    │               '245.22 ms'                │  '251.82 ms'  │
│      firstPaint      │               '256.56 ms'                │  '333.02 ms'  │
│   domContentLoaded   │               '682.74 ms'                │  '728.68 ms'  │
│        loaded        │               '683.54 ms'                │  '729.62 ms'  │
│ firstContentfulPaint │               '1265.06 ms'               │  '1316.5 ms'  │
│      firstBlock      │              '13108.84 ms'               │ '12927.98 ms' │

Too bad we don't report the ± variances and can't estimate how significant and non-random the changes are.

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.

This is such a great optimization, nice one @jsnajdr 👍

Could you please add some testing instructions to the PR?

From the perspective of the user interface, what compromise are we making when limiting the timezone data? Would be worth evaluating where it's actually utilized and how it would affect the end users.

Comment on lines 160 to 164
new MomentTimezoneDataPlugin( {
startYear: 2000,
endYear: 2030,
} ),
Copy link
Member

Choose a reason for hiding this comment

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

This will require us to maintain the years every now and then. Does it perhaps make sense to make these dynamic?

const currentYear = new Date().getFullYear();

and then something like:

Suggested change
new MomentTimezoneDataPlugin( {
startYear: 2000,
endYear: 2030,
} ),
startYear: currentYear - 20,
endYear: currentYear + 10,

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather keep it dumb instead of dynamic. We need to start at least in 2003, the year of WordPress first release, and in 2030, we surely won't use moment.js any more 😉

Copy link
Member

Choose a reason for hiding this comment

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

What you're saying is true, but how does it relate to the timezone config? Yes, WordPress started in 2003, but you can have a post written earlier (in 1997 for example) and that's fine. You can also schedule a post for 2040, that's perfectly fine. Perhaps we can keep the end-year dynamic at least? Otherwise, we may easily forget to update it as we get closer to it, and as much as we should remove moment.js by then, WordPress is known to maintain backwards compatibility forever. This might mean that the @wordpress/date package will remain forever, albeit unused in Gutenberg, and that would mean that this config will also remain for the time being.

Copy link
Member

Choose a reason for hiding this comment

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

I was also curious about other CPTs which may not adhere to when they were posted. For example, someone could create a historical timeline using CPTs where each event is a post and has a historical date. (Though does that even matter in the context of this webpack plugin?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, if you're creating a history site and want a post to have an exact time of a historical event, e.g., November 22, 1963, at 12:30 p.m. CST, you might run into trouble if we ship a TZ range that doesn't include year 1963. But I'm not sure what exactly will happen. For this particular date probably nothing, because it's safely outside the DST range. But if you choose a date which would be DST in 1963 but not today, or vice versa, the editor might show you a time that's off by one hour.

@sgomes
Copy link
Contributor

sgomes commented Jun 15, 2023

Too bad we don't report the ± variances and can't estimate how significant and non-random the changes are.

I definitely agree; insufficient analysis can sometimes suggest the wrong thing. That being said, the numbers shown here don't raise much suspicion to me. A massive reduction in JS to be parsed and compiled (and what matters there is uncompressed size, not gzipped size, so we're talking 1/10th) is bound to have a measurable impact on interactivity metrics. Since this JS loads as part of the critical path, I would expect that to reflect in first paint and every subsequent time-based metric.

Assuming that the existing functionality is preserved, this is an excellent interim improvement as we hopefully move towards a solution that doesn't involve shipping timezone data down the wire at all, but instead relies on the system's.

Fantastic work, @jsnajdr! 🙌

@jsnajdr
Copy link
Member Author

jsnajdr commented Jun 15, 2023

Could you please add some testing instructions to the PR?

All places that use getDate or dateI18n are potentially affected. And are supposed to show dates with the right offset when you switch your site to a certain timezone. Places like this:

Screenshot 2023-06-15 at 17 39 19

But it seems to me that post publish dates behave differently -- it shows as published at 5:48 am, no matter which timezone I choose. Even without this PR. So maybe that date has a different meaning, TZ-agnostic.

@tyxla
Copy link
Member

tyxla commented Jun 16, 2023

Could you please add some testing instructions to the PR?

All places that use getDate or dateI18n are potentially affected. And are supposed to show dates with the right offset when you switch your site to a certain timezone. Places like this:

Screenshot 2023-06-15 at 17 39 19 But it seems to me that post publish dates behave differently -- it shows as published at 5:48 am, no matter which timezone I choose. Even without this PR. So maybe that date has a different meaning, TZ-agnostic.

Yeah, that was my understanding as well, but I tried both with and without the timezones and nothing changed for my tests. It would be nice to confirm that there's an area that actually is directly affected by this, and to reflect this in testing instructions so we can ensure we understand the consequences of the PR. WDYT?

@jsnajdr
Copy link
Member Author

jsnajdr commented Jun 16, 2023

It would be nice to confirm that there's an area that actually is directly affected by this

Yes, I also have some open questions like this -- it's possible that Gutenberg itself doesn't use the bundled timezones at all. Because the date package creates a special WP_ZONE timezone, using data from server-generated inline script:

Screenshot 2023-06-16 at 10 33 11

And uses that as the default timezone when formatting dates.

But that's bad because this timezone definition is incomplete, the offset: 2 value is valid only for today, June 16, because we're in a daylight saving time right now. If I use this data to format a winter January 16 date, it will have wrong offset, the right one is offset: 1. You need a full CEST zone definition to format all dates correctly.

but you can have a post written earlier (in 1997 for example) and that's fine.

Then it will probably use the 2003 rules which is not entirely bad. The typical rules change between 1997 and 2003 is that the DST switching times have shifted to be a week later or earlier.

@tyxla
Copy link
Member

tyxla commented Jun 16, 2023

Perhaps @youknowriad could be able to help? He worked on #841 back in the day.

@youknowriad
Copy link
Contributor

Yes, I also have some open questions like this -- it's possible that Gutenberg itself doesn't use the bundled timezones at all. Because the date package creates a special WP_ZONE timezone, using data from server-generated inline script:

If I remember correctly, we do this indeed for i18n reasons. These labels come translated from the backend.

@tyxla
Copy link
Member

tyxla commented Jun 19, 2023

If I remember correctly, we do this indeed for i18n reasons.

Would be great if someone can provide more info about this if they have it at the top of their head. We'll most definitely be digging into understanding all the impact that moment.js has, since at some point we want to get rid of it completely, but for the time being, it would be nice to understand the actual utility of the timezone data. I've been testing in a few of those date components in Bulgarian and nothing seems to change when I stop loading the timezone data.

@youknowriad
Copy link
Contributor

@tyxla WordPress allows you to tweak your timezone settings, date format and time format from the "settings -> general" wp-admin page. and these changes need to be reflected in the UI and I think that's what this timezone is about.

@tyxla
Copy link
Member

tyxla commented Jun 19, 2023

@tyxla WordPress allows you to tweak your timezone settings, date format and time format from the "settings -> general" wp-admin page. and these changes need to be reflected in the UI and I think that's what this timezone is about.

Thanks for confirming, but since I knew that already and I've been tweaking those settings, the dates always appeared in Bulgarian properly for me, regardless of whether I'm loading timezone data or not. Hope that makes more sense as to where the confusion comes from.

@youknowriad
Copy link
Contributor

If by time zone data you mean moment-timezone, than yeah maybe that is useless since we always define on in WordPress but there are questions about third-party cases that don't rely on the "WP" timezone. (I hope I'm understanding this properly)

@tyxla
Copy link
Member

tyxla commented Jun 19, 2023

If by time zone data you mean moment-timezone, than yeah maybe that is useless since we always define on in WordPress

That's what I mean, yes. Thanks for confirming - it's worth knowing that we can explore NOT loading the huge moment timezones blob at all. @jsnajdr is this something you'd like to try out?

but there are questions about third-party cases that don't rely on the "WP" timezone.

I'd love to understand more about these cases - do you happen to have an example? We could benefit a lot from making the moment timezone data optional or opt-in.

@youknowriad
Copy link
Contributor

I'd love to understand more about these cases - do you happen to have an example? We could benefit a lot from making the moment timezone data optional or opt-in.

Not much to be honest, I just have very vague memories that at some point we attempted changes about how "moment" locales were handled and it broke some plugins (it was something about locales being define globally in the moment singleton or something), but to be honest, that's very far in the past. Not sure it should stop any explorations.

@tyxla
Copy link
Member

tyxla commented Jun 19, 2023

Not much to be honest, I just have very vague memories that at some point we attempted changes about how "moment" locales were handled and it broke some plugins (it was something about locales being define globally in the moment singleton or something), but to be honest, that's very far in the past. Not sure it should stop any explorations.

Sounds great!

@jsnajdr - in that case how do you feel about keeping the span minimal - like for this year only for example? Hopefully, that may keep the 3rd party plugin use case working while getting us a very solid file size improvement.

@jsnajdr
Copy link
Member Author

jsnajdr commented Jun 19, 2023

I've been testing in a few of those date components in Bulgarian and nothing seems to change when I stop loading the timezone data.

It seems indeed that the timezone data are not used, at least for post dates. And it leads to wrong results. The Core REST API (wp/v2) returns a date field with no timezone or offset specified, like 2023-02-02T05:48:00. That's 5:48 am on February 2, in the CET zone, winter time. In UTC this is 4:48 am because the winter offset is +1h. But the WP timezone sent from server specifies a +2h offset, because it's summer time now, and this date will be interpreted as 3:48 am UTC.

Here's how my testing post looks like in the wp-admin post list:

Screenshot 2023-06-19 at 15 25 35

but in Gutenberg, the PostScheduleToggle component will show time offset by one hour, and is in conflict with what the DateTimePicker shows.

Screenshot 2023-06-19 at 15 26 43

That's because DateTimePicker displays the date as-is, without trying to interpret the time zone. I think it will start also showing bad results once the local (browser) time zone is different from the server (site) zone.

Ideally, we shouldn't use the timezone.offset from the server settings, but the timezone.abbr name, and pass that name to moment.tz. moment.tz knows all the rules for the zone, not just one offset, and will be able to handle things correctly.

@tyxla
Copy link
Member

tyxla commented Jun 29, 2023

Ideally, we shouldn't use the timezone.offset from the server settings, but the timezone.abbr name, and pass that name to moment.tz. moment.tz knows all the rules for the zone, not just one offset, and will be able to handle things correctly.

Seems like this is work for another PR. This one can be merged separately IMHO.

The only thing that gets me anxious is the 2030 limit. Perhaps we can change it to be the current year + 5 years, or + 10 years?

And then for the future, I'd love us to explore getting rid of moment.js completely. Temporal is at stage 3 and we can start experimenting with it already. One way to do it could be to create a completely new date package and migrate all Gutenberg usages to the new one. The old one could remain indefinitely in a deprecated state, but will be unused by the core project.

@jsnajdr
Copy link
Member Author

jsnajdr commented Jun 30, 2023

The only thing that gets me anxious is the 2030 limit. Perhaps we can change it to be the current year + 5 years, or + 10 years?

I changed the limit to 2040, 10 years more. I don't like the idea to calculate the year dynamically based on the current year. That makes the builds non-reproducible. On the same commit, you get different output on different dates. Also, the build machine's date can be completely wrong, like 01-01-1970.

@github-actions
Copy link

Flaky tests detected in bb62463.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5419716524
📝 Reported issues:

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.

Let's ship it 🚀 Thanks @jsnajdr 🙌

@jsnajdr jsnajdr merged commit 8c91da7 into trunk Jun 30, 2023
51 checks passed
@jsnajdr jsnajdr deleted the optimize/date-moment-tz branch June 30, 2023 08:44
@github-actions github-actions bot added this to the Gutenberg 16.2 milestone Jun 30, 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] Build Tooling Issues or PRs related to build tooling [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants