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() +})