Script loading: read the concat blob's handle list, so a lazily-loaded package stops replacing wp.hooks - #717
Merged
Conversation
…d package stops replacing wp.hooks
The lazy script loader answered "is this already here?" with one
signal: a `<script src>` in the document with the same origin and
path. That signal is blind to Core's script concatenation, which is
ON by default in wp-admin — only `SCRIPT_DEBUG` or an explicit
`CONCATENATE_SCRIPTS = false` turns it off, which is why no developer
environment ever showed this.
Under it, every script below `wp-includes/js/` and `wp-admin/js/` is
served from one `load-scripts.php` response and has no tag of its
own. So the sniff said "absent" for `wp-hooks`, the loader appended
it, and re-executing it assigned a fresh registry to
`window.wp.hooks`: every subscriber the shell installed at boot went
deaf while the actions kept firing on the new one, and every window
sat under its loading overlay.
Two triggers, both reachable on a stock production admin: mounting
any first-party widget whose script declares a `wp-*` package (six
do), and pressing Cmd-K, whose deferred palette manifest replays the
same Core handles through the same blind test.
A concat blob is not opaque, though. It names the handles it carries
in its own query string, because that is how `load-scripts.php` knows
what to serve. `src/script-presence.ts` names the question and
answers it from both signals — `findScriptByPath` moves there
unchanged, and `isScriptInDocument( { url, handle } )` adds the
handle test. Chunks are joined before the comma split, the way Core
reassembles them, since `_print_scripts()` cuts the list every 128
characters and a cut lands mid-name as often as not.
`ScriptExtras` gains `handle`, threaded where a Core package can
travel: `scriptDeps` entries (already stamped server-side — the
docblock now says why that field is load-bearing rather than
informational) and the command-palette manifest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…is off Review pass on #717. The three tests the fix shipped with pin the server (every dependency names its handle), the loader (a handle in the blob is skipped) and the palette replay (the handle reaches the presence test). None of them pins the path #715 actually reports: src/widgets/server-sync.ts forwarding a widget's scriptDeps into the loader. A mapping there that dropped `handle` would pass every one of them and still re-inject wp-hooks. widgets-lazy.test.ts now runs that path with the real loader and the real presence test against a concat blob naming wp-hooks; stripping `handle` in the sync fails it with hooks.min.js appended. Three places said concatenation is off "only under SCRIPT_DEBUG". script_concat_settings() also honours CONCATENATE_SCRIPTS = false, as the module header already says; the two docs and the PHPUnit docblock now say the same. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #715.
What was happening
The lazy script loader answered one question before appending a tag — is this already here? — and it had one way of answering: a
<script src>in the document with the same origin and path.That signal is blind to Core's script concatenation, and concatenation is on by default in wp-admin.
script_concat_settings()only turns it off underSCRIPT_DEBUGor an explicitCONCATENATE_SCRIPTS = false, and it does not depend oncan_compress_scripts(that one only gates gzip). Under it, every script belowwp-includes/js/andwp-admin/js/is served from a singleload-scripts.phpresponse and has no tag of its own.So the sniff said "absent" for
wp-hooks, the loader appended it, and re-executing it assigned a fresh registry towindow.wp.hooks.src/hooks.tsresolveswindow.wp?.hookson every call whilesrc/api/facade.tssnapshots it once at boot, so from that moment the two diverged: everything the shell subscribed at boot sat on the old registry, everydoActionwent to the new one.WINDOW_CONTENT_LOADEDfired anddesktop-mode/window-loading-exitnever heard it, and every window sat under its loading overlay — #707's symptom from a new cause, with #711's fix working correctly underneath.Note the asymmetry that made
wp-hooksthe one to break: Core attaches anafterinline towp-i18n(script-loader.php,wp_default_packages_inline_scripts()), which breaks concat and prints i18n standalone wherefindScriptByPathfinds it.wp-hookscarries no inline data, so it is swallowed by the blob.Two triggers, both on a stock production admin:
wp-*package. Six first-party ones do —post-stats,drafts,heartbeat(which declareswp-hooksdirectly),recent-comments,site-views,starter— andopenstation_resolve_script_dependencies()putswp-hooksin each closure. This is what the issue reports.src/commands/palette-assets.tsreplays the deferred palette manifest — which listswp-hooks— through the same blind test. The file's own comment already worried about this class of thing ("re-executingwp-datawould wipe every registered store"), which is the same bug with a worse blast radius on any site where a plugin printedwp-dataat boot.Why no test environment showed it:
SCRIPT_DEBUGistruein the Core-checkout dev host and in wp-env's default config. Concatenation is off in every environment we develop against.The fix
The bug is not "the dedupe missed a file". It is that a question with several possible answers had only ever been asked one way. So the change names the question and gives it a home.
src/script-presence.ts— has this document already run this script?Two signals, because one is not enough:
<script src>.findScriptByPathmoves here verbatim — it is the same question, and it was living inside the loader.load-scripts.phpblob. The missing one. A blob is not opaque: it names the handles it carries in its own query string, because that is howload-scripts.phpknows what to serve.Reassembled the way Core reassembles it — join every
load[…]value first, then split on commas — because_print_scripts()cuts the handle list every 128 characters with no regard for name boundaries, so splitting a chunk on its own yieldswp-hoandoks. Chunks are ordered numerically by theirchunk_Nindex; Core sorts those keys as strings, which agrees for the first ten and is Core's own quirk past them, and we want whole names either way.This is document truth, not a server snapshot: no new global, no new payload key, no late-footer hook whose timing has to be right, nothing that can drift out of sync with what actually printed, and it stays live for a blob printed after boot.
ScriptExtrasgainshandle?, threaded exactly where a Core package can travel —scriptDepsentries and the command-palette manifest. Both already carried the handle across the wire; it was typed as informational. It is now load-bearing, and the docblocks on both sides say so.Tests
The two loader cases were checked against the old logic rather than assumed — reverting the guard reproduces the report exactly:
tests/vitest/script-presence.test.ts— the chunk join, numeric ordering pastchunk_9(asserted on the names, since the comma count survives a mis-ordered join and would not show it),load-styles.phpignored, empty on aSCRIPT_DEBUGpage, origin still part of the identity, and a blob printed after the first query still seen.tests/vitest/vendor-loader-deps.test.ts— a concatenated dependency skipped while a genuinely absent sibling still loads; and the same for a concatenated handle passed as the bundle itself (its own handle, so the URL memo from the first case cannot be what makes it pass).tests/vitest/widgets-lazy.test.ts— the path the issue actually reports, end to end: the real loader and the real presence test, a concat blob namingwp-hooks, and a widget whosescriptDepslist it. The three tests above pin the server, the loader and the palette replay, but none of them pinssrc/widgets/server-sync.tsforwardingscriptDepsintact — a mapping there that droppedhandlewould pass all of them and still re-injectwp-hooks. Checked by mutation: strippinghandlein the sync fails this with+ "/wp-includes/js/dist/hooks.min.js".tests/vitest/command-palette-assets.test.ts— the replay hands the handle to the presence test, not just the URL.tests/phpunit/tests/resolveScriptDependencies.php— every resolved dependency names its handle, with a failure message saying what breaks if it stops. Dropping that field would not fail loudly; it would just silently re-injectwp-hooksagain.Docs
docs/architecture.md— the lazy-load section claimed "the script tag dedupes by URL", which was the inaccurate sentence. Now says why URL alone cannot work for Core packages.docs/migration-wp-package-globals.md— where the dependency replay is documented for plugin authors, so this is where "a package the page already has is never replayed" belongs, and why that is answered by handle.docs/javascript-reference.md— theloadVendorScriptrow gainsextras?and the note to passhandlewhen you know it.script_concat_settings()turns it off underSCRIPT_DEBUGor an explicitCONCATENATE_SCRIPTS = false, notSCRIPT_DEBUGalone — the module header already had this right.Two adjacent things deliberately left alone
openstation_resolve_script_payload()never appliesscript_loader_src. Under a CDN/offload plugin the URL shipped is the un-rewritten one, so the fetch itself can be wrong, not just the sniff. Different bug, different fix (mirror Core's base_url → ver → filter →esc_urlsequence); folding it in here would muddy both. The handle test happens to cover the sniff half.scriptDepsat all. Native windows, commands, settings tabs, wallpapers, games and file openers declare packages that are never replayed — already documented as a known gap in the migration note. Extending it is a payload change per builder, not a new mechanism.Checks
npm run typecheck·npm run lint·npm run test:js(5315 passed, 425 files) ·npm run lint:php·npm run test:php(2647 passed) ·npm run build— all clean.Credit to @juanlentino for a diagnosis that was correct end to end, including the
wp.os.hooks !== window.wp.hooksmeasurement that made it fast to confirm.🤖 Generated with Claude Code