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

Build: Use .min.js suffix for bundled JavaScript #23926

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Jul 14, 2020

Description

The webpack-bundled packages intended for consumption via WordPress scripts in build/ are shipped with the Gutenberg plugin as minified sources.

Contrary to WordPress convention, these minified sources use the .js extension rather than .min.js.

Use the .min.js extension for bundled sources.

Note:
The .min.js extension is hard-coded in a few places. This is optimized for the most common use case — when the scripts are used as part of the Gutenberg plugin.

This is not accurate in development (npm run dev) where the scripts are not minified. I consider this to be an acceptable tradeoff. Developers are unlikely to be confused by the .min.js extension containing readable source.

How has this been tested?

The Gutenberg plugin enqueues .min.js from build and their dependencies correctly.
Build (npm run build:plugin-zip) and install the plugin on a site. The editor works correctly.

Types of changes

Internal.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

Size Change: -1.02 MB (89%) 🏆

Total Size: 1.14 MB

Filename Size Change
build/a11y/index.js 0 B -1.14 kB (0%)
build/annotations/index.js 0 B -3.67 kB (0%)
build/api-fetch/index.js 0 B -3.39 kB (0%)
build/autop/index.js 0 B -2.82 kB (0%)
build/blob/index.js 0 B -620 B (0%)
build/block-directory/index.js 0 B -7.67 kB (0%)
build/block-editor/index.js 0 B -115 kB (0%)
build/block-library/index.js 0 B -132 kB (0%)
build/block-serialization-default-parser/index.js 0 B -1.88 kB (0%)
build/block-serialization-spec-parser/index.js 0 B -3.1 kB (0%)
build/blocks/index.js 0 B -48.2 kB (0%)
build/components/index.js 0 B -199 kB (0%)
build/compose/index.js 0 B -9.67 kB (0%)
build/core-data/index.js 0 B -11.5 kB (0%)
build/data-controls/index.js 0 B -1.29 kB (0%)
build/data/index.js 0 B -8.46 kB (0%)
build/date/index.js 0 B -5.38 kB (0%)
build/deprecated/index.js 0 B -772 B (0%)
build/dom-ready/index.js 0 B -569 B (0%)
build/dom/index.js 0 B -3.23 kB (0%)
build/edit-navigation/index.js 0 B -10.8 kB (0%)
build/edit-post/index.js 0 B -304 kB (0%)
build/edit-site/index.js 0 B -16.8 kB (0%)
build/edit-widgets/index.js 0 B -9.35 kB (0%)
build/editor/index.js 0 B -45.1 kB (0%)
build/element/index.js 0 B -4.65 kB (0%)
build/escape-html/index.js 0 B -733 B (0%)
build/format-library/index.js 0 B -7.72 kB (0%)
build/hooks/index.js 0 B -2.13 kB (0%)
build/html-entities/index.js 0 B -622 B (0%)
build/i18n/index.js 0 B -3.56 kB (0%)
build/is-shallow-equal/index.js 0 B -709 B (0%)
build/keyboard-shortcuts/index.js 0 B -2.51 kB (0%)
build/keycodes/index.js 0 B -1.94 kB (0%)
build/list-reusable-blocks/index.js 0 B -3.12 kB (0%)
build/media-utils/index.js 0 B -5.32 kB (0%)
build/notices/index.js 0 B -1.79 kB (0%)
build/nux/index.js 0 B -3.4 kB (0%)
build/plugins/index.js 0 B -2.56 kB (0%)
build/primitives/index.js 0 B -1.4 kB (0%)
build/priority-queue/index.js 0 B -789 B (0%)
build/redux-routine/index.js 0 B -2.85 kB (0%)
build/rich-text/index.js 0 B -13.9 kB (0%)
build/server-side-render/index.js 0 B -2.71 kB (0%)
build/shortcode/index.js 0 B -1.7 kB (0%)
build/token-list/index.js 0 B -1.27 kB (0%)
build/url/index.js 0 B -4.06 kB (0%)
build/viewport/index.js 0 B -1.85 kB (0%)
build/warning/index.js 0 B -1.13 kB (0%)
build/wordcount/index.js 0 B -1.17 kB (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.39 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.67 kB 0 B
build/block-directory/style-rtl.css 944 B 0 B
build/block-directory/style.css 945 B 0 B
build/block-editor/index.min.js 115 kB 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.6 kB 0 B
build/block-library/editor.css 7.59 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-library/style-rtl.css 7.77 kB 0 B
build/block-library/style.css 7.77 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.2 kB 0 B
build/components/index.min.js 199 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.8 kB 0 B
build/compose/index.min.js 9.67 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.46 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 569 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.min.js 304 kB 0 B
build/edit-post/style-rtl.css 5.6 kB 0 B
build/edit-post/style.css 5.6 kB 0 B
build/edit-site/index.min.js 16.8 kB 0 B
build/edit-site/style-rtl.css 3.04 kB 0 B
build/edit-site/style.css 3.04 kB 0 B
build/edit-widgets/index.min.js 9.35 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.min.js 45.1 kB 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.min.js 2.13 kB 0 B
build/html-entities/index.min.js 622 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 709 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.min.js 5.32 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.13 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@sirreal sirreal marked this pull request as ready for review July 14, 2020 11:54
@sirreal
Copy link
Member Author

sirreal commented Jul 14, 2020

The size bot seems to be confused by the renames. For some reason the "new" (.min.js) packages don't offset the "removed" (.js) packages.

The min versions do appear if you expand the "View Unchanged" section.

@sirreal sirreal self-assigned this Jul 14, 2020
@sirreal sirreal added [Type] Code Quality Issues or PRs that relate to code quality Framework Issues related to broader framework topics, especially as it relates to javascript Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts labels Jul 14, 2020
@sirreal sirreal requested a review from oandregal July 15, 2020 14:28
@youknowriad
Copy link
Contributor

This is not accurate in development (npm run dev) where the scripts are not minified. I consider this to be an acceptable tradeoff. Developers are unlikely to be confused by the .min.js extension containing readable source.

I'd love thoughts from @WordPress/gutenberg-core about whether this is an acceptable tradeoff.

@nerrad
Copy link
Contributor

nerrad commented Jul 15, 2020

This is not accurate in development (npm run dev) where the scripts are not minified. I consider this to be an acceptable tradeoff. Developers are unlikely to be confused by the .min.js extension containing readable source.

I did wonder if folks use the presence of .min.js to determine whether the current state is a production build or not, that could introduce some confusion, but realistically this is only a potential issue for folks developing Gutenberg and presumably there'd be some awareness around the state of the build in those cases (not to mention the need to rebuild if you're uncertain to begin with).

So, ya I think it's an acceptable tradeoff.

I'm assuming none of this work impacts the build process when things land in WP core (and that's a separate build process)?

@sirreal
Copy link
Member Author

sirreal commented Jul 15, 2020

I'm assuming none of this work impacts the build process when things land in WP core (and that's a separate build process)?

Yes, I believe that's a separate process handled here: https://github.com/WordPress/wordpress-develop/blob/b00172339467889b736cf0a8d0a7bc5a502b1158/tools/webpack/packages.js

@fullofcaffeine
Copy link
Member

Looks good to me 👍

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Ok let's do this.

@sirreal sirreal merged commit 40c6c8f into master Jul 16, 2020
@sirreal sirreal deleted the update/use-min-infix-for-minified-js branch July 16, 2020 08:10
@sirreal
Copy link
Member Author

sirreal commented Jul 16, 2020

Thanks, everyone!

@github-actions github-actions bot added this to the Gutenberg 8.6 milestone Jul 16, 2020
sirreal added a commit that referenced this pull request Jul 28, 2020
ellatrix pushed a commit that referenced this pull request Jul 28, 2020
ellatrix added a commit that referenced this pull request Jul 28, 2020
* Revert "Build: Use .min.js extension for bundled JavaScript (#23926)" (#24239)

This reverts commit 40c6c8f.

* Remove deprecated blockType.context support (#24155)

Co-authored-by: Jon Surrell <[email protected]>
Co-authored-by: Riad Benguella <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants