Align WithElements typing with runtime naming

.changeset/warm-planes-refactor.md

@@ -0,0 +1,18 @@
+---
+"@openproject/stimulus-elements": minor
+---
+
+Fix `WithElements` to camelize keys like the runtime, and warn on colliding element keys
+
+- `WithElements<{ menu_item: string }>` now yields `menuItemElement` /
+  `menuItemElements` / `hasMenuItemElement`, matching the runtime accessors.
+  Previously it produced the wrong `menu_itemElement` names; if you worked
+  around this by passing pre-camelized keys, those still work — but usages
+  typed against the old snake_case accessor names must be renamed.
+- Element keys whose generated accessor names collide (e.g. `foo` and `_foo`
+  both produce `hasFooElement`, or `foo`'s predicate vs `hasFoo`'s getter)
+  now emit a console warning naming both keys; the later definition wins that
+  property, as before.
+- The naming rule (key → accessor triple + attribute suffix) now lives in one
+  module, with the acronym behaviour (`htmlURL` → `html-u-r-l`) locked in by
+  tests.

.github/workflows/test.yml

@@ -17,4 +17,5 @@ jobs:
       - run: bun install --frozen-lockfile
       - run: bunx playwright install chromium --with-deps
       - run: bun run test
+      - run: bun run typecheck
       - run: bun run build

CONTEXT.md

@@ -0,0 +1,45 @@
+# Domain glossary
+
+Vocabulary for this library. Use these terms in code, tests, and reviews.
+
+## Terms
+
+**Element definition**
+The mapping from one `static elements` key to its accessor triple and
+attribute-override suffix. Owned by `src/element-definition.ts` — the single
+place the naming rule lives, at runtime (`elementDefinition`) and at the type
+level (`Camelize`, `WithElements`).
+
+**Accessor triple**
+The three generated properties per element definition: `xElement`
+(`Element | null`), `xElements` (`Element[]`), `hasXElement` (`boolean`),
+where `x` is the camelized key.
+
+**Attribute override**
+The per-instance selector override read from the controller element:
+`data-{identifier}-{suffix}-element`, where `{suffix}` is the dasherized
+element name. Non-empty values win over the static selector; read live.
+
+**Blessing**
+Stimulus's mechanism for extending controllers at registration time.
+`ElementsBlessing` (`src/blessing.ts`) turns `static elements` into the
+accessor triples via element definitions.
+
+## Recorded decisions
+
+- **Acronym lock-in.** Dasherization is naive and Stimulus-compatible:
+  `htmlURL` → suffix `html-u-r-l`. Locked in by tests; not to be "fixed".
+- **Collision policy.** The three generated names share one property
+  namespace. A clash between names from *different* raw keys (e.g. `foo` vs
+  `_foo` on `hasFooElement`, or `foo` vs `hasFoo` across kinds) warns and the
+  later definition wins that property. Same-raw-key redefinition (subclass
+  overriding a parent selector) stays silent. Nothing throws.
+- **Type level unions colliding roles.** `WithElements` cannot model
+  property-level last-wins (runtime key order decides), so a colliding name
+  gets the union of every role that generates it (e.g. `hasFooElement:
+  boolean | Element | null`), forcing callers to narrow. An intersection was
+  rejected: it is silently assignable to *both* roles on reads, hiding the
+  pathology instead of surfacing it.
+- **Type/runtime lockstep.** `Camelize<K>` mirrors the runtime `camelize`
+  regex exactly (ASCII-only, tail-recursive). Twin sample tables live in
+  `test/element-definition.test.ts` and `test/types.test-d.ts` — keep in sync.

README.md

@@ -92,9 +92,9 @@ class MyController extends Controller {
 }
 ```
 
-`WithElements` keys must already be camelCase (matching the generated accessor
-names) — e.g. use `menuItem`, not `menu_item`, in the type argument even though
-the runtime `static elements` key may be `menu_item`.
+`WithElements` camelizes keys exactly like the runtime, so you can pass your
+`static elements` keys verbatim — `menu_item` and `menuItem` both yield
+`menuItemElement` / `menuItemElements` / `hasMenuItemElement`.
 
 ## Releasing
 

index.ts

@@ -1,3 +1,3 @@
 export { installElements } from "./src/install"
 export { ElementsBlessing } from "./src/blessing"
-export type { WithElements } from "./src/types"
+export type { WithElements } from "./src/element-definition"

package.json

@@ -15,12 +15,13 @@
   "files": ["dist", "README.md"],
   "sideEffects": false,
   "scripts": {
-    "build": "bun build ./index.ts --outdir dist --format esm --external @hotwired/stimulus && tsc -p tsconfig.build.json",
+    "build": "rm -rf dist && bun build ./index.ts --outdir dist --format esm --external @hotwired/stimulus && tsc -p tsconfig.build.json",
     "changeset": "changeset",
     "changeset:version": "changeset version",
     "prepublishOnly": "bun run build",
     "release": "bun run build && changeset publish",
-    "test": "vitest run"
+    "test": "vitest run",
+    "typecheck": "tsc --noEmit"
   },
   "keywords": ["stimulus", "hotwired", "elements", "blessing", "dom"],
   "license": "MIT",

src/blessing.ts

@@ -1,4 +1,5 @@
-import { camelize, capitalize, dasherize, readInheritableStaticObjectPairs } from "./helpers"
+import { readInheritableStaticObjectPairs } from "./helpers"
+import { mergeElementDefinitions, type ElementDefinition } from "./element-definition"
 import { queryOne, queryAll } from "./query"
 
 interface ElementScope {
@@ -7,14 +8,11 @@ interface ElementScope {
 }
 
 export function ElementsBlessing(constructor: unknown): PropertyDescriptorMap {
-  const merged = new Map<string, string>()
-  for (const [key, selector] of readInheritableStaticObjectPairs<string>(constructor, "elements")) {
-    merged.set(key, selector) // later-wins → subclass overrides
-  }
+  const pairs = readInheritableStaticObjectPairs<string>(constructor, "elements")
 
   const properties: PropertyDescriptorMap = {}
-  for (const [key, selector] of merged) {
-    Object.assign(properties, propertiesForElementDefinition(camelize(key), selector))
+  for (const { definition, selector } of mergeElementDefinitions(pairs)) {
+    Object.assign(properties, propertiesForElementDefinition(definition, selector))
   }
   return properties
 }
@@ -25,20 +23,23 @@ function resolveSelector(scope: ElementScope, attrSuffix: string, staticSelector
   return staticSelector
 }
 
-function propertiesForElementDefinition(name: string, selector: string): PropertyDescriptorMap {
-  const attrSuffix = dasherize(name)
+function propertiesForElementDefinition(
+  def: ElementDefinition,
+  selector: string,
+): PropertyDescriptorMap {
+  const attrSuffix = def.attributeSuffix
   return {
-    [`${name}Element`]: {
+    [def.getterName]: {
       get(this: ElementScope): Element | null {
         return queryOne(this.element, resolveSelector(this, attrSuffix, selector))
       },
     },
-    [`${name}Elements`]: {
+    [def.pluralName]: {
       get(this: ElementScope): Element[] {
         return queryAll(this.element, resolveSelector(this, attrSuffix, selector))
       },
     },
-    [`has${capitalize(name)}Element`]: {
+    [def.predicateName]: {
       get(this: ElementScope): boolean {
         return queryOne(this.element, resolveSelector(this, attrSuffix, selector)) !== null
       },

src/element-definition.ts

@@ -0,0 +1,124 @@
+// Single owner of the element-naming rule: one `static elements` key maps to
+// the accessor triple (`xElement` / `xElements` / `hasXElement`) and the
+// attribute-override suffix (`data-{identifier}-{suffix}-element`).
+//
+// Acronym keys keep Stimulus's naive dasherization on purpose:
+// `htmlURL` → suffix `html-u-r-l`.
+
+export interface ElementDefinition {
+  readonly getterName: string
+  readonly pluralName: string
+  readonly predicateName: string
+  readonly attributeSuffix: string
+}
+
+export function elementDefinition(key: string): ElementDefinition {
+  const name = camelize(key)
+  return {
+    getterName: `${name}Element`,
+    pluralName: `${name}Elements`,
+    predicateName: `has${capitalize(name)}Element`,
+    attributeSuffix: dasherize(name),
+  }
+}
+
+// Merge raw `static elements` pairs into definitions:
+// - same raw key: later-wins silently (subclass overrides parent selector)
+// - different raw keys claiming the same generated property name: warn;
+//   the later descriptor wins that property (Object.assign semantics).
+// The three generated names share one property namespace, so `foo`/`_foo`
+// collide on the predicate and `foo`/`hasFoo` collide across kinds.
+export function mergeElementDefinitions(
+  pairs: [string, string][],
+): { definition: ElementDefinition; selector: string }[] {
+  const byRawKey = new Map<string, string>()
+  for (const [key, selector] of pairs) {
+    byRawKey.set(key, selector) // later-wins → subclass overrides
+  }
+
+  const claimedBy = new Map<string, string>()
+  const merged: { definition: ElementDefinition; selector: string }[] = []
+  for (const [key, selector] of byRawKey) {
+    const definition = elementDefinition(key)
+    for (const property of [
+      definition.getterName,
+      definition.pluralName,
+      definition.predicateName,
+    ]) {
+      const incumbent = claimedBy.get(property)
+      if (incumbent !== undefined && incumbent !== key) {
+        console.warn(
+          `[stimulus-elements] Element keys ${JSON.stringify(incumbent)} and ${JSON.stringify(key)} ` +
+            `both define property ${JSON.stringify(property)}; using ${JSON.stringify(selector)} from ${JSON.stringify(key)}`,
+        )
+      }
+      claimedBy.set(property, key)
+    }
+    merged.push({ definition, selector })
+  }
+  return merged
+}
+
+function camelize(value: string): string {
+  return value.replace(/[-_]([a-z0-9])/gi, (_match, char: string) => char.toUpperCase())
+}
+
+function capitalize(value: string): string {
+  return value.length === 0 ? value : value.charAt(0).toUpperCase() + value.slice(1)
+}
+
+function dasherize(value: string): string {
+  return value.replace(/([A-Z])/g, (_match, char: string) => `-${char.toLowerCase()}`)
+}
+
+type Separator = "-" | "_"
+type Digit = "0" | "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9"
+type LowerAlpha =
+  | "a" | "b" | "c" | "d" | "e" | "f" | "g" | "h" | "i" | "j" | "k" | "l" | "m"
+  | "n" | "o" | "p" | "q" | "r" | "s" | "t" | "u" | "v" | "w" | "x" | "y" | "z"
+// Mirrors the runtime regex char class [a-z0-9] with the `i` flag: ASCII only.
+type Camelizable = Digit | LowerAlpha | Uppercase<LowerAlpha>
+
+// Tail-recursive (accumulator) form — the nested form hits TS2589 on long keys.
+type CamelizeImpl<S extends string, Acc extends string> =
+  S extends `${infer Head}${infer Tail}`
+    ? Head extends Separator
+      ? Tail extends `${infer Next}${infer Rest}`
+        ? Next extends Camelizable
+          ? CamelizeImpl<Rest, `${Acc}${Uppercase<Next>}`>
+          : CamelizeImpl<Tail, `${Acc}${Head}`>
+        : `${Acc}${Head}`
+      : CamelizeImpl<Tail, `${Acc}${Head}`>
+    : `${Acc}${S}`
+
+// Type-level twin of `camelize` — must stay in lockstep with the regex above.
+export type Camelize<S extends string> = CamelizeImpl<S, "">
+
+// Numeric keys are stringified like the runtime (`Object.keys`) does.
+type ElementName<T> = Camelize<`${keyof T & (string | number)}`>
+
+type GetterNames<T> = `${ElementName<T>}Element`
+type PluralNames<T> = `${ElementName<T>}Elements`
+type PredicateNames<T> = `has${Capitalize<ElementName<T>>}Element`
+
+// Declaration-merging helper: describe the accessors a `static elements`
+// definition generates, so controllers get typed `this.xElement` access.
+//
+//   interface MyController extends WithElements<{ backdrop: string }> {}
+//   class MyController extends Controller {
+//     static elements = { backdrop: "#backdrop" }
+//   }
+//
+// Keys are camelized exactly like the runtime does, so snake_case and
+// kebab-case keys yield the same accessor names in both worlds.
+//
+// Each property is the union of every role that generates its name. For
+// non-colliding keys that is a single role and the exact accessor type;
+// when keys collide (`foo`/`hasFoo` both produce `hasFooElement`) the
+// union forces callers to narrow, since runtime key order decides.
+export type WithElements<T extends Record<string, string>> = {
+  [P in GetterNames<T> | PluralNames<T> | PredicateNames<T>]:
+    | (P extends GetterNames<T> ? Element | null : never)
+    | (P extends PluralNames<T> ? Element[] : never)
+    | (P extends PredicateNames<T> ? boolean : never)
+}

src/helpers.ts

@@ -1,15 +1,3 @@
-export function camelize(value: string): string {
-  return value.replace(/[-_]([a-z0-9])/gi, (_match, char: string) => char.toUpperCase())
-}
-
-export function capitalize(value: string): string {
-  return value.length === 0 ? value : value.charAt(0).toUpperCase() + value.slice(1)
-}
-
-export function dasherize(value: string): string {
-  return value.replace(/([A-Z])/g, (_match, char: string) => `-${char.toLowerCase()}`)
-}
-
 export function readInheritableStaticObjectPairs<T = unknown>(
   constructor: unknown,
   propertyName: string,

test/blessing.test.ts

@@ -142,6 +142,64 @@ test("override applies to plural and has accessors", () => {
   expect(ctrl.hasBackdropElement).toBe(true)
 })
 
+// Characterization: acronym keys keep Stimulus's naive dasherization —
+// `htmlURL` maps to the attribute suffix `html-u-r-l`. Locked-in behaviour.
+test("acronym key yields naive-dasherized override attribute", () => {
+  class C {
+    static elements = { htmlURL: "#backdrop" }
+  }
+  const host = fixture()
+  const ctrl = bless(C, host)
+  expect(ctrl.htmlURLElement).toBe(host.querySelector("#backdrop"))
+  expect(ctrl.hasHtmlURLElement).toBe(true)
+
+  host.setAttribute("data-test-html-u-r-l-element", ".item")
+  expect(ctrl.htmlURLElement).toBe(host.querySelector(".item"))
+})
+
+test("subclass same-key override does not warn", () => {
+  const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
+  class Base {
+    static elements = { thing: "#backdrop" }
+  }
+  class Child extends Base {
+    static override elements = { thing: ".item" }
+  }
+  bless(Child, fixture())
+  expect(warn).not.toHaveBeenCalled()
+  warn.mockRestore()
+})
+
+test("cross-key predicate collision warns; both getters stay, later predicate wins", () => {
+  const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
+  class C {
+    // foo and _foo produce distinct getters but the same hasFooElement predicate
+    static elements = { foo: ".nope", _foo: "#backdrop" }
+  }
+  const host = fixture()
+  const ctrl = bless(C, host)
+  expect(ctrl.fooElement).toBeNull()
+  expect(ctrl.FooElement).toBe(host.querySelector("#backdrop"))
+  // later key (_foo, "#backdrop") wins the shared predicate property
+  expect(ctrl.hasFooElement).toBe(true)
+  expect(warn).toHaveBeenCalledTimes(1)
+  warn.mockRestore()
+})
+
+test("cross-kind collision warns; hasFooElement resolves to later key's getter", () => {
+  const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
+  class C {
+    // foo's predicate hasFooElement vs hasFoo's singular getter hasFooElement
+    static elements = { foo: ".item", hasFoo: "#backdrop" }
+  }
+  const host = fixture()
+  const ctrl = bless(C, host)
+  // later definition's getter shadows the predicate: Element, not boolean
+  expect(ctrl.hasFooElement).toBe(host.querySelector("#backdrop"))
+  expect(warn).toHaveBeenCalledTimes(1)
+  warn.mockRestore()
+})
+
 test("invalid override selector warns once and falls back to null / []", () => {
   const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
   class C {