Add vendor bundling of blocks frontend bundle to improve cart/checkout performance #45859

samueljseay · 2024-03-23T03:55:30Z

Changes proposed in this Pull Request:

Recently I discovered while reviewing the cart and checkout webpack builds that there was a relatively low-hanging opportunity for optimization.

Specifically we don't do any vendor bundling in the front-end webpack bundle. There are so many entrypoints in this bundle, so even one small dependency being duplicated across all of them, drastically impacts the payload size for cart and checkout. This is especially true because checkout for example, has many inner blocks that are all entrypoints which will all end up duplicating any vendor dependency.

Furthermore, the blocks-checkout and blocks-components scripts that were being built in a separate bundle were not able to make use of vendor bundling because most of their dependencies were duplicated from the front-end bundle.

So to solve this, I moved those bundles across to the front-end bundle and turned on vendor bundling of node_modules there. On a production build, in testing, this reduced the payload of the cart page by 11% and the checkout page by 17.9%.

Closes #45680

How to test the changes in this Pull Request:

I am leaning in on the e2e tests here to see that this still works. For tester though you should do a smoke test of:

Shop page and functionality on frontend
Cart page and functionality on frontend
Checkout page and functionality on frontend

And also you should test all of the above in the editor as well. Try changing block settings, add / remove the blocks and ensure that no JavaScript exceptions have been thrown.

Changelog entry

Automatically create a changelog entry from the details below.

Significance

Patch
Minor
Major

Type

Fix - Fixes an existing bug
Add - Adds functionality
Update - Update existing functionality
Dev - Development related task
Tweak - A minor adjustment to the codebase
Performance - Address performance issues
Enhancement - Improvement to existing functionality

Message

Introduce vendor bundling to the blocks cart and checkout pages to improve performance.

Comment

…fig.

github-actions · 2024-03-23T03:56:05Z

Hi @sunyatasattva,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

github-actions · 2024-03-23T04:12:58Z

Test Results Summary

Commit SHA: 3688f6b

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 38s
E2E Tests	364	0	0	9	0	373	9m 21s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

…cing issues.

… bundles.

…mmerce-blocks, woocommerce

samueljseay · 2024-03-26T04:09:09Z

plugins/woocommerce-blocks/bin/webpack-configs.js

+						name: 'wc-blocks-vendors',
+						chunks: ( chunk ) => {
+							return (
+								chunk.name !== 'product-button-interactivity'


For some reason, turning on vendor bundling in the product-button-interactivity bundle caused some imports of that bundle to be tree-shaken. I've opted just not to vendor bundle for that at the moment. It was the only place there was an issue. The chunk is loaded in the shop page anyway so won't have an impact on cart and checkout. Also there will be some refactoring to remove the dependency on @wordpress/components that this has (due to using the woo notice component), when that comes up i'll revisit if this one can (or needs) to be further optimized.

EDIT, actually its worth mentioning that once @wordpress/components is removed from this block, there won't be anything significant to vendor bundle anyway. Apart from that it's very, very small.

samueljseay · 2024-03-26T04:09:40Z

plugins/woocommerce-blocks/bin/webpack-configs.js

@@ -378,20 +383,17 @@ const getFrontConfig = ( options = {} ) => {
 				minSize: 200000,
 				automaticNameDelimiter: '--',
 				cacheGroups: {
-					...getCacheGroups(),
-					'base-components': {


This was duplicated from an identical entry generated by getCacheGroups so this is tidy up.

samueljseay · 2024-03-26T04:10:32Z

plugins/woocommerce-blocks/bin/webpack-configs.js

@@ -327,7 +331,8 @@ const getFrontConfig = ( options = {} ) => {
 			// @see https://github.com/Automattic/jetpack/pull/20926
 			chunkFilename: `[name]-frontend${ fileSuffix }.js?ver=[contenthash]`,
 			filename: `[name]-frontend${ fileSuffix }.js`,
-			uniqueName: 'webpackWcBlocksJsonp',
+			uniqueName: 'webpackWcBlocksFrontendJsonp',
+			library: [ 'wc', '[name]' ],


Webpack DEWP is actually non-functional in most of the bundles right now, because you must set a library export name if you want dependencies to be externalized. By setting this we enable DEWP on this bundle.

@samueljseay can you help me understand this? What does it mean for DEWP to be non-functional in the bundles? As in it's not resolving imports e.g. @woocommerce/blocks-checkout to window.wc.blocksCheckout?

@opr yes sorry this is sort of misleading, I think I made a mistake saying it's non-functional. It's just that without this, it won't externalize any dependency to the window that is declared in this bundle.

Maybe that was intentional originally, to just have it load things from the window but not need to externalize anything itself in bundles outside of the "main" bundle.

Thanks Sam, ah I see what you mean, so anything we want to put onto the winow.wc wasn't going there from this package?

samueljseay · 2024-03-26T04:12:29Z

plugins/woocommerce-blocks/bin/webpack-configs.js

@@ -327,7 +331,8 @@ const getFrontConfig = ( options = {} ) => {
 			// @see https://github.com/Automattic/jetpack/pull/20926
 			chunkFilename: `[name]-frontend${ fileSuffix }.js?ver=[contenthash]`,
 			filename: `[name]-frontend${ fileSuffix }.js`,
-			uniqueName: 'webpackWcBlocksJsonp',
+			uniqueName: 'webpackWcBlocksFrontendJsonp',


When I initially started optimizing the build i got runtime errors because we mistakenly had the same unique name for different bundles. This is a no-no. If 2 bundles on the same page have same unique name they can accidentally load packages from each others bundles and this was happening to me. The front end bundle would try load from the main vendor bundle rather than the frontend vendor bundle, causing issues if some parts of a dependency were tree shaken in that vendor bundle but not in mine. I think for consistency each entry needs a unique name to avoid this kind of conflict in future.

This makes sense, I don't see any issues with changing the names to be unique.

samueljseay · 2024-03-26T04:12:59Z

plugins/woocommerce-blocks/bin/webpack-entries.js

@@ -236,6 +234,10 @@ const entries = {
 	frontend: {
 		reviews: './assets/js/blocks/reviews/frontend.ts',
 		...getBlockEntries( 'frontend.{t,j}s{,x}' ),
+
+		blocksCheckout: './packages/checkout/index.js',


Move these entry points to the frontend bundle.

samueljseay · 2024-03-26T04:13:54Z

...sts/e2e/tests/cart/cart-checkout-block-translations.shopper.block_theme.side_effects.spec.ts

+		await beanieAddToCartButton.click();
+
+		// Add to cart initiates a request that could be interrupted by navigation, wait till it's done.
+		await expect( beanieAddToCartButton ).toHaveText( /in winkelwagen/ );


These tests had a race condition that was exposed, I guess by slight changes in the bundling. If you navigate before the request to add to cart has finished you might interrupt it, so this waits for that to ensure these pass.

samueljseay · 2024-03-26T04:14:20Z

plugins/woocommerce/src/Blocks/AssetsController.php

@@ -61,8 +61,9 @@ public function register_assets() {
 		// The price package is shared externally so has no blocks prefix.
 		$this->api->register_script( 'wc-price-format', 'assets/client/blocks/price-format.js', array(), false );

-		$this->api->register_script( 'wc-blocks-checkout', 'assets/client/blocks/blocks-checkout.js', array() );
-		$this->api->register_script( 'wc-blocks-components', 'assets/client/blocks/blocks-components.js', array() );
+		$this->api->register_script( 'wc-blocks-vendors-frontend', $this->api->get_block_asset_build_path( 'wc-blocks-vendors-frontend' ), array(), false );


Add the new vendor bundle as a dependency to blocks and checkout scripts.

senadir · 2024-03-26T10:42:04Z

Furthermore, the blocks-checkout and blocks-components scripts that were being built in a separate bundle were not able to make use of vendor bundling because most of their dependencies were duplicated from the front-end bundle.

I'm not sure I understand this, does it mean that they have a lot of shared things that you combined into the vendor script?

samueljseay · 2024-03-26T11:44:32Z

@senadir not very well worded on my part, and probably incomplete explanation.

What I'm trying to say is that moving blocks-checkout and blocks-components to the frontend bundle allows us to get the most benefit from vendor bundles because within the bundle they were in they don't share so many dependencies with other entrypoints, but rather introduce a lot of dependencies just for those 2 files, but those dependencies they introduce happen to in many cases already be being pulled in to the frontend bundle for the other entrypoints there. So when we bundle them in frontend bundle we get the maximum kb savings from vendor bundling there. (We can't share vendor scripts across bundles either, i think you know that but just mentioning it for anyone who isn't familiar with Webpack).

Aljullu

Thanks for working on these improvements, @samueljseay! I could see the bundle size improvements in the Cart and the Checkout pages and they are pretty impressive, good job!

I just have a couple of comments/questions:

The bundle size seems to have increased in some other simpler blocks. Ie: I tested adding the Reviews by Category and the All Products blocks in two different pages. When comparing trunk with this branch, it looks like the total bundle size to render those blocks has increased slightly. I don't think it's a big deal, as this seems to only happen when pages have one or very few blocks and most of that extra size comes from blocks-vendors-frontend so it will be cached across pages. But wanted to mention anyway. 🙂
If I'm understanding the code right, we are assuming all frontend scripts will have a dependency on either blocks-checkout or blocks-components and, because of that, blocks-vendors-frontend will be enqueued. However, some time in the future there might be a block with a frontend script that imports from a node_modules package but doesn't import from blocks-checkout nor blocks-components. In that case, we would need to manually declare blocks-vendors-frontend as a dependency, is that correct?

This brings me to the third question. This PR extracts the vendor scripts from all frontend packages and creates the wc-blocks-vendors-frontend script. I wonder if you considered doing the same but instead of bundling the vendor scripts of all frontend packages doing it only for the blocks-checkout and blocks-components scripts. In other words, instead of using the frontend Webpack entry, we would create a new separate one for blocks-checkout and blocks-components. I have no idea if that would make much sense or if you had already evaluated that and discarded for some reason, but wanted to mention it to know your thoughts.

None of these comments are blockers neither suggestions to change, I'm mostly asking them to better understand why we prefer this approach. Also keep in mind it was a long time since I worked on the Webpack config so I might be missing a lot of context. 😅

Btw, thanks for adding inline comments explaining the code changes, they were super helpful when reviewing the PR!

opr

@samueljseay thanks a lot for working on this! The changes you've made make sense to me, and I tested the Cart/Checkout blocks, I also tested some of the shop/add to cart flow, such as filter blocks but my work there was not exhaustive.

Before merging this, I propose we invite Rubik/Kirigami/Origami to really test the flows their team is responsible using various different settings.

samueljseay · 2024-03-29T02:11:49Z

@Aljullu

The bundle size seems to have increased in some other simpler blocks. Ie: I tested adding the Reviews by Category and the All Products blocks in two different pages. When comparing trunk with this branch, it looks like the total bundle size to render those blocks has increased slightly. I don't think it's a big deal, as this seems to only happen when pages have one or very few blocks and most of that extra size comes from blocks-vendors-frontend so it will be cached across pages. But wanted to mention anyway.

Yes thanks for calling this out, you're right that if a standalone entry with very few dependencies from the vendor bundle loads it will be overall larger than before. As you said, vendor is cached across pages, but it certainly is a trade-off. In future we could split off more bundles if we can find a logical way to separate significant vendor dependencies.

If I'm understanding the code right, we are assuming all frontend scripts will have a dependency on either blocks-checkout or blocks-components and, because of that, blocks-vendors-frontend will be enqueued. However, some time in the future there might be a block with a frontend script that imports from a node_modules package but doesn't import from blocks-checkout nor blocks-components. In that case, we would need to manually declare blocks-vendors-frontend as a dependency, is that correct?

Exactly. And I think this problem also exists with the other bundle that uses a vendor script too (can't remember off top of my head if its core or main bundle). If you don't consider that your entrypoint has dependency on wc-blocks it will also fail. It's something to be careful of for sure, but if we're adding entrypoints, it will be immediately obvious that your entry point doesn't work during development. So my biggest concern would be an existing entrypoint somehow losing its dependency on both of these 2 scripts. I don't have a good way to ensure this because the vendor bundling is manual, I kinda thought DEWP would handle vendor bundling but it doesn't. Maybe there is a better way to solve this in future.

This brings me to the third question. This PR extracts the vendor scripts from all frontend packages and creates the wc-blocks-vendors-frontend script. I wonder if you considered doing the same but instead of bundling the vendor scripts of all frontend packages doing it only for the blocks-checkout and blocks-components scripts. In other words, instead of using the frontend Webpack entry, we would create a new separate one for blocks-checkout and blocks-components. I have no idea if that would make much sense or if you had already evaluated that and discarded for some reason, but wanted to mention it to know your thoughts.

The reason imo not to do this, is because even though some small dependencies do get a little bigger when they share almost no dependencies with the vendors script, is that most of the scripts in the frontend bundle still benefit. There are many inner blocks of the cart and checkout pages that get loaded and before they were duplicating the same dependencies over and over. The cumulative payload optimization is not just the reduction in size of blocks-components and blocks-checkout but also in entrypoints such checkout-blocks/shipping-address-frontend et al, they also get to share the vendor dependencies instead of bundling them once per entrypoint. I think this is also why checkout benefits the most. It has a bunch of inner blocks that all duplicate vendor deps. With these changes checkout drops 17% in size. I think in future we should be able to uncover ways to further optimize bundling and splitting though. And then you might think maybe we should have cart and checkout bundles, but alas, they still share so much thanks to blocks-components, so I don't think this splitting could be optimized too much more right now and blocks-components/checkout having their own bundle will make performance overall worse imho.

I have more ideas for optimization of our bundles though. A timely update of the wordpress polyfill for example should shed quite a lot of weight from all our frontend packages, and the follow up of replacing wordpress components with ariakit will make things smaller again.

samueljseay · 2024-03-29T02:23:58Z

Before merging this, I propose we invite Rubik/Kirigami/Origami to really test the flows their team is responsible using various different settings.

@opr I agree! 💯 I'll put out a call for testing this next week.

Aljullu

Thanks for the in-depth explanation, @samueljseay! ❤️ It makes a lot of sense, I see some trade-offs need to be taken, but I agree that overall this is an improvement to what we have right now.

I'm approving from my side, but it would be great to get more people to review and test it to make sure everything works well.

nerrad · 2024-04-01T10:22:21Z

If I'm understanding the code right, we are assuming all frontend scripts will have a dependency on either blocks-checkout or blocks-components and, because of that, blocks-vendors-frontend will be enqueued. However, some time in the future there might be a block with a frontend script that imports from a node_modules package but doesn't import from blocks-checkout nor blocks-components. In that case, we would need to manually declare blocks-vendors-frontend as a dependency, is that correct?

This has me wondering about the potential impact on extenders that have written scripts that a dependency on any specific block frontend scripts for individual blocks prior to this change. If those individual frontend scripts already had a dependency of blocks-checkout or blocks-components then there will be no issue. So it's likely this isn't a significant concern but still seems like something that should have a dev note included for the next release. At the minimum, it would showcase a nice performance boost.

sunyatasattva · 2024-04-01T12:03:50Z

Thanks @samueljseay for the great work! I have smoke tested all the non-C&C blocks on the editor-side and played around with various settings. No broken behaviour or runtime errors detected. Of course, I didn't go the deepest possible, but I played around with pretty much everything non-C&C.

samueljseay · 2024-04-02T02:01:08Z

This has me wondering about the potential impact on extenders that have written scripts that a dependency on any specific block frontend scripts for individual blocks prior to this change. If those individual frontend scripts already had a dependency of blocks-checkout or blocks-components then there will be no issue. So it's likely this isn't a significant concern but still seems like something that should have a dev note included for the next release. At the minimum, it would showcase a nice performance boost.

Its a good point. I couldn't think of too many situations where you'd end up in this position without the vendor scripts loading, but I agree even the small possibility of an edge case requires that we communicate this. I will look at ensuring we get a dev note included in the next release and I agree we can also showcase the size reduction in C&C especially. 🙏🏻

samueljseay · 2024-04-03T02:54:25Z

@opr I have done some testing of the C&C.

I explored the UI from front-end and editor, I adjusted settings of inner blocks within the C&C and saw no runtime issues or odd behaviours.

samueljseay · 2024-04-04T02:30:51Z

I'm merging this now. We need to keep a close eye on any issues springing up on this between now and next release.

…t performance (#45859)

…/checkout performance (#45859)" This reverts commit ff102c4.

samueljseay added 3 commits March 22, 2024 20:12

WIP vendoring and consolidating bundles.

b164a36

Enable externalizing blocks components and checkout from frontend con…

2c10937

…fig.

Target all node modules.

a568081

github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 23, 2024

woocommercebot requested review from a team and thealexandrelara and removed request for a team March 23, 2024 03:55

samueljseay removed the request for review from thealexandrelara March 23, 2024 03:58

woocommerce deleted a comment from github-actions bot Mar 23, 2024

samueljseay and others added 3 commits March 23, 2024 12:30

Don't vendor split for product-button-interactivity, to avoid introdu…

bd293dd

…cing issues.

Give each bundle a unique name to ensure no runtime conflicts between…

cd2d311

… bundles.

Add changefile(s) from automation for the following project(s): wooco…

fbd7cc8

…mmerce-blocks, woocommerce

samueljseay closed this Mar 25, 2024

samueljseay reopened this Mar 25, 2024

samueljseay added 3 commits March 25, 2024 18:29

Reintroduce fix for product-button-interactivity.

cf794c4

Merge remote-tracking branch 'origin/trunk' into dev/attempt-vendoring

969259c

Fix flakyness in cart/checkout translation e2e tests.

0def7ed

samueljseay commented Mar 26, 2024

View reviewed changes

Adjust file naming to match old bundle to avoid breaking translations.

3688f6b

samueljseay changed the title ~~[WIP] Adjust bundling / vendoring to improve cart/checkout performance~~ Add vendor bundling of blocks frontend bundle to improve cart/checkout performance Mar 26, 2024

samueljseay requested a review from opr March 27, 2024 05:07

opr requested a review from Aljullu March 27, 2024 10:38

Aljullu reviewed Mar 27, 2024

View reviewed changes

opr reviewed Mar 27, 2024

View reviewed changes

opr mentioned this pull request Mar 29, 2024

Replace @wordpress/components Button, Radio, RadioGroup with Ariakit Button #45974

Merged

11 tasks

Aljullu approved these changes Apr 1, 2024

View reviewed changes

sunyatasattva self-requested a review April 1, 2024 12:02

sunyatasattva approved these changes Apr 1, 2024

View reviewed changes

samueljseay merged commit ff102c4 into trunk Apr 4, 2024
64 checks passed

samueljseay deleted the dev/attempt-vendoring branch April 4, 2024 02:31

github-actions bot added this to the 8.9.0 milestone Apr 4, 2024

github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Apr 4, 2024

samueljseay added needs: dev note PR that has some text that needs to be included in the release notes. needs: analysis Indicates if the PR requires a PR testing scrub session. and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Apr 4, 2024

senadir pushed a commit that referenced this pull request Apr 4, 2024

Add vendor bundling of blocks frontend bundle to improve cart/checkou…

882c461

…t performance (#45859)

mattallan mentioned this pull request May 15, 2024

Cart & Checkout Blocks not working on stores with WC 8.9 + Jetpack Boost's "Concatenate JS" feature #47492

Open

5 tasks

vedanshujain added a commit that referenced this pull request May 15, 2024

Revert "Add vendor bundling of blocks frontend bundle to improve cart…

d8938ff

…/checkout performance (#45859)" This reverts commit ff102c4.

nigeljamesstevenson added the release: highlight Issues that have a high user impact and need to be discussed/paid attention to. label May 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vendor bundling of blocks frontend bundle to improve cart/checkout performance #45859

Add vendor bundling of blocks frontend bundle to improve cart/checkout performance #45859

samueljseay commented Mar 23, 2024 •

edited

github-actions bot commented Mar 23, 2024 •

edited

github-actions bot commented Mar 23, 2024 •

edited

samueljseay Mar 26, 2024 •

edited

samueljseay Mar 26, 2024

samueljseay Mar 26, 2024

opr Mar 27, 2024 •

edited

samueljseay Mar 29, 2024 •

edited

opr Mar 29, 2024

samueljseay Mar 26, 2024

opr Mar 27, 2024

samueljseay Mar 26, 2024

samueljseay Mar 26, 2024

samueljseay Mar 26, 2024

senadir commented Mar 26, 2024

samueljseay commented Mar 26, 2024 •

edited

Aljullu left a comment

opr left a comment

samueljseay commented Mar 29, 2024 •

edited

samueljseay commented Mar 29, 2024

Aljullu left a comment

nerrad commented Apr 1, 2024

sunyatasattva commented Apr 1, 2024

samueljseay commented Apr 2, 2024

samueljseay commented Apr 3, 2024

samueljseay commented Apr 4, 2024

Add vendor bundling of blocks frontend bundle to improve cart/checkout performance #45859

Add vendor bundling of blocks frontend bundle to improve cart/checkout performance #45859

Conversation

samueljseay commented Mar 23, 2024 • edited

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Changelog entry

Significance

Type

Message

Comment

github-actions bot commented Mar 23, 2024 • edited

github-actions bot commented Mar 23, 2024 • edited

Test Results Summary

samueljseay Mar 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opr Mar 27, 2024 • edited

Choose a reason for hiding this comment

samueljseay Mar 29, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

senadir commented Mar 26, 2024

samueljseay commented Mar 26, 2024 • edited

Aljullu left a comment

Choose a reason for hiding this comment

opr left a comment

Choose a reason for hiding this comment

samueljseay commented Mar 29, 2024 • edited

samueljseay commented Mar 29, 2024

Aljullu left a comment

Choose a reason for hiding this comment

nerrad commented Apr 1, 2024

sunyatasattva commented Apr 1, 2024

samueljseay commented Apr 2, 2024

samueljseay commented Apr 3, 2024

samueljseay commented Apr 4, 2024

samueljseay commented Mar 23, 2024 •

edited

github-actions bot commented Mar 23, 2024 •

edited

github-actions bot commented Mar 23, 2024 •

edited

samueljseay Mar 26, 2024 •

edited

opr Mar 27, 2024 •

edited

samueljseay Mar 29, 2024 •

edited

samueljseay commented Mar 26, 2024 •

edited

samueljseay commented Mar 29, 2024 •

edited