-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Fix: only dequeue legacy assets for block themes #60758
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
Fix: only dequeue legacy assets for block themes #60758
Conversation
Testing GuidelinesApart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
📝 WalkthroughWalkthroughAdds a changelog entry and updates BlockTemplatesController to early-return from dequeue_legacy_scripts when wp_is_block_theme() is false, so legacy single-product scripts are only dequeued for block themes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant WP as WordPress Theme
participant BTC as BlockTemplatesController
participant Assets as Legacy Scripts
User->>WP: Request single product page
WP->>BTC: dequeue_legacy_scripts()
alt Non-block theme
BTC->>BTC: wp_is_block_theme() == false
note right of BTC #F8ECD1: Early return — no dequeue
BTC-->>WP: Return
else Block theme
BTC->>BTC: wp_is_block_theme() == true
BTC->>Assets: Dequeue `wc-single-product` (legacy)
Assets-->>BTC: Script dequeued
BTC-->>WP: Return
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/woocommerce/src/Blocks/BlockTemplatesController.php (2)
41-43: Style nit: add space afterifper WP coding standards.Keeps consistency with the rest of the file.
- if( ! wp_is_block_theme() ) { + if ( ! wp_is_block_theme() ) {
41-43: Add regression coverage for both theme types.
- Classic theme with block-template-parts support: ensure
wc-single-productremains enqueued and the gallery initializes.- Block theme: ensure the script is dequeued on product pages.
Note: Based on retrieved learnings, keep any existing hard-coded “woocommerce/woocommerce//” template IDs/slugs in e2e tests as-is for backwards-compat checks.
I can draft a minimal e2e test pair exercising these paths. Want me to open a follow-up PR with the tests?
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
plugins/woocommerce/changelog/60758-wooplug-5478-add_theme_supportwoocommerce-breaks-product-image-gallery-in(1 hunks)plugins/woocommerce/src/Blocks/BlockTemplatesController.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
plugins/woocommerce/src/Blocks/BlockTemplatesController.php
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit configuration file
**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
plugins/woocommerce/src/Blocks/BlockTemplatesController.php
🧠 Learnings (3)
📚 Learning: 2025-08-14T08:19:54.208Z
Learnt from: Aljullu
PR: woocommerce/woocommerce#60191
File: plugins/woocommerce/src/Blocks/BlockTemplatesRegistry.php:93-111
Timestamp: 2025-08-14T08:19:54.208Z
Learning: WooCommerce core template files (.html files in src/Blocks/templates/wp_template/) are guaranteed to exist as they are provided by WC core, so defensive file_exists() checks are not necessary when loading these templates in BlockTemplatesRegistry.php.
Applied to files:
plugins/woocommerce/src/Blocks/BlockTemplatesController.php
📚 Learning: 2025-08-14T08:20:36.357Z
Learnt from: Aljullu
PR: woocommerce/woocommerce#60191
File: plugins/woocommerce/client/blocks/tests/e2e/tests/single-product-template/single-product-template-compatibility-layer.spec.ts:4-4
Timestamp: 2025-08-14T08:20:36.357Z
Learning: In WooCommerce e2e tests, some test files intentionally use hard-coded "woocommerce/woocommerce//" template IDs instead of BLOCK_THEME_SLUG to test backwards compatibility with the legacy plugin-scoped template ID format. These should not be updated to use the dynamic slug constant as they serve a specific testing purpose.
Applied to files:
plugins/woocommerce/src/Blocks/BlockTemplatesController.php
📚 Learning: 2025-08-14T08:22:34.886Z
Learnt from: Aljullu
PR: woocommerce/woocommerce#60191
File: plugins/woocommerce/client/blocks/tests/e2e/tests/product-filters/rating-filter-editor.block_theme.spec.ts:36-36
Timestamp: 2025-08-14T08:22:34.886Z
Learning: In WooCommerce e2e tests, some test files intentionally use hard-coded "woocommerce/woocommerce//" template slugs (not just template IDs) instead of BLOCK_THEME_SLUG to test backwards compatibility with the legacy plugin-scoped template slug format. These should not be updated to use the dynamic slug constant as they serve a specific backwards compatibility testing purpose.
Applied to files:
plugins/woocommerce/src/Blocks/BlockTemplatesController.php
🔇 Additional comments (2)
plugins/woocommerce/changelog/60758-wooplug-5478-add_theme_supportwoocommerce-breaks-product-image-gallery-in (1)
1-4: Changelog entry reads well and matches the fix scope.Accurately describes the patch and impact. Nothing else to do.
plugins/woocommerce/src/Blocks/BlockTemplatesController.php (1)
41-43: LGTM: Correctly gate dequeuing to block themes.This early return prevents breaking classic themes that only support block-template-parts. This should resolve the gallery regression while preserving the intended behavior for block themes.
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dinhtungdu, works as expected! Appreciate you responded in the original issue and provided a fix so quickly! 🙏
I think this requires backporting to 10.2 so please add 10.2 milestone before merging!
* fix: only dequeue legacy assets for block themes * Add changefile(s) from automation for the following project(s): woocommerce * chore: lint --------- Co-authored-by: github-actions <github-actions@github.com>
|
IMPORTANT: Merging this PR to the appropriate branches is critical to the release process and ensures that the bug does not cause regressions in the future releases. Cherry picking was successful for |
…themes (#60781) * Fix: only dequeue legacy assets for block themes (#60758) * fix: only dequeue legacy assets for block themes * Add changefile(s) from automation for the following project(s): woocommerce * chore: lint --------- Co-authored-by: github-actions <github-actions@github.com> * Add changefile(s) from automation for the following project(s): woocommerce --------- Co-authored-by: Tung Du <dinhtungdu@gmail.com> Co-authored-by: github-actions <github-actions@github.com>
Submission Review Guidelines:
Changes proposed in this Pull Request:
In #60223, we disabled the legacy single script for block themes. However, we overlooked a case where classic themes support
block-template-parts. This causes issues with the Product Image Gallery in classic themes that have this support. This PR addresses the problem by ensuring we only dequeue the legacy script for block themes.Closes #60750
(For Bug Fixes) Bug introduced in PR #60223
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
block-template-partssupport, like Bootstore provided by @crftwrk in `add_theme_support('woocommerce') breaks Product Image Gallery in single product page (WC 10.2) #60750.Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Fix: ensure we only dequeue legacy assets for block themes.
Changelog Entry Comment
Comment