Dedupe install across bundled copies; pin the ordering invariant

.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.

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
 

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<symbol, boolean>)[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<symbol, boolean>)[BLESSING_TAG] === true,
+  )
+  if (!present) blessings.push(ElementsBlessing)
   installed = true
 }

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

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 = `<div data-controller="early"><span class="thing"></span></div>`
+  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",
+    `<div data-controller="late"><span class="thing"></span></div>`,
+  )
+  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()
+})