Skip to content

Lazy scripts: replay a bundle's declared dependencies on live activation - #825

Merged
epeicher merged 3 commits into
trunkfrom
fix/native-window-script-deps
Sep 16, 2026
Merged

epeicher merged 3 commits into
trunkfrom
fix/native-window-script-deps

Conversation

@epeicher

@epeicher epeicher commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

What it does

A plugin activated from inside the shell now opens its windows configured, without an F5. Reported with AllTerrain Forms: the tile appeared live, but the builder opened with "AllTerrain Forms is not configured on this page" until the page was reloaded.

CleanShot.2026-09-15.at.17.11.15.mp4

Rationale

AllTerrain Forms ships its config on a src-less alias handle (wp_register_script( 'allterrain-forms-config', false ) plus wp_add_inline_script() setting window.allTerrainForms), declared as a dependency of every bundle so it always runs first. On a normal boot WordPress prints that dependency. On a live activation the shell fetches the bundle lazily, and two things were missing:

  • No lazy path except widgets resolved a handle's dependency closure at all.
  • openstation_resolve_script_payload() returned an empty payload for any handle without a src, so the alias could not have been delivered even by a path that walked dependencies.

A second layer showed up while verifying: the plugin registers the same bundle as a command script too, and loadVendorScript memoizes by URL, so whichever path fetches a bundle first decides what ran before it. Fixing native windows alone left the command sync winning that race with no deps. That is why every lazy payload builder ships the closure now, not just native windows.

Implementation

  • openstation_resolve_script_payload() keeps an alias's before / after / l10n with an empty url, mirroring what WP_Scripts::do_item() prints for it (translations excluded, as Core only prints those for a handle it printed a tag for). The command-palette manifest's local copy of that harvest is gone.
  • Native windows: each loadable handle's entry in nativeWindowScriptData carries deps, an ordered handle list whose members land in the same map, so a shared package is serialized once. hydrateServerEntries() joins them into scriptDeps on the entry, its companions and its tabs; loadOnce() hands them to the loader.
  • Commands, settings tabs, dock-rail renderers, title-bar buttons, window actions, unfocus effects, window links, window chrome (themes/controls/slots/chromes), wallpapers, games and desktop-file openers ship scriptDeps via openstation_resolve_script_dependencies(), and each server-sync passes deps through. Widgets already did; the type now lives once as LazyScriptDependency.
  • loadVendorScript: a dependency with an empty url is replayed inline in print order, once per document (replayedAliases), and skipped when Core already printed it. isScriptInDocument() gains that third signal: a tag under Core's printed ids (<handle>-js, <handle>-js-before and siblings), the only trace an alias leaves.

Docs: docs/migration-wp-package-globals.md (the "no other lazy path does this" paragraph is now the opposite), the script note in docs/hooks-reference.md, docs/architecture.md, docs/javascript-reference.md.

Not fixed here: AllTerrain Photo Editor still opens blank after a live activation. Its lienzo bundle is only enqueued on openstation_mode_init at boot and never appears in any payload, so the shell cannot know about it. That is plugin-side: declare it through the openstation_app_window_args filter as a companion scripts handle, and attach window.lienzoConfig at registration rather than at enqueue time. With this PR merged, both then ride the live payload. Follow-up on the plugin: AllTerrainDeveloper/allterrain-photo-editor#10.

Cost of resolving the closure per request

Every sub-builder now calls openstation_resolve_script_dependencies() for each handle it names, with no memo across builders (a handle named by two builders is resolved twice). Measured on a member site as admin, after admin_enqueue_scripts, with 271 registered handles and 827 dependency edges:

Measurement Result
One openstation_build_menu_payload() call, cold / warm 0.70 ms / 0.38 ms
Resolving one handle with a 5-package closure 0.04 ms
Resolving every handle the payload names (16 handles, 31 dependency payloads) 0.2 ms per request

The repeated work is about a fifth of a millisecond on a request that costs hundreds, so it stays as is. A per-request memo is not free either: inline data can be attached to a handle between resolutions (the refresh probe replays admin_enqueue_scripts, and modules attach config at priority 5 while the payload builds at 10), so a memo that outlives that window would ship stale data, which test_probe_payload_carries_data_attached_on_admin_enqueue_scripts exists to catch.

Testing instructions

On an install with AllTerrain Forms, deactivate it, reload the shell, activate it from the Plugins window, then double-click the Forms desktop icon: the builder opens configured. Before this change the same steps reproduce the report.

npm run test:js -- vendor-loader script-presence native-windows-lazy-script commands-server-sync
npm run test:php -- --filter='Tests_OpenStation_ResolveScriptDependencies|Tests_OpenStation_NativeWindowLazyScript|Tests_OpenStation_Commands'

New tests pin: an alias dependency keeps its inline data and an empty one is dropped; a window ships its closure and a dependency later named as a script resolves its own; the command payload ships scriptDeps; alias replay order, once-per-document, and skip-when-printed; id-based presence. Full test:js, test:php, lint, lint:php and typecheck are green.

Open WordPress Playground Preview

A bundle the shell fetches lazily never went through WordPress's own
dependency resolution, and the payload resolver dropped any handle
without a src. A plugin whose config rides a src-less alias handle
declared as its bundle's dependency (AllTerrain Forms) therefore opened,
after a live activation, with the config undefined.

Every lazy payload builder now ships the handle's dependency closure;
native windows carry it as `deps` inside the handle-keyed script-data
map. The loader replays an alias's inline data in print order, once per
document, and recognises Core's printed tag ids so a dependency the page
already has is never run twice.
@epeicher
epeicher requested review from AllTerrainDeveloper and a balanced review from Copilot September 15, 2026 15:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@epeicher

Copy link
Copy Markdown
Collaborator Author

@claude please review

@claude

claude Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Claude finished @epeicher's task in 3m 10s —— View job


Review of #825 — Lazy scripts: replay a bundle's declared dependencies on live activation

  • Gather context (read diff, changed files)
  • Understand implementation: openstation_resolve_script_dependencies(), native windows deps, loadVendorScript alias replay
  • Review PHP changes (payload.php, commands.php, various registries)
  • Review JS changes (native-windows.ts, script-presence.ts, vendor-loader.ts, types.ts)
  • Review tests
  • Post final review feedback

Note on execution: my sandbox in this environment declined approval for any non-git/read-only shell command (npm run test:js, npm run lint, even npx vitest --version), so this is a static read-through rather than a verified test run. If you want me to actually execute the suites named in the PR description, --allowedTools needs to permit npm run test:js/npm run test:php/npm run lint* in this context.

Summary

This is a clean, well-scoped fix. The core insight — that loadVendorScript memoizes by URL so "whichever lazy path fetches a bundle first decides what ran before it," which is why every payload builder needs to ship the dependency closure rather than just native windows — is correct and the implementation follows it through consistently across all ~15 registries (commands, settings tabs, dock-rail renderers, title-bar buttons, window actions, unfocus effects, window links, window chrome themes/controls/slots/chromes, wallpapers, games, desktop-file openers).

What I checked and confirms is correct

  • openstation_resolve_script_payload() (includes/core/payload.php:1955): the alias case (src === '') now correctly returns url => '' while preserving before/after/l10n, and skips translations for an alias — mirrors Core's do_item()/print_translations() behavior since Core never prints translations for a handle it didn't print a <script> tag for.
  • openstation_resolve_script_dependencies(): reuses the existing cycle-safe openstation_script_dependency_closure() (the $seen guard), so no risk of infinite recursion on a dependency cycle. Correctly drops only aliases with no inline data at all.
  • openstation_collect_native_windows_payload()'s $collect_handle closure: the $resolved_as_bundle vs. $script_data split is subtle but correct — a handle reached only as someone else's dependency gets cached with deps => [] (safe, since the closure returned by openstation_resolve_script_dependencies() is already the fully-flattened, topologically-ordered closure, so a dependency-of-a-dependency doesn't need its own nested deps), while a handle later resolved as its own bundle target recomputes and overwrites with its real closure. tests/phpunit/tests/nativeWindowLazyScript.php's test_a_dependency_named_as_a_script_resolves_its_own_closure pins exactly this and it checks out by hand-tracing the code.
  • src/wallpapers/vendor-loader.ts: replayAlias() replays in Core's print order (l10n → before → after, synchronously via injectInline), dedupes via replayedAliases keyed by handle, and isScriptInDocument()'s new printedScriptHandleInDocument() check (-js, -js-before/-after/-extra/-translations suffixes) correctly gives an alias a presence signal it otherwise has none of (no file, no concat-blob entry).
  • src/native-windows.ts's hydrateServerEntries() / depsOf(): correctly resolves a handle's deps (handle list) back into full dependency objects from the shared scriptData map, skips members missing from the map (older server / dropped dep), and the entry.scriptDeps ?? depsOf(...) fallback preserves backward compatibility with the "old inline-entry" cross-version bridge format that's tested separately.
  • types.ts: LazyScriptDependency is a clean extraction of what used to be an inline anonymous type on DesktopWidgetServerEntry, now reused everywhere — no shape drift across the ~15 server-entry interfaces that gained scriptDeps?.
  • Docs (docs/migration-wp-package-globals.md, hooks-reference.md, architecture.md, javascript-reference.md) are updated in the same PR and accurately describe the new behavior, including explicitly retracting the old "no other lazy path does this yet" statement — good adherence to the repo's doc-must-match-code rule.

Minor observations (non-blocking)

  1. Repeated work per request, not new to this PR. openstation_build_menu_payload() runs on every admin page load, and now every one of its ~15 sub-builders calls openstation_resolve_script_dependencies($handle) per handle, each doing its own graph walk with no cross-builder memoization within the request. For a typical handle count this is negligible, and it mirrors the pre-existing widget behavior this PR generalizes, so I wouldn't block on it — just flagging in case a site with unusually many registered handles ever shows up in profiling.
  2. replayedAliases keyed by handle only, never reset. Documented intentionally ("replayed once per document"), consistent with how concatenatedScriptHandles()/module-level caches already behave elsewhere in this loader, so this is a deliberate and reasonable choice, not an oversight.

Testing

I was not able to execute npm run test:js / npm run test:php / npm run lint / npm run typecheck in this environment (all non-git shell commands required approval that wasn't available). Based on the diff alone, the new/updated tests (resolveScriptDependencies.php, nativeWindowLazyScript.php, commands.php, vendor-loader.test.ts, script-presence.test.ts, native-windows-lazy-script.test.ts, commands-server-sync.test.ts) cover the scenarios called out in the PR description (alias-keeps-inline-data, empty-alias-dropped, shared-dependency-serialized-once, a-dependency-later-named-as-its-own-script, replay-order/once-per-document/skip-when-Core-printed, id-based presence) — I didn't find a gap in coverage or a case the implementation mishandles.

No bugs found in this review. This looks ready pending the actual test/lint run.
· Branch: fix/native-window-script-deps

@epeicher

Copy link
Copy Markdown
Collaborator Author

On the "repeated work per request" observation: measured rather than estimated, and it is negligible. On a member site as admin, after admin_enqueue_scripts, with 271 registered handles and 827 dependency edges:

Measurement Result
One openstation_build_menu_payload() call, cold / warm 0.70 ms / 0.38 ms
Resolving one handle with a 5-package closure 0.04 ms
Resolving every handle the payload names (16 handles, 31 dependency payloads) 0.2 ms per request

About a fifth of a millisecond per request, so no memo. A per-request cache would also need scoping to a single build: inline data can be attached to a handle between resolutions (the refresh probe replays admin_enqueue_scripts; modules attach config at priority 5, the payload builds at 10), and a memo that outlives that window ships stale data. Numbers are recorded in the PR description as well.

@epeicher
epeicher enabled auto-merge (squash) September 16, 2026 14:36
@epeicher
epeicher merged commit 252031a into trunk Sep 16, 2026
5 checks passed
@epeicher
epeicher deleted the fix/native-window-script-deps branch September 16, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants