Skip to content

Framework: Decrease build size by fixing import statements#3531

Merged
gziolo merged 1 commit intomasterfrom
update/decrease-build-size
Nov 17, 2017
Merged

Framework: Decrease build size by fixing import statements#3531
gziolo merged 1 commit intomasterfrom
update/decrease-build-size

Conversation

@gziolo
Copy link
Copy Markdown
Member

@gziolo gziolo commented Nov 17, 2017 •
edited
Loading

Description

This PR updates import statements that were using legacy format. It turned out this caused blocks chunk to be also bundled together with editor chunk doubling the size of the chunk.

editor chunk has decreased size from 1.02 MB to 551 kB for production build. This is before compression.

It's worth mention that side-effects from blocks chunk were executed twice including all block registrations!

Before

                         Asset     Size               Chunks                    Chunk Names
           i18n/build/index.js  17.5 kB                    5  [emitted]         i18n
         blocks/build/index.js   552 kB                 1, 4  [emitted]  [big]  blocks
     components/build/index.js   262 kB                 2, 4  [emitted]  [big]  components
          utils/build/index.js  23.4 kB                    3  [emitted]         utils
        element/build/index.js  9.51 kB                    4  [emitted]         element
         editor/build/index.js  1.02 MB        0, 1, 3, 4, 5  [emitted]  [big]  editor
           date/build/index.js  3.08 kB                    6  [emitted]         date
      ./blocks/build/style.css  42.7 kB  0, 1, 3, 4, 5, 1, 4  [emitted]         editor, blocks
./blocks/build/edit-blocks.css  31.3 kB  0, 1, 3, 4, 5, 1, 4  [emitted]         editor, blocks
  ./components/build/style.css  30.8 kB                 2, 4  [emitted]         components
      ./editor/build/style.css  82.3 kB        0, 1, 3, 4, 5  [emitted]         editor

After

                         Asset     Size  Chunks                    Chunk Names
           i18n/build/index.js  17.6 kB       5  [emitted]         i18n
         blocks/build/index.js   551 kB       0  [emitted]  [big]  blocks
     components/build/index.js   260 kB       2  [emitted]  [big]  components
          utils/build/index.js  23.4 kB       3  [emitted]         utils
        element/build/index.js  9.52 kB       4  [emitted]         element
         editor/build/index.js   551 kB       1  [emitted]  [big]  editor
           date/build/index.js  3.08 kB       6  [emitted]         date
      ./blocks/build/style.css  42.7 kB       0  [emitted]         blocks
./blocks/build/edit-blocks.css  31.3 kB       0  [emitted]         blocks
  ./components/build/style.css  30.8 kB       2  [emitted]         components
      ./editor/build/style.css  82.3 kB       1  [emitted]         editor

How Has This Been Tested?

Manually.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@gziolo gziolo added [Type] Build Tooling Issues or PRs related to build tooling Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Bug An existing feature does not function as intended labels Nov 17, 2017
@gziolo gziolo self-assigned this Nov 17, 2017
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 17, 2017 •
edited
Loading

Codecov Report

Merging #3531 into master will increase coverage by 0.15%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3531      +/-   ##
=========================================
+ Coverage   34.85%     35%   +0.15%     
=========================================
  Files         261     261              
  Lines        6717    6733      +16     
  Branches     1225    1229       +4     
=========================================
+ Hits         2341    2357      +16     
  Misses       3691    3691              
  Partials      685     685
Impacted Files Coverage Δ
components/form-file-upload/index.js 71.42% <ø> (ø) ⬆️
components/higher-order/with-api-data/provider.js 25% <ø> (ø) ⬆️
...ents/post-taxonomies/hierarchical-term-selector.js 1.35% <ø> (ø) ⬆️
editor/components/post-visibility/utils.js 100% <ø> (ø) ⬆️
editor/components/word-count/index.js 0% <ø> (ø) ⬆️
editor/store-defaults.js 100% <ø> (ø) ⬆️
blocks/library/image/image-size.js 0% <ø> (ø) ⬆️
components/higher-order/with-api-data/index.js 83.95% <ø> (ø) ⬆️
editor/writing-flow/index.js 0% <0%> (ø) ⬆️
editor/store-persist.js 100% <0%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c93d78...a46bff1. Read the comment docs.

@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Nov 17, 2017

Actually, those changes are straightforward and block my work. I will merge it without review.

@gziolo gziolo merged commit 7a1e75e into master Nov 17, 2017
@gziolo gziolo deleted the update/decrease-build-size branch November 17, 2017 12:19
@mcsf
Copy link
Copy Markdown
Contributor

mcsf commented Nov 17, 2017

Good one. Should we lint against this? Looks like one of those things that's easy to slip by, even more so when refactoring.

@jorgefilipecosta
Copy link
Copy Markdown
Member

jorgefilipecosta commented Nov 17, 2017

Awesome finding 👍 It is good that you merged it, so you could advance I also did a set of smoke tests, and some grepping to find other cases and no problem found / other cases appeared.

@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Nov 17, 2017

I'm sure that it can be handled on the Webpack level. However, it would require some explorations how to avoid scanning sibling folders. I have a feeling that I saw once an article how to achieve it. Eslint might be more challenging to maintain if we ever add a new folder. I guess it wouldn't be hard to update but the trick is how to keep it in sync.

@mcsf mcsf mentioned this pull request Nov 17, 2017
Merged
3 tasks
youknowriad
youknowriad reviewed Nov 20, 2017
View reviewed changes
editor/components/word-count/index.js
@@ -7,7 +7,7 @@ import { connect } from 'react-redux';
* WordPress dependencies
*/
import { getBlocks } from '../../selectors';
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad Nov 20, 2017

Choose a reason for hiding this comment

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

This should probably move to "Internal dependencies"

youknowriad
youknowriad reviewed Nov 20, 2017
View reviewed changes
editor/store-defaults.js
@@ -1,4 +1,4 @@
import { viewPort } from '../utils';
Copy link
Copy Markdown
Contributor

@youknowriad youknowriad Nov 20, 2017

Choose a reason for hiding this comment

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

Missing WordPress dependencies comment :)

@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Nov 20, 2017

@youknowriad did you open PR? I can fix it in a bit since I started it :)

@youknowriad
Copy link
Copy Markdown
Contributor

youknowriad commented Nov 20, 2017

No I did not :)

@gziolo gziolo mentioned this pull request Nov 20, 2017
Merged
@gziolo
Copy link
Copy Markdown
Member Author

gziolo commented Nov 20, 2017

Opened #3561 to address that.

@aduth aduth mentioned this pull request Feb 18, 2019
Merged
@MaxMood96 MaxMood96 mentioned this pull request Dec 20, 2023
Open
@naiba4 naiba4 mentioned this pull request Dec 20, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowriad youknowriad youknowriad left review comments

@aduth aduth Awaiting requested review from aduth

@mcsf mcsf Awaiting requested review from mcsf

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

Assignees

@gziolo gziolo

Labels

Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Bug An existing feature does not function as intended [Type] Build Tooling Issues or PRs related to build tooling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gziolo @mcsf @jorgefilipecosta @youknowriad