Repository navigation
fix(classicpress): register wp-polyfill fallback when missing - #550
faisalahammad wants to merge 2 commits into
Conversation
Generated asset files list wp-polyfill as a dependency of every SCF script. ClassicPress never registers that handle so WordPress silently drops the whole acf chain and the footer inline code throws acf is not defined. Register a no-op fallback when the handle is missing, mirroring the existing react-jsx-runtime polyfill. On normal WordPress the handle already exists so this is a no-op. Fixes WordPress#285
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Remove the acf deps assertion that reads the gitignored acf.asset.php file. The PHPUnit workflow has no build step, so the asset file is absent in CI and the acf script has no wp-polyfill dependency there. The test now asserts only that the fallback registers wp-polyfill. Update the openPreviewPage fixture to click the preview menu item with a regex that matches both the old and the renamed WP trunk label. The pinned helper version only knows the old label, which times out the trunk E2E shards. Refs WordPress#550
faisalahammad
left a comment
faisalahammad
left a comment
There was a problem hiding this comment.
Two CI failures fixed. Pushed as 57686b6.
| Check | Root cause | Fix |
|---|---|---|
| PHPUnit | The new test checked the acf script deps for wp-polyfill. That dep list comes from acf.asset.php, which is gitignored. The PHPUnit workflow has no build step, so the file is missing in CI and the check failed there only. |
The test now checks only that the fallback registers wp-polyfill. That part needs no asset file. |
| E2E on WP trunk | Gutenberg renamed the preview menu item to "Preview (opens in a new tab)". The pinned test helper only looks for the old label "Preview in new tab", so every preview test timed out on trunk. | openPreviewPage fixture now clicks the menu item with a pattern that matches both labels. Same patch already used on #544, #546, #547 and #548. |
No change to the production fix in includes/assets.php. Only the two test files.
Verified local: full PHPUnit suite green with all asset files removed, same condition as CI. JS unit tests green. phpcs-changed clean. PHPStan clean.
Tests ✅ · Verification ✅ · CodeRabbit ✅ · PHP 7.4/8.4 ✅
|
Thanks for the contribution and the effort here! 🙏 A heads-up on how this repo works: a large part of SCF's codebase (field types, Flexible Content, Blocks v3, local fields, forms, value formatting, and most of includes/ and assets/src/js/) is regularly synced from an upstream codebase. Any change we make there, whether a bug fix or a new feature, gets overwritten or causes hard merge conflicts on the next sync. That includes features behind a beta flag. Fixes for those areas arrive through the sync itself, so we can't accept PRs that touch that code, even good ones. |
Every built SCF script lists
wp-polyfillas a dependency. ClassicPress never registers that handle, so WordPress silently drops the wholeacfchain and the footer inline code throwsacf is not defined. This was introduced wheninjectPolyfill: trueshipped in 6.5.1.This change registers a no-op
wp-polyfillfallback when the handle is missing, mirroring the existingreact-jsx-runtimepolyfill in the same class. On normal WordPress the handle already exists so this is a no-op and nothing changes. Block-editor bundles stay correctly disabled on ClassicPress.Closes #285
Use of AI Tools
Assisted by Muse Spark 1.2 (contributor-free) for investigation, implementation and tests. All changes were reviewed and tested by the author.