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.

Sign up for GitHub

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

Jump to bottom

Packages: Add information for webpack about no side effects #17862

Merged
merged 2 commits into from
Oct 10, 2019
Merged

Packages: Add information for webpack about no side effects #17862

merged 2 commits into from
Oct 10, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Oct 9, 2019 •
edited
Loading

Description

Previous work from @talldan: #14135 as explained in #13910:

The sideEffects property indicates whether a package introduces any side effects. It can be specified as either false or as an array of files that have side effects.

By specifying this property, webpack is able to reduce the size of JavaScript bundles by eliminating unused imports:
https://webpack.js.org/guides/tree-shaking/

sideEffects is particularly relevant when implementing feature flags. Without it, features need to be flagged at every possible point. For further details, see this discussion: #13829 (comment)

In our case, this is extremely important for @wordpress/block-library package to ensure that dead code behind the GUTENBERG_PHASE flag isn't bundled in WordPress 5.3 release.

I also included other packages which @talldan discovered as side-effects free. However, I took a more cautious approach and marked only those packages which only export their APIs which are pure.

Testing

Set GUTENBRG_PHASE flag to 1:

diff --git a/package.json b/package.json
index 84f28a49d..abb06b4f1 100644
--- a/package.json
+++ b/package.json
@@ -15,7 +15,7 @@
                "url": "https://github.com/WordPress/gutenberg/issues"
        },
        "config": {
-               "GUTENBERG_PHASE": 2
+               "GUTENBERG_PHASE": 1
        },
        "dependencies": {
                "@wordpress/a11y": "file:packages/a11y",

Make sure that everything works.

Validate that the block library module is 65kb smaller after running npm run build.

Before

                                                 Asset        Size  Chunks                    Chunk Names
                                 ./build/a11y/index.js    2.18 KiB       0  [emitted]         a11y
                          ./build/annotations/index.js     9.7 KiB       1  [emitted]         annotations
                            ./build/api-fetch/index.js    7.05 KiB       2  [emitted]         apiFetch
                                ./build/autop/index.js    6.89 KiB       3  [emitted]         autop
                                 ./build/blob/index.js    1.37 KiB       4  [emitted]         blob
                      ./build/block-directory/index.js    15.8 KiB       5  [emitted]         blockDirectory
                         ./build/block-editor/index.js     301 KiB       6  [emitted]  [big]  blockEditor
                        ./build/block-library/index.js     **361 KiB**       7  [emitted]  [big]  blockLibrary
   ./build/block-serialization-default-parser/index.js    3.63 KiB       8  [emitted]         blockSerializationDefaultParser
      ./build/block-serialization-spec-parser/index.js    10.5 KiB       9  [emitted]         blockSerializationSpecParser
                               ./build/blocks/index.js     170 KiB      10  [emitted]         blocks
                           ./build/components/index.js     559 KiB      11  [emitted]  [big]  components
                              ./build/compose/index.js    9.23 KiB      12  [emitted]         compose
                            ./build/core-data/index.js    32.9 KiB      13  [emitted]         coreData
                        ./build/data-controls/index.js    2.87 KiB      15  [emitted]         dataControls
                                 ./build/data/index.js    24.4 KiB      14  [emitted]         data
                                 ./build/date/index.js    13.3 KiB      16  [emitted]         date
                           ./build/deprecated/index.js    1.61 KiB      17  [emitted]         deprecated
                            ./build/dom-ready/index.js    1.14 KiB      19  [emitted]         domReady
                                  ./build/dom/index.js    7.51 KiB      18  [emitted]         dom
                            ./build/edit-post/index.js    82.8 KiB      20  [emitted]         editPost
                         ./build/edit-widgets/index.js    13.7 KiB      21  [emitted]         editWidgets
                               ./build/editor/index.js     163 KiB      22  [emitted]         editor
                              ./build/element/index.js    9.03 KiB      23  [emitted]         element
                          ./build/escape-html/index.js     1.6 KiB      24  [emitted]         escapeHtml
                       ./build/format-library/index.js    17.7 KiB      25  [emitted]         formatLibrary
                                ./build/hooks/index.js    5.41 KiB      26  [emitted]         hooks
                        ./build/html-entities/index.js    1.34 KiB      27  [emitted]         htmlEntities
                                 ./build/i18n/index.js    8.52 KiB      28  [emitted]         i18n
                     ./build/is-shallow-equal/index.js    1.63 KiB      29  [emitted]         isShallowEqual
                             ./build/keycodes/index.js    4.74 KiB      30  [emitted]         keycodes
                 ./build/list-reusable-blocks/index.js    8.41 KiB      31  [emitted]         listReusableBlocks
                          ./build/media-utils/index.js    12.7 KiB      32  [emitted]         mediaUtils
                              ./build/notices/index.js    4.53 KiB      33  [emitted]         notices
                                  ./build/nux/index.js    7.58 KiB      34  [emitted]         nux
                              ./build/plugins/index.js    6.75 KiB      35  [emitted]         plugins
                       ./build/priority-queue/index.js    1.46 KiB      36  [emitted]         priorityQueue
                        ./build/redux-routine/index.js    9.49 KiB      37  [emitted]         reduxRoutine
                            ./build/rich-text/index.js    45.5 KiB      38  [emitted]         richText
                   ./build/server-side-render/index.js     7.6 KiB      39  [emitted]         serverSideRender
                            ./build/shortcode/index.js    3.96 KiB      40  [emitted]         shortcode
                           ./build/token-list/index.js    3.16 KiB      41  [emitted]         tokenList
                                  ./build/url/index.js    10.8 KiB      42  [emitted]         url
                             ./build/viewport/index.js    2.65 KiB      43  [emitted]         viewport
                            ./build/wordcount/index.js    2.93 KiB      44  [emitted]         wordcount

After

                                                 Asset        Size  Chunks                    Chunk Names
                                 ./build/a11y/index.js    2.18 KiB       0  [emitted]         a11y
                          ./build/annotations/index.js     9.7 KiB       1  [emitted]         annotations
                            ./build/api-fetch/index.js    7.05 KiB       2  [emitted]         apiFetch
                                ./build/autop/index.js    6.89 KiB       3  [emitted]         autop
                                 ./build/blob/index.js    1.37 KiB       4  [emitted]         blob
                      ./build/block-directory/index.js    15.8 KiB       5  [emitted]         blockDirectory
                         ./build/block-editor/index.js     301 KiB       6  [emitted]  [big]  blockEditor
                        ./build/block-library/index.js     **296 KiB**       7  [emitted]  [big]  blockLibrary
   ./build/block-serialization-default-parser/index.js    3.63 KiB       8  [emitted]         blockSerializationDefaultParser
      ./build/block-serialization-spec-parser/index.js    10.5 KiB       9  [emitted]         blockSerializationSpecParser
                               ./build/blocks/index.js     170 KiB      10  [emitted]         blocks
                           ./build/components/index.js     559 KiB      11  [emitted]  [big]  components
                              ./build/compose/index.js    9.23 KiB      12  [emitted]         compose
                            ./build/core-data/index.js    32.9 KiB      13  [emitted]         coreData
                        ./build/data-controls/index.js    2.87 KiB      15  [emitted]         dataControls
                                 ./build/data/index.js    24.4 KiB      14  [emitted]         data
                                 ./build/date/index.js    13.3 KiB      16  [emitted]         date
                           ./build/deprecated/index.js    1.61 KiB      17  [emitted]         deprecated
                            ./build/dom-ready/index.js    1.14 KiB      19  [emitted]         domReady
                                  ./build/dom/index.js    7.51 KiB      18  [emitted]         dom
                            ./build/edit-post/index.js    82.8 KiB      20  [emitted]         editPost
                         ./build/edit-widgets/index.js    13.7 KiB      21  [emitted]         editWidgets
                               ./build/editor/index.js     163 KiB      22  [emitted]         editor
                              ./build/element/index.js    9.03 KiB      23  [emitted]         element
                          ./build/escape-html/index.js     1.6 KiB      24  [emitted]         escapeHtml
                       ./build/format-library/index.js    17.7 KiB      25  [emitted]         formatLibrary
                                ./build/hooks/index.js    5.41 KiB      26  [emitted]         hooks
                        ./build/html-entities/index.js    1.34 KiB      27  [emitted]         htmlEntities
                                 ./build/i18n/index.js    8.52 KiB      28  [emitted]         i18n
                     ./build/is-shallow-equal/index.js    1.63 KiB      29  [emitted]         isShallowEqual
                             ./build/keycodes/index.js    4.74 KiB      30  [emitted]         keycodes
                 ./build/list-reusable-blocks/index.js    8.41 KiB      31  [emitted]         listReusableBlocks
                          ./build/media-utils/index.js    12.7 KiB      32  [emitted]         mediaUtils
                              ./build/notices/index.js    4.53 KiB      33  [emitted]         notices
                                  ./build/nux/index.js    7.58 KiB      34  [emitted]         nux
                              ./build/plugins/index.js    6.75 KiB      35  [emitted]         plugins
                       ./build/priority-queue/index.js    1.46 KiB      36  [emitted]         priorityQueue
                        ./build/redux-routine/index.js    9.49 KiB      37  [emitted]         reduxRoutine
                            ./build/rich-text/index.js    45.5 KiB      38  [emitted]         richText
                   ./build/server-side-render/index.js     7.6 KiB      39  [emitted]         serverSideRender
                            ./build/shortcode/index.js    3.96 KiB      40  [emitted]         shortcode
                           ./build/token-list/index.js    3.16 KiB      41  [emitted]         tokenList
                                  ./build/url/index.js    10.8 KiB      42  [emitted]         url
                             ./build/viewport/index.js    2.65 KiB      43  [emitted]         viewport
                            ./build/wordcount/index.js    2.93 KiB      44  [emitted]         wordcount

@gziolo gziolo added [Type] Performance Related to performance efforts npm Packages Related to npm packages Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Priority] High Used to indicate top priority items that need quick attention labels Oct 9, 2019
@gziolo gziolo requested a review from mcsf October 9, 2019 14:20
@gziolo gziolo self-assigned this Oct 9, 2019
youknowriad
youknowriad reviewed Oct 9, 2019
View reviewed changes
packages/i18n/package.json Outdated
@@ -20,6 +20,7 @@
"main": "build/index.js",
"module": "build-module/index.js",
"react-native": "src/index",
"sideEffects": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this one is sideEffects free (it contain globals)

Copy link
Member Author

Choose a reason for hiding this comment

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

Happy to remove it to move forward 👍

@gziolo gziolo merged commit 79e5749 into master Oct 10, 2019
@gziolo gziolo deleted the update/side-effects branch October 10, 2019 13:33
@youknowriad youknowriad added this to the Gutenberg 6.7 milestone Oct 14, 2019
gziolo added a commit that referenced this pull request Oct 15, 2019
@gziolo
Packages: Add information for webpack about no side effects (#17862)
9b9bfc4 
* Packages: Add information for webpack about no side effects

* Update README.md
@senadir
Copy link
Contributor

senadir commented Dec 4, 2019

@gziolo Any reason why @wordpress/components isn’t included in this?

@gziolo
Copy link
Member Author

gziolo commented Dec 4, 2019

It's a big package, I wanted to avoid taking a risk during the beta phase of WordPress 5.3 release, we should try to include it 👍

@senadir
Copy link
Contributor

senadir commented Dec 4, 2019

I`ve opened a PR doing that (#18911, tests still running), from smoke-testing locally there doesn’t seem to be any regressions.

@jorgefilipecosta
Copy link
Member

Hi @gzilo, does this change still needs a backport/change in WordPress core or the change was already shipped in WordPress 5.3/subsquent package updated?

@jorgefilipecosta jorgefilipecosta mentioned this pull request Feb 4, 2020
Closed
7 tasks
@gziolo
Copy link
Member Author

gziolo commented Feb 5, 2020

Yes, it was cherry-picked for WordPress 5.3 and now it's included in Gutenberg packages by default so no need to do anything 👍

@jorgefilipecosta jorgefilipecosta 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 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youknowriad youknowriad youknowriad approved these changes

@aduth aduth Awaiting requested review from aduth

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@dmsnell dmsnell Awaiting requested review from dmsnell

@ellatrix ellatrix Awaiting requested review from ellatrix

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@nerrad nerrad Awaiting requested review from nerrad

@Soean Soean Awaiting requested review from Soean

@swissspidy swissspidy Awaiting requested review from swissspidy

@talldan talldan Awaiting requested review from talldan

@mcsf mcsf Awaiting requested review from mcsf

Assignees

@gziolo gziolo

Labels
npm Packages Related to npm packages [Priority] High Used to indicate top priority items that need quick attention [Type] Performance Related to performance efforts
Projects
None yet
Milestone
Gutenberg 6.7
Development

Successfully merging this pull request may close these issues.
4 participants
@gziolo @senadir @jorgefilipecosta @youknowriad