From 15d60b0e2cda46700fcf513aef35d126571d38fe Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Mon, 10 Aug 2026 22:58:42 +0200 Subject: [PATCH] fix: dedupe install across bundled package copies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit installElements guarded against double-install only within one module instance: the installed flag and the blessings.includes identity check both fail when the package is bundled twice (two dependency graphs), so each copy pushed its own ElementsBlessing. Tag the blessing with Symbol.for("@openproject/stimulus-elements. blessing") and dedupe on the tag: the global symbol registry is shared across module copies, so any copy recognises a blessing installed by another. Also pin the README's ordering invariant with a characterization test: controllers registered before installElements() silently lack accessors — Stimulus snapshots blessings at registration onto a shadow constructor and leaves no observable trace, so a late-install warning is not implementable without an import-time side effect (forbidden by sideEffects: false). Documented in README and install.ts. --- .changeset/calm-installs-dedupe.md | 5 ++++ README.md | 6 +++- src/install.ts | 24 ++++++++++----- test/install-dedupe.test.ts | 23 +++++++++++++++ test/install-order.test.ts | 47 ++++++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 9 deletions(-) create mode 100644 .changeset/calm-installs-dedupe.md create mode 100644 test/install-dedupe.test.ts create mode 100644 test/install-order.test.ts diff --git a/.changeset/calm-installs-dedupe.md b/.changeset/calm-installs-dedupe.md new file mode 100644 index 0000000..7a3c23e --- /dev/null +++ b/.changeset/calm-installs-dedupe.md @@ -0,0 +1,5 @@ +--- +"@openproject/stimulus-elements": patch +--- + +`installElements()` now dedupes across bundled copies of the package: the blessing is tagged with a `Symbol.for` key, so a second module instance (two dependency graphs bundling the library twice) recognises an already-installed blessing instead of pushing a duplicate. Also documents that installing after controllers were registered fails silently — Stimulus leaves no trace the library could warn on — and pins that failure mode with a test. diff --git a/README.md b/README.md index 698a15e..f6bc99a 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,11 @@ installElements() `installElements()` must run before you register any controllers / call `Application.start()` — blessings are snapshotted per controller at registration time, so installing afterward yields controllers without the -accessors. +accessors. This failure is **silent**: Stimulus leaves no trace of earlier +registrations the library could detect and warn about, so there is no +runtime error — the accessors are simply `undefined`. Calling +`installElements()` more than once is safe, including from two bundled +copies of this package. ## Usage diff --git a/src/install.ts b/src/install.ts index fb1ff4f..9993919 100644 --- a/src/install.ts +++ b/src/install.ts @@ -1,18 +1,26 @@ import { Controller } from "@hotwired/stimulus" import { ElementsBlessing } from "./blessing" +// Cross-bundle identity for the blessing: if this package is bundled twice +// (two dependency graphs), each copy has its own ElementsBlessing function, +// but Symbol.for resolves to the same global symbol — so any copy can +// recognise a blessing installed by another and skip the duplicate push. +const BLESSING_TAG = Symbol.for("@openproject/stimulus-elements.blessing") + let installed = false -// `installed` and the `blessings.includes` check are per-module-instance: if this -// package ends up bundled twice (e.g. via two different dependency graphs), each -// copy tracks its own state and will push its own `ElementsBlessing` onto -// `Controller.blessings`. That's harmless — later registration wins — but it's -// worth knowing this guard doesn't dedupe across module instances, only within one. +// Must run before any register()/Application.start(): Stimulus snapshots +// blessings per controller at registration time and leaves no observable +// trace of prior registrations, so a late install CANNOT be detected or +// warned about — controllers registered earlier just never gain accessors. +// That silent failure mode is pinned by test/install-order.test.ts. export function installElements(): void { if (installed) return + ;(ElementsBlessing as unknown as Record)[BLESSING_TAG] = true const blessings = (Controller as unknown as { blessings: Function[] }).blessings - if (!blessings.includes(ElementsBlessing)) { - blessings.push(ElementsBlessing) - } + const present = blessings.some( + (blessing) => (blessing as unknown as Record)[BLESSING_TAG] === true, + ) + if (!present) blessings.push(ElementsBlessing) installed = true } diff --git a/test/install-dedupe.test.ts b/test/install-dedupe.test.ts new file mode 100644 index 0000000..54c65ae --- /dev/null +++ b/test/install-dedupe.test.ts @@ -0,0 +1,23 @@ +import { test, expect } from "vitest" +import { Controller } from "@hotwired/stimulus" +import { installElements } from "../src/install" + +// Runs in its own file so the install module is in its virgin state. +test("installElements does not duplicate a blessing from another bundled copy", () => { + const blessings = (Controller as any).blessings as Function[] + const before = blessings.length + + // Simulate a second copy of this package (two dependency graphs bundling + // it twice): different function identity, same Symbol.for tag. + const foreign = function ElementsBlessing(): PropertyDescriptorMap { + return {} + } + ;(foreign as any)[Symbol.for("@openproject/stimulus-elements.blessing")] = true + blessings.push(foreign) + + installElements() + + // only the foreign copy is present — install recognised the tag and did + // not push a second, identically-behaving blessing + expect(blessings.length).toBe(before + 1) +}) diff --git a/test/install-order.test.ts b/test/install-order.test.ts new file mode 100644 index 0000000..a2d28bd --- /dev/null +++ b/test/install-order.test.ts @@ -0,0 +1,47 @@ +import { test, expect } from "vitest" +import { Application, Controller } from "@hotwired/stimulus" +import { installElements } from "../src/install" + +const tick = () => new Promise((r) => setTimeout(r, 20)) + +// Characterizes the README's hard invariant: installElements() must run +// before register()/start(). Stimulus snapshots blessings per controller at +// registration time and leaves no observable trace the library could warn +// on (see CONTEXT.md), so violating the invariant fails SILENTLY — this +// test pins that failure mode and would catch Stimulus ever changing it. +// Runs in its own file so no other test has installed the blessing first. +test("controllers registered before installElements silently lack accessors", async () => { + class EarlyController extends Controller { + static elements = { thing: ".thing" } + } + document.body.innerHTML = `
` + const app = Application.start() + app.register("early", EarlyController) + await tick() + const earlyEl = document.querySelector('[data-controller~="early"]')! + const early: any = app.getControllerForElementAndIdentifier(earlyEl, "early") + + // silent failure: no accessors, no warning, no error + expect(early.thingElement).toBeUndefined() + expect(early.hasThingElement).toBeUndefined() + + // installing afterwards does not retro-bless already-registered controllers + installElements() + expect(early.thingElement).toBeUndefined() + + // but a controller registered after install gains the accessors + class LateController extends Controller { + static elements = { thing: ".thing" } + } + document.body.insertAdjacentHTML( + "beforeend", + `
`, + ) + app.register("late", LateController) + await tick() + const lateEl = document.querySelector('[data-controller~="late"]')! + const late: any = app.getControllerForElementAndIdentifier(lateEl, "late") + expect(late.thingElement).toBe(lateEl.querySelector(".thing")) + + app.stop() +})