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

Split lib/products-values #44567

Merged
merged 71 commits into from Aug 3, 2020
Merged

Split lib/products-values #44567

merged 71 commits into from Aug 3, 2020

Conversation

sgomes
Copy link
Contributor

@sgomes sgomes commented Jul 30, 2020

lib/products-values is currently a large monolith, consisting of many different imports and exports. This makes it hard for the bundler to tree-shake effectively, and introduces potential problems because of circular dependencies between lib/products-values and lib/plans, for example.

This PR splits the library into many small modules, while re-exporting everything from the index file. This should result in minimal changes in consumers of this library, while enabling better tree-shaking.

I don't expect immediate performance benefits from this PR, as there are likely other large libraries to untangle first. I was wrong; see the comments below.

This PR is working towards the end goal of removing the dependency on lib/plans from the critical path, which may yet turn out to be impossible.

Changes proposed in this Pull Request

  • Split the various exports in index.js into individual modules
  • Split assertValidProduct into a new utils/ folder
  • Move all of the translated constants from constants.js to a new translations.js
  • Add package.json with sideEffects config, to enable the bundler to take advantage of the modularisation of the library
  • Ensure lib/plans and lib/domains refer to the new individual modules instead of the index file, to avoid circular dependencies

Testing instructions

This PR doesn't change any functional code, it only moves some code around. As such, a correct build and passing automated tests should be enough to ensure that nothing got broken.

@sgomes sgomes added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Janitorial labels Jul 30, 2020
@sgomes sgomes requested review from ChaosExAnima, jsnmoon, tyxla and a team July 30, 2020 16:49
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

matticbot commented Jul 30, 2020

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Webpack Runtime (~30 bytes removed 📉 [gzipped])

name      parsed_size           gzip_size
manifest       -159 B  (-0.1%)      -30 B  (-0.1%)

Webpack runtime for loading modules. It is included in the HTML page as an inline script. Is downloaded and parsed every time the app is loaded.

App Entrypoints (~3500 bytes removed 📉 [gzipped])

name        parsed_size           gzip_size
entry-main     -14070 B  (-1.0%)    -3500 B  (-1.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~124312 bytes added 📈 [gzipped])

name                    parsed_size            gzip_size
checkout                   +25292 B   (+1.6%)    +4617 B  (+1.2%)
plans                      +23972 B   (+4.7%)    +4305 B  (+3.1%)
domains                    +22659 B   (+2.0%)    +4206 B  (+1.6%)
purchases                  +22522 B   (+2.3%)    +4256 B  (+1.8%)
settings-performance       +22091 B   (+7.7%)    +4081 B  (+5.0%)
email                      +21566 B   (+6.5%)    +3977 B  (+4.4%)
stats                      +21384 B   (+2.5%)    +3969 B  (+1.9%)
activity                   +21283 B   (+4.2%)    +3919 B  (+3.0%)
settings-security          +21268 B   (+7.4%)    +3944 B  (+4.9%)
settings                   +21233 B   (+4.5%)    +3916 B  (+3.0%)
settings-writing           +21227 B   (+4.1%)    +3922 B  (+2.9%)
marketing                  +21167 B   (+4.9%)    +3910 B  (+3.4%)
settings-jetpack           +20898 B   (+9.5%)    +3845 B  (+6.1%)
settings-discussion        +20898 B   (+9.5%)    +3852 B  (+6.1%)
themes                     +20669 B   (+5.5%)    +3823 B  (+3.7%)
theme                      +20669 B   (+7.1%)    +3826 B  (+4.8%)
signup                     +20655 B   (+9.1%)    +3836 B  (+6.6%)
jetpack-connect            +20594 B   (+3.1%)    +3820 B  (+2.2%)
woocommerce                +20546 B   (+1.0%)    +3818 B  (+0.7%)
export                     +20529 B  (+10.2%)    +3809 B  (+6.5%)
customize                  +20529 B  (+11.8%)    +3809 B  (+7.3%)
accept-invite              +20529 B   (+7.0%)    +3923 B  (+5.0%)
help                       +20340 B   (+4.4%)    +3850 B  (+3.2%)
account-close              +20340 B   (+6.5%)    +3901 B  (+4.6%)
post-editor                 +5140 B   (+0.2%)    +1317 B  (+0.2%)
plugins                     +4736 B   (+1.1%)    +1279 B  (+1.1%)
earn                        +4460 B   (+1.5%)    +1215 B  (+1.6%)
feature-upsell              +4035 B   (+2.5%)    +1146 B  (+2.8%)
posts-custom                +3301 B   (+0.9%)    +1102 B  (+1.1%)
posts                       +3301 B   (+0.9%)    +1102 B  (+1.1%)
media                       +3301 B   (+0.8%)    +1104 B  (+1.0%)
hosting                     +3301 B   (+1.4%)    +1110 B  (+1.7%)
gutenberg-editor            +3301 B   (+0.4%)    +1104 B  (+0.5%)
backup                      +3235 B   (+0.8%)    +1098 B  (+1.0%)
scan                        +3183 B   (+0.9%)    +1085 B  (+1.2%)
zoninator                   +2972 B   (+1.0%)    +1040 B  (+1.2%)
wp-super-cache              +2972 B   (+1.3%)    +1040 B  (+1.7%)
sites                       +2972 B   (+3.4%)    +1040 B  (+3.9%)
sensei                      +2972 B   (+2.9%)    +1040 B  (+3.4%)
preview                     +2972 B   (+1.9%)    +1040 B  (+2.4%)
people                      +2972 B   (+0.8%)    +1040 B  (+1.0%)
pages                       +2972 B   (+1.2%)     +916 B  (+1.3%)
migrate                     +2972 B   (+2.0%)    +1040 B  (+2.5%)
jetpack-cloud-settings      +2972 B   (+2.2%)    +1040 B  (+2.7%)
jetpack-cloud               +2972 B   (+3.3%)    +1040 B  (+3.8%)
import                      +2972 B   (+1.3%)    +1040 B  (+1.6%)
home                        +2972 B   (+0.7%)    +1040 B  (+0.9%)
hello-dolly                 +2972 B   (+2.8%)    +1040 B  (+3.2%)
google-my-business          +2972 B   (+1.0%)    +1040 B  (+1.2%)
concierge                   +2972 B   (+0.9%)    +1040 B  (+1.3%)
comments                    +2972 B   (+0.6%)    +1040 B  (+0.8%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~14409 bytes added 📈 [gzipped])

name                                                      parsed_size            gzip_size
async-load-design-blocks                                     +20895 B   (+0.7%)    +3981 B   (+0.6%)
async-load-design                                            +20340 B   (+1.1%)    +3925 B   (+0.9%)
async-load-my-sites-current-site-stale-cart-items-notice     +17557 B  (+30.8%)    +2769 B  (+15.8%)
async-load-my-sites-sidebar                                  +17474 B  (+10.0%)    +2730 B   (+5.7%)
async-load-components-web-preview-component                   +2168 B   (+0.5%)     +405 B   (+0.3%)
async-load-my-sites-site-settings-seo-settings-form            +998 B   (+0.4%)     +134 B   (+0.2%)
async-load-signup-steps-domains                                +338 B   (+0.1%)      +96 B   (+0.2%)
async-load-reader-sidebar                                      +329 B   (+0.5%)      +66 B   (+0.4%)
async-load-my-sites-current-site-notice                        +329 B   (+0.5%)      +66 B   (+0.4%)
async-load-blocks-jitm-templates-sidebar-banner                +329 B   (+1.2%)      +68 B   (+1.0%)
async-load-blocks-jitm-templates-notice                        +329 B   (+1.2%)      +68 B   (+0.9%)
async-load-blocks-jitm-templates-default                       +329 B   (+1.2%)      +67 B   (+1.0%)
async-load-blocks-product-purchase-features-list                +94 B   (+0.3%)      +20 B   (+0.3%)
async-load-signup-steps-plans                                   +47 B   (+0.0%)       +7 B   (+0.0%)
async-load-blocks-product-plan-overlap-notices                  +13 B   (+0.2%)       +7 B   (+0.3%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@sgomes
Copy link
Contributor Author

sgomes commented Jul 30, 2020

Looks like I was wrong, there are immediate benefits, actually! 🎉

This PR appears to remove ~14KB (3.5KB gzipped) from the critical path, offloading it to shared chunks that get used in just the sections that require the code.

Please disregard the matticbot totals; ICFY calculates totals incorrectly, and makes it seem like this PR disproportionately increases sections to make up for the entrypoint. That's not the case; most of the changes are ending up in shared chunks, which get counted multiple times in the total.

For most user journeys, this PR will be a net win.

@sgomes
Copy link
Contributor Author

sgomes commented Aug 3, 2020

Thank you for the review, @scinos! 🙇

@sgomes sgomes merged commit 11ff8e4 into master Aug 3, 2020
@sgomes sgomes deleted the update/split-lib-products-values branch August 3, 2020 09:20
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 3, 2020
@a8ci18n
Copy link

a8ci18n commented Aug 20, 2020

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/4247512

Hi @sgomes, could you please edit the description of this PR and add a screenshot for our translators? Ideally it'd include all of the following strings:

  • Real-time backups save every change and one-click restores get you back online quickly.
  • Help your site visitors find answers instantly so they keep reading and buying.
  • Automated spam protection for comments and forms. Save time, get more responses, and give your visitors a better experience.

Thank you in advance!

@sgomes
Copy link
Contributor Author

sgomes commented Aug 21, 2020

@a8ci18n: there should be no new strings with this PR.

@a8ci18n
Copy link

a8ci18n commented Aug 31, 2020

Translation for this Pull Request has now been finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants