Deregister core cart/checkout scripts and styles when rendering the blocks #2842
Conversation
Aljullu
added
status: needs review
focus: performance
|
Size Change: +1.43 kB (0%) Total Size: 1.6 MB
ℹ️ View Unchanged
|
Aljullu
force-pushed
the
fix/2239-remove-core-cart-checkout-js-from-blocks
branch
from
cd56af7
to
fd30737
Compare
There was a problem hiding this comment.
Great work here Albert! Tests well and I like your implementation of the jQuery event translation layer.
I have a couple minor feedback comments. I tested this in chrome and I tested a new stripe CC purchase. I also tested with ChromePay (and switching chipping options in the express payment method). Experienced no regressions.
However, if you haven't already, it'd be good to test in different browsers before merging.
| const invalidateCartData = () => { | ||
| if ( cartItems.length === 0 ) { | ||
| dispatch( storeKey ).invalidateResolutionForStore(); | ||
| scrollToTop(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Nifty! While this works great for cartItems removed from the block, I wonder if we should also add an event listener for when an item is removed from the cart js widget and show the empty cart placeholder when the items in the cart are empty? If you agree, definitely can be done in another issue (I just thought it would be a good use of the jQuery translation layer as well)
There was a problem hiding this comment.
Good point, done in 4cfb838.
The Cart widget was not working for me in the Cart page, so to test I needed to create a post with the Cart block and the Cart widget in the sidebar:
| export const translateJQueryEventToNative = ( | ||
| jQueryEventName, | ||
| nativeEventName, | ||
| bubbles = false, | ||
| cancelable = false | ||
| ) => { |
| jQuery( document ).on( jQueryEventName, () => { | ||
| dispatchEvent( nativeEventName, bubbles, cancelable ); | ||
| } ); |
There was a problem hiding this comment.
I think we'll still want to add some handling if jQuery is NOT present in the window. There will be effort to eliminate jQuery usage so we'll want to gracefully handle when it's not present. Can you add a check here for that?
There was a problem hiding this comment.
Makes sense. Added the check in 35f7a60.
Co-authored-by: Darren Ethier <darren@roughsmootheng.in>
nerrad
force-pushed
the
fix/2239-remove-core-cart-checkout-js-from-blocks
branch
from
4cfb838
to
d487a5e
Compare
| jQuery( document ).on( jQueryEventName, () => { | ||
| dispatchEvent( nativeEventName, bubbles, cancelable ); | ||
| } ); |
There was a problem hiding this comment.
I'm wondering, does this jQuery event ever need to be unregistered (using off) or is that handled because the native event dispatched via dispatchEvent might already be removed in the dom (just a confidence check question here)?
There was a problem hiding this comment.
That's something that was bothering me too. I was relying on the fact that the jQuery event would be cleared out when navigating to another page. But I think it makes more sense registering and unregistering it with component mount/unmount.
I refactored it in c708db7, so the useEffect takes care of adding and removing the jQuery → native event subscriptions when it's mounted and unmounted.
Fixes #2239
This PR deregisters the scripts and styles used by cart and checkout shortcodes when rendering the blocks. I took the list of scripts/styles to deregister from these lines in Core:
https://github.com/woocommerce/woocommerce/blob/master/includes/class-wc-frontend-scripts.php#L363-L377
It looks like we were relying on some logic from
cart.jsto redirect the empty cart to the full cart when a product was added. I needed to rewrite that logic in the block. In order to do that, I needed to subscribe to a jQuery event since that's what's used in Core. I added a translation layer to convert jQuery events to JS native events. While not ideal, that's the best solution I could find given the current situation where core uses jQuery.How to test the changes in this Pull Request:
Test Empty cart redirects to the Full cart when a product is added:
full cartview.[products limit="3" columns="3" visibility="featured" ]).Note: It's not possible to test this flow with the All Products block because of #2836.
Test there are no regressions in the purchase flow:
Changelog