google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.22k stars 278 forks source link

Optimize bundles to reduce the total assets size #4610

Open eugene-manuilov opened 2 years ago

eugene-manuilov commented 2 years ago

Feature Description

We need to review our bundles to find an opportunity to reduce the total size of the generated assets. If we can shrink the total size down, it will let us include the development (non-minified) assets into the release that can be used with the SCRIPT_DEBUG mode.

We have the npm run test:analyze command that runs the bundle analyzer tool for the main assets. We need to use this tool to see which modules are included in bundles and find a way how to optimize them to take less space.

127 0 0 1_8888_


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

aaemnnosttv commented 2 years ago

@felixarntz @tofumatt let's try to prioritize this in the not-too-distant future πŸ˜„

eugene-manuilov commented 1 year ago

AC βœ”οΈ

asvinb commented 1 year ago

Unassigning myself from the ticket since I'll be OoO. Created ticket #6139 and was looking at SVG optimizations where I think we can tweak things further.

10upsimon commented 9 months ago

@techanvil @eugene-manuilov I am pinging you both specifically as I see you've both been close to this issue at some stage in the past.

While re-visiting this some time after the issue was created, it appears that common modules that repeat themselves as being bundled into various modules that import and make use of them. Such examples include i18n.js, utils.js and also cache.js.

the vendor module (aka node_modules) is without a doubt the largest occupier of space, at around 2.7MB prior to gzipping. At a first glance, 2 large chunks of modules that stood out were the following:

Lodash

Lodash is included in it's entirety because of usages such as import { filter } from 'lodash'; as opposed to a more modular form such as import { filter } from 'lodash/filter';. The latter should ensure that only the necessary lodash modules are bundled into entry point file(s) as opposed to the entire library.

Further to this (and this may have been discussed elsewhere), Lodash is (apparently?) included with WordPress. Is there a need to define this at a project level at all, would this make a difference in the sense that even if it were imported from another location included with WP, it may ultimately still be bundled?

Rules.json

Upon researching the origin of rules.json, it was clear that it was a reference source for the psl library, used by adsense urls.js for domain parsing.

Upon further research, it was discovered that an alternative package was available, did not make use of a massive JSON dictionary, and was substantially more performant at an application level regardless of bundle size. This library, tldts, performs significantly more operations per second at a substantially reduced overhead. This can be observed in these benchmarking results.

Not only will swapping out psl for tldts remove the large json presence in the build, but it too will lend to a more optimized experience.

Unused Dependencies

This area was a little more tricky to run without false positives, as I believe the project entry points are not particularly black and white for the tools to understand.

The first tool used to check for unused dependencies was depcheck, a commonly used tool but not without it's flaws (mostly related to false positives).

Running npx depcheck produced the following output:

Unused dependencies

* @material/select
* @material/textfield
* @material/theme
* accessible-autocomplete
* element-closest
* formdata-polyfill
* natives
* polyfill-library
* promise-polyfill
* psl
* punycode
* svgxuse
* url-polyfill
* uuid
* whatwg-fetch
Unused devDependencies

* @babel/plugin-transform-runtime
* @babel/preset-react
* @popperjs/core
* @svgr/webpack
* @wordpress/babel-preset-default
* @wordpress/browserslist-config
* @wordpress/stylelint-config
* archiver
* babel-loader
* css-loader
* eslint-scope
* file-loader
* fs-extra
* git-repo-info
* postcss-loader
* puppeteer-core
* sanitize-filename
* sass-loader
* semver-compare
* semver-regex
* stylelint-config-standard-scss
* stylelint-declaration-strict-value
* stylelint-order
* stylelint-scss
* webpack-cli
* webpack-strea
Missing dependencies

* googlesitekit-components: ./tests/js/ThrowErrorComponent.js
* googlesitekit-data: ./tests/js/WithRegistrySetup.js
* redux: ./tests/js/redux-debug.js
* history: ./tests/js/test-utils.js
* react-router: ./tests/js/utils.js
* cross-spawn: ./tests/e2e/script.js
* jest-matcher-utils: ./tests/e2e/config/wordpress-debug-log/index.js
* @babel/parser: ./tests/backstop/scenarios.js
* @babel/traverse: ./tests/backstop/scenarios.js
* @material/menu-surface: ./assets/sass/admin.scss
* @material/react-tab: ./assets/sass/admin.scss
* @material/react-tab-indicator: ./assets/sass/admin.scss
* @material/react-tab-scroller: ./assets/sass/admin.scss
* components-gm3: ./assets/sass/admin.scss
* components: ./assets/sass/admin.scss
* @wordpress/api-fetch__non-shim: ./assets/js/api-fetch-shim.js
* @wordpress/i18n__non-shim: ./assets/js/googlesitekit-i18n.js
* googlesitekit-modules: ./assets/js/googlesitekit-modules-adsense.js
* googlesitekit-widgets: ./assets/js/googlesitekit-modules-adsense.js
* googlesitekit-api: ./assets/js/util/test/getExistingTagURLs.js
* @testing-library/dom: ./assets/js/googlesitekit/widgets/components/WidgetAreaRenderer.test.js

While these can and should be used as a basic means of reference, they should be taken with a pinch of salt and used merely as a baseline for investigation. Static dependency analysis does usually lack full project context, and false positives are oftentimes reported.

Unimported Dependencies

A check for unimported dependencies was performed used the unimported library. It is worth noting that this library requires node >= 16 and thus temporarily enabling said version is and was required to successfully run the tooling.

Running npx unimported produced the following output:

unresolved imports  : 9
unused dependencies : 21
unimported files    : 352

The unimported files are largely false positives as a result of the tests directory.

Unresolved imports were reported as follows:

 9 unresolved imports

 1 β”‚ googlesitekit-data
2 β”‚ googlesitekit-api
3 β”‚ googlesitekit-components
4 β”‚ googlesitekit-modules
5 β”‚ @wordpress/api-fetch__non-shim
6 β”‚ @wordpress/i18n__non-shim
7 β”‚ ./pagespeed--desktop
8 β”‚ ./pagespeed--mobile
9 β”‚ ./report

It is obvious that these are likely all false positives, but worth cross checking with results from depcheck. This tooling also has the ability to configure entry points and ignore lists via a config file.

Unused dependencies were detected as follows:

21 unused dependencies

   1 β”‚ @material/button
   2 β”‚ @material/layout-grid
   3 β”‚ @material/linear-progress
   4 β”‚ @material/list
   5 β”‚ @material/select
   6 β”‚ @material/textfield
   7 β”‚ @material/theme
   8 β”‚ accessible-autocomplete
   9 β”‚ autoprefixer
  10 β”‚ element-closest
  11 β”‚ formdata-polyfill
  12 β”‚ intl
  13 β”‚ intl-locales-supported
  14 β”‚ natives
  15 β”‚ polyfill-library
  16 β”‚ promise-polyfill
  17 β”‚ psl
  18 β”‚ svgxuse
  19 β”‚ url-polyfill
  20 β”‚ uuid
  21 β”‚ whatwg-fetch

These are worth cross checking with results of depcheck and other such bundle/dependency analysis tools.

There were a number of unimported files detected, seemingly all false positives but worth a glance for consideration.

Knip

Last of the tooling explored was Knip, which claims to "Find unused files, dependencies and exports in JavaScript and TypeScript projects". It has a good write up on Smashing Magazine.

It is worth noting that Knip requires node >= 16, so temporarily switching node versions with nvm or equivalent was required in order to analyze the workspace. This did not seem to negatively affect the operation.

After adding knip as a devDependency and running it via npm run knip (required to be added as script property), the following output was observed:

Unused dependencies (21)

@material/button           package.json
@material/layout-grid      package.json
@material/linear-progress  package.json
@material/list             package.json
@material/select           package.json
@material/textfield        package.json
@material/theme            package.json
accessible-autocomplete    package.json
element-closest            package.json
formdata-polyfill          package.json
intl                       package.json
intl-locales-supported     package.json
natives                    package.json
polyfill-library           package.json
promise-polyfill           package.json
psl                        package.json
punycode                   package.json
svgxuse                    package.json
url-polyfill               package.json
uuid                       package.json
whatwg-fetch               package.json
Unused devDependencies (27)

@componentdriven/csf            package.json
@popperjs/core                  package.json
@testing-library/jest-dom       package.json
@wordpress/browserslist-config  package.json
@wordpress/jest-console         package.json
@wordpress/jest-preset-default  package.json
amphtml-validator               package.json
archiver                        package.json
babel-jest                      package.json
dockerode                       package.json
element-internals-polyfill      package.json
eslint-scope                    package.json
expect-puppeteer                package.json
fetch-mock-jest                 package.json
file-loader                     package.json
fs-extra                        package.json
git-repo-info                   package.json
glob                            package.json
husky                           package.json
jest-localstorage-mock          package.json
jest-puppeteer                  package.json
jsdom                           package.json
jsdom-testing-mocks             package.json
lint-staged                     package.json
puppeteer-core                  package.json
sanitize-filename               package.json
webpack-stream                  package.json
Unused exports in namespaces (52)

isStorageAvailable                      assets/js/googlesitekit/api/cache.js:106:14                                
initialState                            assets/js/googlesitekit/datastore/forms/index.js:35:14                     
actions                                 assets/js/googlesitekit/datastore/forms/index.js:36:14                     
controls                                assets/js/googlesitekit/datastore/forms/index.js:37:14                     
reducer                                 assets/js/googlesitekit/datastore/forms/index.js:38:14                     
resolvers                               assets/js/googlesitekit/datastore/forms/index.js:39:14                     
selectors                               assets/js/googlesitekit/datastore/forms/index.js:40:14                     
default                                 assets/js/googlesitekit/datastore/forms/index.js:46:8                      
initialState                            assets/js/googlesitekit/datastore/location/index.js:28:14                  
actions                                 assets/js/googlesitekit/datastore/location/index.js:29:14                  
controls                                assets/js/googlesitekit/datastore/location/index.js:30:14                  
reducer                                 assets/js/googlesitekit/datastore/location/index.js:31:14                  
resolvers                               assets/js/googlesitekit/datastore/location/index.js:32:14                  
selectors                               assets/js/googlesitekit/datastore/location/index.js:33:14                  
default                                 assets/js/googlesitekit/datastore/location/index.js:39:8                   
controls                                assets/js/googlesitekit/datastore/site/index.js:55:14                      
reducer                                 assets/js/googlesitekit/datastore/site/index.js:56:14                      
resolvers                               assets/js/googlesitekit/datastore/site/index.js:57:14                      
default                                 assets/js/googlesitekit/datastore/site/index.js:64:8                       
initialState                            assets/js/googlesitekit/datastore/ui/index.js:35:14                        
actions                                 assets/js/googlesitekit/datastore/ui/index.js:36:14                        
controls                                assets/js/googlesitekit/datastore/ui/index.js:37:14                        
reducer                                 assets/js/googlesitekit/datastore/ui/index.js:38:14                        
resolvers                               assets/js/googlesitekit/datastore/ui/index.js:39:14                        
selectors                               assets/js/googlesitekit/datastore/ui/index.js:40:14                        
default                                 assets/js/googlesitekit/datastore/ui/index.js:46:8                         
controls                                assets/js/googlesitekit/datastore/user/index.js:60:2                       
reducer                                 assets/js/googlesitekit/datastore/user/index.js:61:2                       
resolvers                               assets/js/googlesitekit/datastore/user/index.js:62:2                       
default                                 assets/js/googlesitekit/datastore/user/index.js:77:8                       
default                                 assets/js/googlesitekit/widgets/default-areas.js:39:8                      
default                                 assets/js/googlesitekit/widgets/default-contexts.js:32:8                   
getTagPermissionsNoAccess               assets/js/modules/analytics/datastore/__fixtures__/index.js:28:20          
getTimeColumnVaxisFormat                assets/js/modules/analytics/util/time-column-format.js:28:17               
isValidProfileID                        assets/js/modules/analytics/util/validation.js:104:29                      
pagespeedMobileNoFieldDataNoStackPacks  assets/js/modules/pagespeed-insights/datastore/__fixtures__/index.js:71:30 
defaultTagWeb                           assets/js/modules/tagmanager/datastore/__factories__/builders.js:145:14    
liveContainerVersionBuilder             assets/js/modules/tagmanager/datastore/__factories__/builders.js:227:14    
generateHTMLWithNoTag                   assets/js/modules/tagmanager/datastore/__factories__/html-with-tag.js:69:17
isValidDateInstance                     assets/js/util/date-range/index.js:22:9                                    
getCurrentDateRangeDayCount             assets/js/util/date-range/index.js:25:9                                    
createDurationFormat                    assets/js/util/i18n.js:125:14                                              
prepareForReadableLargeNumber           assets/js/util/i18n.js:224:14                                              
readableLargeNumber                     assets/js/util/i18n.js:247:14                                              
storageAvailable                        assets/js/util/storage.js:27:14                                            
getStorage                              assets/js/util/storage.js:93:14                                            
searchConsole                           tests/e2e/specs/modules/analytics/fixtures/admin-bar.js:1:9                
disableFeature                          tests/e2e/utils/features.js:32:23                                          
screenshot                              tests/e2e/utils/step-and-screenshot.js:35:23                               
clearScreenshots                        tests/e2e/utils/step-and-screenshot.js:98:23                               
registerAllStoresOn                     tests/js/utils.js:469:14                                                   
subscribeWithUnsubscribe                tests/js/utils.js:476:14
Duplicate exports (1)

collect, collectActions, collectContr...  assets/js/googlesitekit/data/utils.js

Many of these, such as test related files, may be false positives.

Conditional Loading of Modules

It may be worth looking to see if there are any modules that can be conditionally loaded only when used, using something like Suspense.

SVG Optimization

SVG's appear to be optimized in terms of only importing modularly what is needed, and when needed, therefore the SVG's are not bundled within all modules, and only when required.

mxbclang commented 8 months ago

Reassigning to @eugene-manuilov and @techanvil for review.

eugene-manuilov commented 3 weeks ago

@aaemnnosttv @tofumatt @techanvil

I have made an experiment and combined all assets except gm2 and gm3 components into one bundle and compared the size of the resulting files. So far we can reduce the size of the resulting bundle in a bit more than 2 times:

npm run test:analyze

Before: image

After: image

Plus, if we can remove unused dependencies that @10upsimon mentioned in his research, we can get the final bundle even more small. Should we re-consider this task to update our module assets to be bundled into a single file? Moving this task back to AC.

aaemnnosttv commented 1 day ago

Thanks @eugene-manuilov! It looks like there is a lot of opportunity for efficiency gains for sure, although we may want to keep a few entry points and leverage more code splitting using dynamic imports to avoid duplicate bundling rather than keeping only a single main bundle. For example, there is a lot of JS we don't need to load for the admin bar app.

As for the unused dependencies reported in @10upsimon 's findings, I think there are likely many false positives there due to the way we import things with aliases or other things as there are a number of things I can see which aren't accurate there. As for unused devDependencies, those are likely from tests and things but those shouldn't be included in bundles anyways.

So I think this is worth moving forward with, but not necessarily moving to a single entry point for all JS.

On a related note, I believe the plugin version is injected into the vendor bundle like others which causes its content hash to change on every release, even when no vendor dependencies have changed. If we can prevent this, that would be a nice enhancement to avoid re-downloading this on every update since this value would never be used/needed in vendor deps.