Fix: WooCommerce Blocks causing malfunction of the navigation block on WordPress 6.3 #10283
Conversation
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/blocks/classic-template/archive-product.ts
assets/js/blocks/classic-template/product-search-results.ts assets/js/blocks/product-categories/block.tsx assets/js/blocks/product-collection/inspector-controls/inherit-query-control.tsx assets/js/editor-components/template-notice/index.tsx assets/js/templates/revert-button/index.tsx |
|
Size Change: -1.33 kB (0%) Total Size: 1.36 MB
ℹ️ View Unchanged
|
woocommercebot
requested review from
a team and
thealexandrelara
and removed request for
a team
nefeline
added
type: bug
There was a problem hiding this comment.
One clue I found related to the original issue is that with WooCommerce not active, wp.editSite does not have a value in the post editor whereas when it is active it does. This indicates that something in WooCommerce is loaded in the post editor environment with a dependency on @wordpress/site-editor and this ultimately causes the error.
I notice that in your changes, you removed the templates/revert-button/index.tsx path from the side-effects array configuration in package.json. That file imports the @wordpress/site-editor package (and is the only file in all of Woo to do so). Likely not a coincidence and also indicates that the revert button functionality might not be working as expected in production builds (I don't see it in your testing notes).
package.json
Outdated
| @@ -27,8 +27,7 @@ | |||
| "./assets/js/blocks/filter-wrapper/register-components.ts", | |||
| "./assets/js/blocks/product-query/variations/**.tsx", | |||
| "./assets/js/blocks/product-query/index.tsx", | |||
| "./assets/js/blocks/product-query/inspector-controls.tsx", | |||
| "./assets/js/templates/revert-button/index.tsx" | |||
There was a problem hiding this comment.
What was this path removed from the side-effects configuration?
There was a problem hiding this comment.
Since revert-button/index.tsx is the only file that has a @wordpress/edit-site dependency. I suspect this is actually the cause of the issue, especially since it is imported as a root asset! It should only be imported/utilized in the site editor environment.
There was a problem hiding this comment.
Since revert-button/index.tsx is the only file that has a @wordpress/edit-site dependency. I suspect this is actually the cause of the issue, especially since it is imported as a root asset! It should only be imported/utilized in the site editor environment.
Oh, nice catch: that sounds like the cause of the problem here indeed!
What was this path removed from the side-effects configuration?
The revert button works as expected with and without having "./assets/js/templates/revert-button/index.tsx" in this side-effect: it looks like this was added so it can run on production builds. Tagging @gigitux for confirmation.
There was a problem hiding this comment.
Thanks for working on this!
Since revert-button/index.tsx is the only file that has a @wordpress/edit-site dependency. I suspect this is actually the cause of the issue, especially since it is imported as a root asset! It should only be imported/utilized in the site editor environment.
I was aware of this issue, I tried to use a dynamic import, but it didn't work. I wrote a message on the Gutenberg PR. I have no idea how to fix this in a good way, but I'm happy to help if needed!
The revert button works as expected with and without having "./assets/js/templates/revert-button/index.tsx" in this side-effect: it looks like this was added so it can run on production builds. Tagging @gigitux for confirmation.
True!
There was a problem hiding this comment.
The revert button works as expected with and without having "./assets/js/templates/revert-button/index.tsx" in this side-effect: it looks like this was added so it can run on production builds. Tagging @gigitux for confirmation.
That's odd, the described behaviour in this PR of the navigation block issue being resolved in production builds but not in development builds is consistent with the import being stripped out of production builds because it is considered a side-effect. This should be easily verifiable by checking to see if wp.editSite is available in the post editor with a production build of Woo Blocks.
There was a problem hiding this comment.
You are right; I may have hit caching on my initial tests. I just had an idea to circumvent this: I will try to conditionally load the necessary script via PHP rather than JS, as I suspect it's not possible to truly conditionally import a module directly in the JavaScript execution flow.
…le in the site-editor.
Interesting! That looks like the cause of the problem indeed, but I wonder what is the correct approach to follow, because based on the release notes for WordPress 6.3, importing from |
It is the only way to import that function :/ We should try to import the package with a dynamic import. I tried, but I wasn't able to do it 🤔 |
…ng on the template
Aljullu
force-pushed
the
10259-woocommerce-causing-malfunction-of-the-navigation-block-on-wordpress-63
branch
from
067326e to
2057f0f
Compare
… edited inside a template
Yeah, it's tricky. I was trying to figure out alternative approaches, but nothing came to mind up until now. Should we work on the revert button on a separate PR to unblock the patch release? |
We have times to fix the issue (I would say at least one week), is there a particular reason you want to do a patch release now? (I would prefer reduce the number of patch release) |
@gigitux Edit: I just noticed that when you attempted to solve this problem back in May, Riad mentioned over here that we:
Did you make any progress on that front? |
|
Update regarding the I made some attempts to solve the problem, including:
But none of those worked. We are following the recommendations from Gutenberg for the usage of the |
…e-navigation-block-on-wordpress-63
There was a problem hiding this comment.
What Jarda was getting at in his comment on the Gutenberg ticket is that we should be building a different entrypoint for the script that requires wp.edit-site so that script gets conditionally loaded in the site editor context. Right now revert-button, is a direct dependent of the wc-blocks-shared-context entry point and it shouldn't be.
A few concerns about the latest changes:
- The extra configuration for
@wordpress/edit-siteas an external is not needed. That's handled already in our configuration. - The changes to enqueueing
wc-blocks-shared-contextwith no dependencies is fragile (it should utilize anyassets.phpfile if it exists. The current implementation is fragile if in the future a dependency is added, it could easily be missed to have that dependency registered here. - Explicitly enqueuing
wp-edit-siteshould be unnecessary. If we break out a new entry point script forrevert_button, that script registration only on the site editor context should automatically take care of the correct dependencies assuming we're using the context of its relevantassets.phpfor getting them.
|
My apologies @nerrad : I should have moved this PR to draft/in progress as I'm still figuring things out: I was experimenting with webpack externals and a few other ideas: will ping you as soon as this is ready. |
|
Closing this out in favor of #10388 |
This PR solves the conflict preventing the core's navigation block from being correctly previewed in the editor while having WooCommerce 7.9.0 running alongside WordPress version 6.3.
Fixes #10259
Investigation and Details
This issue was introduced on WooCommerce Blocks version
10.3.0by #9468 . That PR was created to solve an issue where the Products (Beta) block would not be available in the editor inserter when runningnpm start.After reverting the changes that caused the conflict, and ensuring the mini-cart could still be inserted in the editor (as that block was also relying on the same
isSiteEditorPagehook), @Aljullu helped me out with circumventing other limitations found, such as ensuring we can still hide the Inherit query control in the post/page editor and insert the Products (Beta) block while runningnpm start.@Aljullu also brought up this work by @nerrad that fixed some issues related to
useSelect()in WC core which could be related to the problem we are facing here and served as an inspiration: woocommerce/woocommerce#37641If anyone has a detailed explanation for the cause of this issue I would highly appreciate it: is it because we were calling
useSelect()directly? If that's the case, why? Is it something else entirely?Important Note
In its current shape and form, this PR fixes the problem for all users while still allowing us to insert the impacted blocks on
npm start, the only caveat is that the navigation block itself still throws the same error exclusively onnpm start.Since this limitation impacts developers solely, and we need this fix to be included in the core of Woo (on version 8.0.0) ASAP as WordPress 6.3 will be released on August 8th, a follow-up issue is being opened to address this limitation separately.
Screenshots
User Facing Testing
wp core update --version=6.3-RC1 --forcefollowed bywp core update-db. Alternatively, you can also download the zip for the release over here.npm run build)WooCommerce Visibility
Performance Impact
Changelog