Merge query helpers into one scopedQuery module

.changeset/quick-moles-search.md

@@ -0,0 +1,5 @@
+---
+"@openproject/stimulus-elements": patch
+---
+
+Merge the internal `queryOne`/`queryAll` helpers into one `scopedQuery(root, selector)` module returning `{ first, all, exists }`. The invalid-selector warn-once registry is now keyed per root element (WeakMap) instead of process-global, so warnings reset naturally with the DOM and the test-only `resetSelectorWarnings` export is gone. No public API change.

CONTEXT.md

@@ -25,6 +25,11 @@ Stimulus's mechanism for extending controllers at registration time.
 `ElementsBlessing` (`src/blessing.ts`) turns `static elements` into the
 accessor triples via element definitions.
 
+**Scoped query**
+The single DOM-lookup module (`src/query.ts`): `scopedQuery(root, selector)`
+returns `{ first, all, exists }`. Owns the falsy-root guard, invalid-selector
+handling, and the warn-once policy; callers never see those concerns.
+
 ## Recorded decisions
 
 - **Acronym lock-in.** Dasherization is naive and Stimulus-compatible:
@@ -40,6 +45,10 @@ accessor triples via element definitions.
   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.
+- **Warn-once is per root element.** The invalid-selector warning registry
+  is a `WeakMap` keyed by the query root, not a process-global set. Lifetime
+  is an implementation detail: warnings die with the element, tests need no
+  reset hook, and each controller element reports a bad selector once.
 - **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.

src/blessing.ts

@@ -1,6 +1,6 @@
 import { readInheritableStaticObjectPairs } from "./helpers"
 import { mergeElementDefinitions, type ElementDefinition } from "./element-definition"
-import { queryOne, queryAll } from "./query"
+import { scopedQuery } from "./query"
 
 interface ElementScope {
   element: Element
@@ -28,20 +28,22 @@ function propertiesForElementDefinition(
   selector: string,
 ): PropertyDescriptorMap {
   const attrSuffix = def.attributeSuffix
+  const query = (scope: ElementScope) =>
+    scopedQuery(scope.element, resolveSelector(scope, attrSuffix, selector))
   return {
     [def.getterName]: {
       get(this: ElementScope): Element | null {
-        return queryOne(this.element, resolveSelector(this, attrSuffix, selector))
+        return query(this).first()
       },
     },
     [def.pluralName]: {
       get(this: ElementScope): Element[] {
-        return queryAll(this.element, resolveSelector(this, attrSuffix, selector))
+        return query(this).all()
       },
     },
     [def.predicateName]: {
       get(this: ElementScope): boolean {
-        return queryOne(this.element, resolveSelector(this, attrSuffix, selector)) !== null
+        return query(this).exists()
       },
     },
   }

src/query.ts

@@ -1,52 +1,58 @@
-const warnedSelectors = new Set<string>()
+const warnedByRoot = new WeakMap<Element, Set<string>>()
 
-function warnOnce(selector: string, reason: unknown): void {
-  if (warnedSelectors.has(selector)) return
-  warnedSelectors.add(selector)
+function warnOnce(root: Element, selector: string, reason: unknown): void {
+  let seen = warnedByRoot.get(root)
+  if (!seen) {
+    seen = new Set()
+    warnedByRoot.set(root, seen)
+  }
+  if (seen.has(selector)) return
+  seen.add(selector)
   console.warn(
     `[stimulus-elements] Ignoring invalid selector ${JSON.stringify(selector)}:`,
     reason,
   )
 }
 
-function isNonEmptySelector(selector: string): boolean {
-  return typeof selector === "string" && selector.trim().length > 0
+export interface ScopedQuery {
+  first(): Element | null
+  all(): Element[]
+  exists(): boolean
 }
 
-export function resetSelectorWarnings(): void {
-  warnedSelectors.clear()
+const EMPTY_QUERY: ScopedQuery = {
+  first: () => null,
+  all: () => [],
+  exists: () => false,
 }
 
-export function queryOne(
+export function scopedQuery(
   root: Element | null | undefined,
   selector: string,
-): Element | null {
-  if (!root) return null
-  if (!isNonEmptySelector(selector)) {
-    warnOnce(selector, "selector is empty")
-    return null
-  }
-  try {
-    return root.querySelector(selector)
-  } catch (error) {
-    warnOnce(selector, error)
-    return null
+): ScopedQuery {
+  if (!root) return EMPTY_QUERY
+  if (typeof selector !== "string" || selector.trim().length === 0) {
+    warnOnce(root, selector, "selector is empty")
+    return EMPTY_QUERY
   }
-}
-
-export function queryAll(
-  root: Element | null | undefined,
-  selector: string,
-): Element[] {
-  if (!root) return []
-  if (!isNonEmptySelector(selector)) {
-    warnOnce(selector, "selector is empty")
-    return []
+  const first = (): Element | null => {
+    try {
+      return root.querySelector(selector)
+    } catch (error) {
+      warnOnce(root, selector, error)
+      return null
+    }
   }
-  try {
-    return Array.from(root.querySelectorAll(selector))
-  } catch (error) {
-    warnOnce(selector, error)
-    return []
+  return {
+    first,
+    all() {
+      try {
+        return Array.from(root.querySelectorAll(selector))
+      } catch (error) {
+        warnOnce(root, selector, error)
+        return []
+      }
+    },
+    exists: () => first() !== null,
   }
 }

test/blessing.test.ts

@@ -1,8 +1,5 @@
-import { test, expect, beforeEach, vi } from "vitest"
+import { test, expect, vi } from "vitest"
 import { ElementsBlessing } from "../src/blessing"
-import { resetSelectorWarnings } from "../src/query"
-
-beforeEach(() => resetSelectorWarnings())
 
 // Apply a blessing's descriptors onto a fake controller bound to `element`.
 function bless(constructor: unknown, element: Element, identifier = "test"): any {

test/integration.test.ts

@@ -1,12 +1,10 @@
 import { test, expect, beforeEach, afterEach, vi } from "vitest"
 import { Application, Controller } from "@hotwired/stimulus"
 import { installElements } from "../src/install"
-import { resetSelectorWarnings } from "../src/query"
 
 let app: Application
 
 beforeEach(() => {
-  resetSelectorWarnings()
   installElements()
 })
 

test/query.test.ts

@@ -1,8 +1,7 @@
 import { test, expect, beforeEach, vi } from "vitest"
-import { queryOne, queryAll, resetSelectorWarnings } from "../src/query"
+import { scopedQuery } from "../src/query"
 
 beforeEach(() => {
-  resetSelectorWarnings()
   document.body.innerHTML = `
     <section id="root">
       <span class="item">1</span>
@@ -17,49 +16,73 @@ function root(): Element {
   return document.getElementById("root")!
 }
 
-test("queryOne returns the first scoped match or null", () => {
-  expect(queryOne(root(), "#only")).toBe(document.getElementById("only"))
-  expect(queryOne(root(), ".missing")).toBeNull()
+test("first() returns the first scoped match or null", () => {
+  expect(scopedQuery(root(), "#only").first()).toBe(document.getElementById("only"))
+  expect(scopedQuery(root(), ".missing").first()).toBeNull()
 })
 
-test("queryOne is scoped to root — elements outside are not found", () => {
+test("first() is scoped to root — elements outside are not found", () => {
   // ".item" outside #root exists, but scoped query only sees the two inside
-  const found = queryOne(root(), ".item")
+  const found = scopedQuery(root(), ".item").first()
   expect(found).toBe(root().querySelector(".item"))
   expect(found!.textContent).toBe("1")
 })
 
-test("queryAll returns a real Array of scoped matches", () => {
-  const items = queryAll(root(), ".item")
+test("all() returns a real Array of scoped matches", () => {
+  const items = scopedQuery(root(), ".item").all()
   expect(Array.isArray(items)).toBe(true)
   expect(items.length).toBe(2) // outside .item excluded
   expect(items.map((el) => el.textContent)).toEqual(["1", "2"])
 })
 
-test("queryAll returns [] when nothing matches", () => {
-  expect(queryAll(root(), ".none")).toEqual([])
+test("all() returns [] when nothing matches", () => {
+  expect(scopedQuery(root(), ".none").all()).toEqual([])
 })
 
-test("missing root returns null / [] without warning", () => {
+test("exists() reports whether a scoped match is present", () => {
+  expect(scopedQuery(root(), "#only").exists()).toBe(true)
+  expect(scopedQuery(root(), ".missing").exists()).toBe(false)
+})
+
+test("methods can be destructured — no this dependence", () => {
+  const { first, all, exists } = scopedQuery(root(), ".item")
+  expect(first()!.textContent).toBe("1")
+  expect(all().length).toBe(2)
+  expect(exists()).toBe(true)
+})
+
+test("missing root returns null / [] / false without warning", () => {
   const warn = vi.spyOn(console, "warn")
-  expect(queryOne(null, ".item")).toBeNull()
-  expect(queryAll(undefined, ".item")).toEqual([])
+  expect(scopedQuery(null, ".item").first()).toBeNull()
+  expect(scopedQuery(undefined, ".item").all()).toEqual([])
+  expect(scopedQuery(null, ".item").exists()).toBe(false)
   expect(warn).not.toHaveBeenCalled()
   warn.mockRestore()
 })
 
-test("invalid selector warns once and returns null / []", () => {
+test("invalid selector warns once per root and returns null / []", () => {
+  const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
+  expect(scopedQuery(root(), "###").first()).toBeNull()
+  expect(scopedQuery(root(), "###").all()).toEqual([])
+  expect(warn).toHaveBeenCalledTimes(1) // once per (root, selector), across accesses
+  warn.mockRestore()
+})
+
+test("empty / whitespace selector warns once per root and returns null / []", () => {
   const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
-  expect(queryOne(root(), "###")).toBeNull()
-  expect(queryAll(root(), "###")).toEqual([])
-  expect(warn).toHaveBeenCalledTimes(1) // once per selector, across both calls
+  expect(scopedQuery(root(), "   ").first()).toBeNull()
+  expect(scopedQuery(root(), "   ").all()).toEqual([])
+  expect(warn).toHaveBeenCalledTimes(1)
   warn.mockRestore()
 })
 
-test("empty / whitespace selector warns once and returns null / []", () => {
+test("a fresh root gets its own warning — registry is per element, no reset needed", () => {
   const warn = vi.spyOn(console, "warn").mockImplementation(() => {})
-  expect(queryOne(root(), "   ")).toBeNull()
-  expect(queryAll(root(), "   ")).toEqual([])
+  scopedQuery(root(), "###").first()
   expect(warn).toHaveBeenCalledTimes(1)
+
+  document.body.innerHTML = `<section id="root"></section>`
+  scopedQuery(root(), "###").first()
+  expect(warn).toHaveBeenCalledTimes(2)
   warn.mockRestore()
 })