Stork: drop inline IIFE for an ES module under _emanote-static/js/ by srid · Pull Request #672 · srid/emanote

srid · 2026-04-26T19:18:55Z

The Stork search controller was the last interactive behavior still living as an inline <script> IIFE in a Heist template. This is the follow-up named in #663's body — Stork was deliberately deferred there because its onclick="window.emanote.stork.toggleSearch()" call sites span four templates and the migration warranted a focused change. Now it takes the same shape as the other behaviors landed in #663: a module under _emanote-static/js/, picked up by the importmap, no window.emanote.stork global, no marker <script id="emanote-stork"> element.

Templates expose the search trigger via data-emanote-stork-toggle; a single delegated click listener on document covers the four sites (sidebar + breadcrumbs + minimal-layout header + modal backdrop) — the backdrop's role is "close" rather than "open", which works because the backdrop is only visible while the modal is open. The previously-separate inline theme-mirror script in stork-search.tpl (a MutationObserver re-skinning the search dialog when the user flips the dark-mode toggle) folded into the module too, since document.documentElement is never replaced by morph and the wrapper element re-resolves on each fire. Base URL comes from document.baseURI instead of a custom data attribute — same value, no marker element to maintain.

A morph-time regression surfaced during review and is fixed in-PR: the dialog rendered unstyled after a route switch because the new #stork-wrapper element gets no stork-wrapper-edible{,-dark} class — the MutationObserver only fires when <html>.class actually changes, and a plain morph nav doesn't toggle the theme. The fix folds applyStorkTheme() into the existing onMorph callback that already marks the index stale, so both post-morph concerns hang off the same hook. A new @morph-tagged scenario navigates-then-searches and asserts the wrapper carries one of the edible classes; it failed on the original code, passes now. Issue #673 tracks the broader auto-morph e2e mode that would surface this entire class of regression automatically across every behavior.

Stays inline by necessity. The vendor <script src=".../stork/stork.js"> (the WASM loader that defines window.stork) keeps loading from <head> as a non-module blocking script — it must parse before our module evaluates so stork.initialize() and stork.register() resolve. A module-load guard fails fast with a diagnostic if the load order ever breaks.

WASM init is once-per-session. Same memory-leak trade-off as before (#411): onMorph(markIndexAsStale) flips a flag on each in-app navigation, and refreshIndex re-registers lazily the next time the user opens search.

E2e: four scenarios cover Ctrl+K opens, Esc closes, the sidebar search button opens, and the morph-then-search styling assertion. The Then("the Stork search modal is {string}", …) assertion checks the body's stork-overflow-hidden-important class (the actual open-state truth, not computed visibility — the container is position:fixed with a backdrop, so getComputedStyle reads display: block either way; only the class flips).

Eleven commits: the primary migration plus four hickey/lowy fixes (searchShown mirror flag → DOM-derived getter; vendor window.stork guard; comment correctness in base.tpl; morph-survival docstring allows combining strategies), three police fixes (silent URL-parse warn in getBaseUrl, MODAL_HIDDEN_CLASS constant, registerIndex(forceOverwrite) over options bag), an e2e step fixup, the morph-regression test + fix, and a merge from master. With this in, the inline-IIFE inventory named in #643 is fully closed.

Try it locally

nix run github:srid/emanote/stork-es-module

Generated by /do on Claude Code (model claude-opus-4-7).

The Stork controller was the last interactive behavior still living as an inline IIFE in a Heist template — it was named in #663's PR description as 'kept inline because its onclick="window.emanote.stork.toggleSearch()" call sites span four templates'. This is that follow-up: stork.js takes the same shape as theme-toggle, code-copy, toc-spy, and footnote-popup, and the four onclick attrs flip to a data-emanote-stork-toggle attribute consumed via event delegation. Notable bits: - Vendor stork.js (the WASM loader that defines window.stork) stays loaded as a non-module <script src> from the head — blocking, so it parses before our module evaluates. - WASM init runs once per session (top-level module evaluation, on window.load if document isn't complete yet). Same memory-leak trade-off documented in #411 — index re-registration on each morph would leak, so onMorph(markIndexAsStale) defers the refresh until the user actually opens search. - The dark-mode mirror (.dark on <html> → .stork-wrapper-edible-dark on #stork-wrapper) was a separate inline <script> in stork-search.tpl; it folded cleanly into stork.js as a single MutationObserver on documentElement that re-resolves the wrapper on each fire (so it survives morph without re-attaching). - Base URL comes from document.baseURI (the <base href=…> in head) instead of a marker <script id="emanote-stork" data-…> attribute — same value, no need for a marker element. E2e: three new scenarios cover Ctrl+K open, Esc close, and click- to-open via the search-icon button. The 'I press' step uses Playwright's keyboard-combo syntax for both the modifier shortcut and the bare key.

The searchShown module flag duplicated the body's .stork-overflow-hidden-important class — the open-state truth was already in the DOM. The keydown handler read the flag to decide whether Esc should close the modal; if anything outside this module ever mutated the body class (a future utility, devtools, an adjacent script), the flag would drift and Esc would silently no-op while the modal looked open. Replace with isSearchShown() that queries the body class. One extra classList.contains call per Esc keypress; negligible.

Without the guard, a future template tweak that reorders or removes the vendor <script src=".../stork/stork.js"> in stork-search-head.tpl would surface as a cryptic 'stork is not defined' on the user's first search click, far from the load-order cause. Throw at module evaluation with a pointer to the failing template instead.

The comment named the still-inline script as the 'Stork controller', but Stork's controller is now an ES module under _emanote-static/js/stork.js — the inline script is just the vendor WASM loader (window.stork). Reword to name what's actually inline and why each piece is.

The original 'a behavior must choose ONE strategy' phrasing was true when only toc-spy used onMorph (exactly one strategy). Stork now combines event delegation (clicks), onMorph (index staleness), and a documentElement MutationObserver (theme mirror) in one module — the prior wording would mislead a future behavior author into thinking combined use is prohibited. Reword to allow it explicitly with the stork example as the concrete shape.

The bare-fallback shape (try { … } catch { return '/' }) hid the case where document.baseURI was unparseable. Reaching that path means the index + WASM fetches go to '/' which is almost certainly wrong, with no console signal. Add a warn that names the cause.

The 'stork-overflow-hidden-important' literal sat at three sites (toggleSearch, clearSearch, isSearchShown) — past the rule-of-three threshold and crossing the JS↔CSS boundary (the class is defined in stork-search-head.tpl's <style> block). Lift to a module constant.

…ns bag The options bag had only one consumer of one field — refreshIndex passed { forceOverwrite: true }, the two startup callers passed nothing. An explicit boolean parameter is clearer at both call sites (registerIndex() and registerIndex(true)) and removes the parameter- sprawl shape that invites future ad-hoc fields.

Two e2e mismatches: - The 'Stork modal is hidden' / 'is visible' assertions used unquoted state words but the step pattern is {string}. Cucumber routed them as undefined. Quote the gherkin literals so they match. - The 'click Stork search trigger' step picked the first matching button via .first(), which at the 1280x720 test viewport is the breadcrumbs button (md:hidden, mobile-only). Filter to :visible so the sidebar button (the actual md+ trigger) is selected. E2e now 21/21 in live, 20+1-skipped in static.

srid · 2026-04-26T19:19:28Z

Hickey/Lowy Analysis

#	Lens	Finding	Disposition
1	Hickey	`searchShown` mirror flag could drift from body class (silent Esc)	Fixed in this PR
2	Hickey	`window.stork` not guarded → cryptic error far from load-order cause	Fixed in this PR
3	Lowy	`base.tpl` comment misnamed inline as "Stork controller"	Fixed in this PR
4	Lowy	`morph.js` docstring said "must choose ONE strategy" — Stork combines	Fixed in this PR
–	Both	9 No-op findings (intentional design, structurally sound)	No-op

Hickey rationale

(1) searchShown shadowed the stork-overflow-hidden-important body class. If anything outside the module ever mutated that class (a future utility, devtools), the keydown handler would silently no-op on Esc when the modal looked open. Replaced with isSearchShown() that queries the body class — one extra classList.contains per Esc keypress, eliminates the drift hazard.

(2) window.stork is a vendor global the module assumes is defined at evaluate time; the load-order constraint was documented in a comment but not enforced. Without the guard, a future template tweak that reorders or removes the vendor <script src> in stork-search-head.tpl would surface as stork is not defined on the user's first click — far from the actual cause. Added a typeof stork === 'undefined' → throw at module load with a pointer to the failing template.

Lowy rationale

(3) base.tpl's <emanoteJsBundle /> comment described the still-inline Stork script as the "Stork controller", but Stork's controller is now an ES module — the inline is just the vendor WASM loader. Renamed to "vendor Stork WASM loader" so a future reader doesn't think the controller is still inline.

(4) The morph.js morph-survival docstring (added in #663 review) said "a behavior must choose ONE strategy". Stork now combines strategies 2 (event delegation), 3 (MutationObserver), and 4 (onMorph) — distinct concerns, distinct dependency shapes, all in one module. Reworded to allow combination explicitly with stork as the concrete example, so the next behavior author isn't misled.

No-op findings (kept verbatim from sub-agent review)

data-emanote-stork-toggle dual role (open + close) is fine — backdrop is only visible while modal is open, so toggleSearch() derives action from current state.
MutationObserver filter scope is pragmatic — narrowing further to ".dark substring change" would add regex/parse overhead per mutation.
getBaseUrl() via document.baseURI is a justified simplification, not a leaky abstraction (<base href> is unconditional in base.tpl).
registerIndex(forceOverwrite=…) lazy-on-search trade-off (vs. eager-on-morph) is honest given typical browsing patterns.
Stork stays cleanly separate from the four Move interactive JS to ES modules with live-server cache busting #663 behaviors (single-direction import { onMorph } from '@emanote/morph').
e2e step shapes match patterns elsewhere in the suite.

`/code-police` follow-ups (3 commits)

Fact-check: getBaseUrl URL-parse catch was silent → now logs console.warn so a malformed document.baseURI surfaces instead of search degrading to root with no signal.
Elegance: 'stork-overflow-hidden-important' literal hit the rule-of-three across toggleSearch / clearSearch / isSearchShown → extracted to MODAL_HIDDEN_CLASS.
Elegance: registerIndex(options) had a one-field options bag; only one of two callers passed anything → replaced with explicit forceOverwrite = false boolean.

E2e: 21 scenarios; live=21/21, static=20+1-skipped (the @morph toc-spy scenario).

srid · 2026-04-26T19:21:22Z

`/do` results

Step	Status	Duration	Verification
sync	✓	0s	forge=github; lost-crash worktree
research	✓	1m 24s	4 onclick call sites + inline theme-mirror + vendor `<script src>` (stays)
branch	✓	6s	stork-es-module from origin/master
implement	✓	3m 17s	stork.js (~125 LOC) + 4 attr swaps + theme-mirror folded in + 3 e2e scenarios
check	✓	40s	cabal build all
docs	✓	0s	CHANGELOG sub-bullet under existing #643 entry
fmt	✓	16s	pre-commit clean
commit	✓	33s	primary feature commit
hickey+lowy	✓	6m 0s	4 findings, 4 fixed; 9 No-ops
police	✓	4m 0s	rules clean; fact-check 1 fix; elegance 2 fixes
test	✓	5m 11s	e2e live 21/21; static 20+1-skipped
create-pr	✓	1m 21s	draft PR + hickey/lowy comment
ci	✓	1m 23s	`vira ci` passed on `7fac1359`
Total		24m 29s

Slowest step: hickey+lowy (6m 0s) — two parallel sub-agent reviews dominated, similar shape to #663's resumed run.

Optimization suggestions

test ran 5m 11s — both modes had to rebuild after the dirty-tree warning interrupted Nix's cached output. A subsequent run of just e2e-static + e2e-live after the first build cache was warm took ~7s combined; the cold first-run cost is the real story.
hickey+lowy and police reviewed a much smaller diff than Move interactive JS to ES modules with live-server cache busting #663 (10 files / +213 −108 vs ~900 LOC) but ran for 10m combined. The cost is the agent latency itself, not anything about the diff size — running them in parallel inside a single response (which I did) is the only knob.
No CI retries — five rounds of fixes (4 hickey/lowy + 3 police) all landed clean and vira ci passed first try.

Workflow completed at 2026-04-26T17:20Z. PR is ready for review: ships the last interactive behavior into the ES-module pattern established by #663, closes the inline-IIFE inventory named in #643.

Reproduce a post-morph regression in stork.js: the dialog renders unstyled after a route switch because the new #stork-wrapper element gets no stork-wrapper-edible{,-dark} class. The MutationObserver on <html>.class only fires when the theme is toggled; a plain navigation produces no observer fire, so the fresh wrapper stays un-classed and edible.css rules don't apply. This commit adds the @morph-tagged scenario asserting the wrapper carries one of the edible classes after navigate-then-search; it fails on the current code. Fix lands in the next commit.

…fter route switch) The MutationObserver on <html>.class only fires when the theme toggles. A plain morph navigation doesn't change <html>.class, so the new #stork-wrapper (a fresh element from the morphed-in body) never gets stork-wrapper-edible{,-dark} applied — and edible.css / edible-dark.css have no class to attach to. The user sees an unstyled <input> + <output> on every search opened after the first route switch. Fold applyStorkTheme() into the existing onMorph(...) callback that already marks the index stale. Same hook, same morph event — both concerns are 'something post-morph the new DOM needs from us'. The test added in da40d32 flips green. Issue #673 tracks the broader auto-morph e2e infrastructure that would surface this entire regression class automatically. This targeted fix is in scope for #672; the auto-morph mode lands separate.

#675) **Morph-survival of every behavior is now a structural property of the e2e suite, not an opt-in `@morph` tag.** Before this PR, exactly one path was tested via Ema's in-app morph nav (the toc-spy scenario from #663); every other scenario reached its target via `page.goto`, so the morph code path was uncovered for everything else. That gap is how both the toc-spy bug (#667) and the Stork theme-mirror-after-morph regression ([#672 review](#672)) shipped — each was invisible to the existing fresh-load tests. `EMANOTE_MODE=morph` reuses the live backend but rewrites every `When I open` step into an Ema-internal route switch. A `Before` hook primes each scenario with `page.goto("/")` + `await window.ema.ready`, then subsequent `I open` calls go through `window.ema.switchRoute(...)` and wait for `EMAHotReload`. *Every behavior assertion in the suite now implicitly runs against "the page that landed via morph", not via fresh load.* `just e2e-morph` and a third `E2E (morph)` CI step join the existing `live` and `static` runs. The mode-axis volatility is encapsulated in two new tiny modules: `tests/support/mode.ts` for env validation and the `Mode` union, `tests/support/navigation.ts` for `openRoute` / `morphNav` / `primeMorph`. *Step definitions stay completely mode-agnostic — they call `openRoute(page, url)` and the dispatch happens behind the boundary.* `hooks.ts` is now just Cucumber lifecycle plumbing. > **Note for reviewers**: a subtle bug surfaced once two morph navigations ran back-to-back in a single scenario (which never happened before — `live` only morphed once per `@morph` scenario). Splitting `await ema.ready` and the `addEventListener("EMAHotReload") + switchRoute(...)` into separate `page.evaluate` round-trips opens a Node-side gap where the script-reload that morph triggers can destroy the execution context before the listener is registered. The fix in `44ac526e` keeps the entire ready→listener→switchRoute sequence in one async evaluate so it runs back-to-back in browser context. Closes #673. _Generated by [`/do`](https://github.com/srid/agency) on Claude Code (model `claude-opus-4-7`)._

The Stork ES-module PR (#672) accumulated seven follow-up commits across /code-police and /hickey+/lowy. Three patterns recur often enough to warrant a project rule: - one-field options bags (replace with a positional param) - mirror flags shadowing DOM state (query the DOM) - vendor globals from separate <script src> (guard at module load) Plus two calibration examples for built-in rules: silent URL/parse fallbacks must log, and string literals across hot-path methods hit the rule-of-three at exactly three sites.

srid added 9 commits April 26, 2026 15:02

Merge remote-tracking branch 'origin/master' into stork-es-module

547f3db

srid mentioned this pull request Apr 26, 2026

e2e: run all scenarios after a single Ema morph nav as a third mode #673

Closed

srid added 3 commits April 26, 2026 18:41

Merge remote-tracking branch 'origin/master' into stork-es-module

2b5c539

srid marked this pull request as ready for review April 26, 2026 23:04

srid merged commit 06ff6dd into master Apr 26, 2026
5 checks passed

srid deleted the stork-es-module branch April 26, 2026 23:05

srid mentioned this pull request Apr 26, 2026

e2e: add morph mode that route-switches via window.ema.switchRoute #675

Merged

srid mentioned this pull request May 4, 2026

Default theme refresh: attached card, chip palette, heatmap timeline #699

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stork: drop inline IIFE for an ES module under _emanote-static/js/#672

Stork: drop inline IIFE for an ES module under _emanote-static/js/#672
srid merged 13 commits intomasterfrom
stork-es-module

srid commented Apr 26, 2026 •

edited

Loading

Uh oh!

srid commented Apr 26, 2026

Uh oh!

srid commented Apr 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

srid commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Try it locally

Uh oh!

srid commented Apr 26, 2026

Hickey/Lowy Analysis

Hickey rationale

Lowy rationale

No-op findings (kept verbatim from sub-agent review)

/code-police follow-ups (3 commits)

Uh oh!

srid commented Apr 26, 2026

/do results

Optimization suggestions

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

srid commented Apr 26, 2026 •

edited

Loading

`/code-police` follow-ups (3 commits)

`/do` results