Skip to content

feat: load simulation panel UI in loadLazy (AEM Sidekick)#62

Merged
ramboz merged 4 commits into
v2from
claude/aem-experimentation-loader-ecd752
Jul 23, 2026
Merged

feat: load simulation panel UI in loadLazy (AEM Sidekick)#62
ramboz merged 4 commits into
v2from
claude/aem-experimentation-loader-ecd752

Conversation

@ramboz

@ramboz ramboz commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

In v2 the experiment/audience/campaign simulation UI was moved out of the plugin's own injected overlay and into a hosted MFE that the AEM Sidekick extension opens. But nothing on the page ever loaded it: loadLazy() in src/index.js was an empty placeholder, so the Sidekick "Experimentation" button dispatched its custom event to nobody — a silent no-op with no console error. This is the issue reported in SITES-48456.

This makes the plugin own that wiring in its lazy phase, so a consuming project no longer copies a listener script into tools/sidekick/ or adds anything to head.html.

Changes

  • loadLazy(document, options) now:
    • binds the Sidekick toolbar button (custom:aem-experimentation-sidekick) to the panel, falling back to the sidekick-ready event since the extension injects its element asynchronously;
    • lazy-loads the hosted MFE client.js on first use and toggles the panel thereafter;
    • opens the panel automatically on a ?experiment=<id>/<variant> simulation deep-link;
    • restores the panel after a variant-switch triggers a full-page reload.
  • It is gated to preview/dev only (setDebugMode) and never runs in production.
  • New simulationUI option'auto' (default) | 'sidekick' | 'universal-editor' | false — provides a clean way to distinguish the EDS + Sidekick surface from the Universal Editor, where the panel is delivered as a UE extension instead. 'auto' only activates on the Sidekick path, so a UE context naturally gets nothing.
  • setupCommunicationLayer is now idempotent, so the eager and lazy phases can both ensure the postMessage handshake exists (the panel now works even on preview pages with no experiment configured yet). Its reload handler also preserves the panel's open state across the reload.
  • Docs: README gains runExperimentationLazy in the loader, the loadLazy() wiring, the simulationUI option, and an "Enabling the simulation panel (AEM Sidekick)" section. That section keeps the still-necessary Sidekick plugin registration (config.json) and the helix5 persisted-config / PUT-vs-POST gotcha, and drops the obsolete copy-script / head.html steps. documentation/experiments.md's Simulation section is corrected (v1 overlay → v2 panel, audienceexperiment query param). Adds documentation/sidekick/config.json as a reference.

Relationship to #61

This supersedes the fragile half of #61: the ~167-line copy-paste aem-experimentation.js listener is no longer needed because the plugin owns it (versioned, no CSP-nonce trap, no silent no-op). The genuinely project-side parts of #61 — the Sidekick config.json registration and the helix5 admin-config gotcha — are preserved here in the README. Thanks to @keepthebyte for working out the reference implementation and documenting the gotchas first-hand.

Test plan

  • npm run lint:js clean
  • npx playwright test — 88 passed
  • Maintainer verification of the live Sidekick panel behavior against a real v2 project (MFE URL / handshake).

🤖 Generated with Claude Code

v2 moved the experiment simulation UI to a hosted MFE opened by the AEM
Sidekick extension, but nothing actually loaded it: loadLazy() was an empty
placeholder, so clicking the Sidekick button was a silent no-op.

loadLazy() now wires the Sidekick toolbar button to the panel, lazy-loads the
MFE on first use, honors the ?experiment=<id>/<variant> deep-link, and restores
the panel after a variant-switch reload. It is gated to preview/dev only and
never runs in production. setupCommunicationLayer is now idempotent so eager and
lazy phases can both ensure the postMessage handshake is available.

A new `simulationUI` option ('auto' | 'sidekick' | 'universal-editor' | false)
distinguishes the EDS + Sidekick surface from the Universal Editor, where the
panel is delivered as a UE extension instead.

This supersedes the manual copy-paste listener script approach; docs updated to
just register the Sidekick plugin and wire runExperimentationLazy() into the
project's loadLazy().

Refs SITES-48456

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Keep the simulation/preview code out of the core engine so it never ships or
parses on the eager/critical path in production. The panel controller, Sidekick
button wiring, deep-link/reload handling, and the postMessage handshake now live
in a dedicated src/simulation.js that loadLazy pulls in via dynamic import() only
in preview environments. index.js (the engine) shrinks by ~200 lines and no
longer carries any UI code; production pages never fetch simulation.js.

The handshake (setupCommunicationLayer) moves from the eager phase to the lazy
module alongside the panel it serves. It was already preview-gated, and the panel
that uses it is itself lazy/user-triggered, so this is behavior-preserving in
practice; the simulation panel continues to require the loadLazy wiring.

No public API change: loadLazy, runExperimentationLazy, and the simulationUI
option are unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ramboz

ramboz commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Updated (a8606da) to address a performance concern: the simulation/preview UI no longer lives in the core engine (index.js). It's been extracted into a dedicated src/simulation.js that loadLazy pulls in via a dynamic import(), only in preview environments.

Why this matters: previously the panel/handshake code (~200 lines) shipped inside the eager-loaded engine module and was parsed on every production experiment page, despite only ever running in preview. Now:

  • index.js (the engine) is ~200 lines leaner and carries no UI code.
  • simulation.js is a separate chunk fetched only when loadLazy reaches the import in preview — production never downloads or parses it.
  • Still versioned/owned by the plugin (no per-project copy-paste), so it doesn't reintroduce docs: document Sidekick Experimentation panel setup for v2 #61's drift problem.

The postMessage handshake (setupCommunicationLayer) moved into the lazy module too, so it's all one preview concern — behavior-preserving since it was already preview-gated and the panel that uses it is lazy/user-triggered.

No public API change (loadLazy, runExperimentationLazy, simulationUI unchanged). Lint clean; full suite green.

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.26950% with 25 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/simulation.js 81.19% 15 Missing and 7 partials ⚠️
src/index.js 87.50% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@ramboz
ramboz requested review from FentPams and sirgil2009 July 22, 2026 21:29
@Dereje24

Copy link
Copy Markdown
Collaborator

Multi-perspective review

Reviewed via the diff plus targeted raw-file fetches at head (a8606da) — confirmed DEFAULT_OPTIONS, setDebugMode, and the file tree; did not run build/lint/Playwright locally. Perspectives applied: Architecture, SRE/Reliability, Security, Product/Consumer.

Summary

Makes the plugin own the Sidekick simulation-panel wiring in loadLazy, replacing the fragile per-project copy-paste from #61, and (in the follow-up commit) extracts it into a lazily import()-ed src/simulation.js so no simulation code ships on the production eager path. Clean, well-documented, correctly preview/dev-gated. Close to mergeable; main gaps are one reliability edge and an under-documented consumer migration.

Strengths

  • The a8606da refactor is the right call — moving ~200 lines of panel/handshake code out of the eager engine into a dynamic chunk means production pages never fetch or parse simulation code. Concrete critical-path win.
  • Defense-in-depth gating — the loader's coarse isPreview check and loadLazy's authoritative setDebugMode re-check are identical logic, so there's no "loader allows but plugin silently rejects" trap.
  • Idempotent communication layer — the isCommunicationLayerInitialized flag cleanly prevents duplicate message listeners.
  • Thoughtful UX — the ?experiment=<id>/<variant> deep-link auto-open and the sessionStorage-based re-open across a variant-switch reload.
  • Docs are thorough — README preserves the genuinely project-side parts of docs: document Sidekick Experimentation panel setup for v2 #61 (Sidekick config.json, the helix5 PUT-creates / POST-updates gotcha) and documentation/experiments.md is correctly updated (v1 overlay → v2 panel, audienceexperiment).

Blockers

None. Preview/dev-only authoring tooling that never runs in production.

Should Fix

1. A failed MFE script load permanently disables the buttonsrc/simulation.js (createSimulationPanelController, load/toggle)
On script.onerror, loadPromise rejects but is left assigned, and open() swallows the rejection into a console.debug. Every subsequent toggle() then hits if (!loadPromise) → false, skips open(), calls togglePanel(false) on a container that was never injected (no-op), and returns the already-rejected promise. A single transient network blip on first click bricks the Experimentation button until a full page reload, with only a debug-level log. Reset the promise so the next interaction retries:

script.onerror = (e) => {
  loadPromise = null; // allow retry on next toggle
  reject(e);
};

Perspective: SRE / Reliability

2. Moving the handshake out of the eager phase is a consumer behavior change that isn't flagged as a migrationsrc/index.js (loadEager), README
Previously loadEager called setupCommunicationLayer whenever debug was enabled, so the hlx:experimentation-get-config and hlx:last-modified-request responders existed for any upgraded project on preview. Now they only exist if the project also wires runExperimentationLazy/loadLazy. A project that bumps the plugin but doesn't add the new lazy wiring silently loses that handshake on preview. The PR body's top section still asserts "the eager and lazy phases can both ensure the postMessage handshake exists" — true after commit 1, but stale after commit 2 (it's now lazy-only). Please reconcile the description and add an explicit upgrade/migration note (and a changelog/VERSION bump — VERSION is still '1.1.0').
Perspective: Product / Consumer & Architecture

Nice to Have

1. setupSimulationUI is not idempotentsrc/simulation.js
Unlike the comm layer, the UI wiring has no guard: each setupSimulation call creates a fresh controller and adds another SIMULATION_SIDEKICK_EVENT listener. If a consumer's loadLazy runs more than once (e.g. re-invoked on client-side navigation), the button fires toggle() twice per click and appears dead. A module-level flag or a dataset marker on the sidekick element would close this.

2. Bounded container wait resolves even on timeout with no feedbacksrc/simulation.js (waitForContainer)
After 20×200ms the promise resolves regardless, then togglePanel(true) silently no-ops because the container never appeared. Consider debug()-logging the timeout so a maintainer verifying against a real MFE URL can tell "MFE never injected its container" apart from "button not wired."

3. Pre-existing: config handshake replies to '*' with no origin allow-listsrc/simulation.js (setupCommunicationLayer, hlx:experimentation-get-config branch)
It clones window.hlx and postMessages it with target origin '*' and doesn't validate event.origin. Moved, not introduced, and preview-only — but a reasonable moment to note for a follow-up (constrain the reply origin to the known MFE origin). Not for this PR.

Recommendations

  • Consider giving the two "preview" definitions a single source of truth. The coarse isPreview gate is hand-copied into every consumer's experiment-loader.js and must stay in lockstep with setDebugMode. They match today; exporting a helper the loader can call would prevent future drift.
  • During the live Sidekick verification in the test plan, confirm both aem-sidekick and legacy helix-sidekick selectors and the sidekick-ready fallback all bind correctly.

Verdict

Ready to merge: with fixes — design is sound and production-safe, but the failed-load lockout (#1) and the undocumented eager→lazy handshake migration (#2) are behavior-affecting for consumers and worth addressing first. Neither is hard.

@FentPams

Copy link
Copy Markdown
Contributor

Solid fix — right shape: plugin owns the wiring, lazy-loaded, prod-gated, and simulationUI cleanly separates the Sidekick surface from UE. Verified locally: the panel contract matches the MFE's client.js (#aemExperimentation + .aemExperimentationHidden, which the MFE toggles itself in its close handler), lint:js is clean, and the 88 Playwright tests pass. The button binding is the same logic @keepthebyte validated live in #61, and the reference config.json is unchanged from his, so the "button shows up" half is on known-good footing.

One item worth folding in, plus a couple of small notes.

Handshake timing — the eager+lazy split the description promises would also close a real race. The PR text says setupCommunicationLayer is ensured in "the eager and lazy phases," but as shipped it's only reached from the lazy path (loadEager no longer calls it). For the Sidekick-button flow this is fine and actually race-free — setupSimulation registers the message listener synchronously before it binds the button, and the MFE only boots on click, so the listener always precedes any MFE postMessage.

The gap is the AEM Sidekick bookmarklet, which injects client.js independently of the plugin and can fire during page load, before the lazy phase has registered the listener. The MFE's config request is one-shot with a timeout→null (ExperimentEngineService.requestFreshConfig), so a bookmarklet opened too early gets a blank/stale panel. On v2 the listener lived in loadEager (debug), which shrank that window; moving it to lazy widens it — and this is the exact path used in the originating thread.

Minimal fix that gets both: keep the small setupCommunicationLayer in index.js and call it from loadEager under setDebugMode, with the idempotent guard making the lazy re-call a no-op. The heavy MFE client.js still loads only on click, so the page-performance win is preserved — only the tiny message handler comes back to eager. That makes the description accurate and closes the bookmarklet race, and it restores v2 behavior for existing projects that bump the subtree without adding runExperimentationLazy.

Minor:

  • sidekick-ready fallback uses { once: true }; if the event fires before loadLazy runs and the element isn't yet queryable, the button never binds. Low-probability, but it's the silent-no-op class this PR closes — a short re-query interval would harden it.
  • SIMULATION_MFE_URL is hard-pinned to prod, and the runExperimentationLazy preview check duplicates setDebugMode across the repo boundary. Both non-blocking — flagging only so they're on the record.

The one thing I can't check from here is the live Sidekick panel (button dispatch + client.js injection against the real extension) — same box you've left unchecked. Given the binding matches the field-validated #61 logic, I'd expect it to hold; a quick real-project smoke test closes it out. 👍

Dereje24 pushed a commit that referenced this pull request Jul 22, 2026
…ager

Address review findings on PR #62 (both the multi-perspective review and
@FentPams' approval notes):

- Keep the tiny, UI-less postMessage handshake (setupCommunicationLayer) in
  index.js and register it eagerly from loadEager under setDebugMode, so a
  Sidekick bookmarklet that injects the MFE early in page load doesn't race
  past the listener (one-shot config request -> blank/stale panel). The
  idempotent guard makes loadLazy's re-call a no-op. Only the handler returns
  to eager; the heavy MFE client.js still loads lazily on click, so production
  pages download/parse no simulation code. This also restores v2 behavior for
  projects that bump the plugin without adding runExperimentationLazy.
- Reset loadPromise on script.onerror so a transient MFE load failure retries
  on the next toggle instead of permanently disabling the button.
- Make setupSimulationUI idempotent so a re-invoked loadLazy doesn't stack
  duplicate button listeners; bind each sidekick element once via a dataset
  marker and add a bounded re-query so a missed sidekick-ready isn't silent.
- Debug-log the waitForContainer timeout so a missing MFE container is
  distinguishable from an unwired button.
- Bump VERSION/package.json to 1.2.0; README documents the eager-handshake /
  lazy-UI split (no consumer migration needed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ager

Address review findings on PR #62 (multi-perspective review + @FentPams' approval notes):

- Keep the tiny, UI-less postMessage handshake (setupCommunicationLayer) in
  index.js and register it eagerly from loadEager under setDebugMode, so a
  Sidekick bookmarklet that injects the MFE early in page load doesn't race
  past the listener (one-shot config request -> blank/stale panel). The
  idempotent guard makes loadLazy's re-call a no-op. Only the handler is eager;
  the heavy MFE client.js still loads lazily on click, so production pages
  download/parse no simulation code. Restores v2 behavior for projects that
  bump the plugin without adding runExperimentationLazy.
- Reset loadPromise on script.onerror so a transient MFE load failure retries
  on the next toggle instead of permanently disabling the button.
- Make setupSimulationUI idempotent; bind each sidekick element once via a
  dataset marker and add a bounded re-query so a missed sidekick-ready event
  isn't silent.
- Debug-log the waitForContainer timeout so a missing MFE container is
  distinguishable from an unwired button.
- Bump the exported VERSION const to 1.2.0; README documents the
  eager-handshake / lazy-UI split (no consumer migration needed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Dereje24
Dereje24 force-pushed the claude/aem-experimentation-loader-ecd752 branch from f65603e to 133c22b Compare July 22, 2026 22:59
@FentPams

Copy link
Copy Markdown
Contributor

Re-reviewed at 133c22b — this addresses the handshake point cleanly: setupCommunicationLayer is now registered eagerly in loadEager under setDebugMode, with the idempotent guard making the lazy re-call a no-op, so a Sidekick bookmarklet no longer races a lazy-only listener while the heavy MFE client.js stays deferred to first click. The sidekick-ready hardening (once-guard + data-aem-experimentation-bound marker + bounded poll) and the onerror loadPromise reset also close the two silent-no-op edges. Re-ran locally: lint:js clean, 88 Playwright tests pass.

Only remaining nit, non-blocking: the new handshake/simulation code is uncovered (codecov ~16% patch). A couple of unit tests around setupCommunicationLayer's message handling and the panel toggle / onerror-retry would lock in the hardening — fine as a follow-up.

One process note, not code: the live Sidekick panel (real extension dispatching the button event + client.js injection end-to-end) still isn't covered by the suite — the box left unchecked above. The binding matches the field-validated #61 logic, so I'd expect it to hold, but a quick real-project click-through would fully close it out. 👍

Add tests/simulation.test.js exercising src/simulation.js, which Codecov
flagged as low-coverage after the extraction. Covers the Sidekick button
toggle, the sidekick-ready fallback, the ?experiment deep-link, the
get-config and last-modified postMessage handshakes, the reload +
panel-reopen flow, and the production / simulationUI=false / universal-editor
gates.

The hosted MFE script is stubbed via page.route (injecting the
#aemExperimentation container), and loadLazy is exposed on window through a
string-content module script so the runner's Babel transform doesn't rewrite
the dynamic import. Brings simulation.js to ~88% statement coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ramboz

ramboz commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Added tests/simulation.test.js (rebased on top of @dilnesaw's hardening commit) to address the Codecov gap on the extracted simulation.js. 11 tests covering:

  • Sidekick button toggle (open → hide), and the sidekick-ready late-injection fallback
  • the ?experiment=<id>/<variant> deep-link auto-open
  • the get-config and last-modified postMessage handshakes
  • the reload → panel-reopen (sessionStorage) flow
  • the production / simulationUI: false / universal-editor gates
  • the two hardening branches from the latest commit: the "bind once across repeated loadLazy calls" guard, and the "retry after a transient MFE load failure" path

The hosted MFE is stubbed via page.route (injecting the #aemExperimentation container); loadLazy is exposed through a string-content module script so the runner's Babel transform doesn't rewrite the dynamic import().

Coverage now: simulation.js ~88% statements / 100% functions, index.js ~94%. The remaining uncovered lines are defensive edge branches (container-timeout, cached-promise short-circuit). Full suite: 99 passed, lint clean.

@ramboz
ramboz merged commit 44828b6 into v2 Jul 23, 2026
3 checks passed
@ramboz
ramboz deleted the claude/aem-experimentation-loader-ecd752 branch July 23, 2026 15:53
github-actions Bot pushed a commit that referenced this pull request Jul 23, 2026
# [1.2.0](v1.1.0...v1.2.0) (2026-07-23)

### Features

* load simulation panel UI in loadLazy (AEM Sidekick) ([#62](#62)) ([44828b6](44828b6)), closes [#61](#61) [#61](#61) [#61](#61)
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.

3 participants