diff --git a/.changeset/quick-moles-search.md b/.changeset/quick-moles-search.md new file mode 100644 index 0000000..3a4db1e --- /dev/null +++ b/.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. diff --git a/CONTEXT.md b/CONTEXT.md index b2cd965..3b4ed02 100644 --- a/CONTEXT.md +++ b/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` 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. diff --git a/src/blessing.ts b/src/blessing.ts index 07ce359..496b38b 100644 --- a/src/blessing.ts +++ b/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() }, }, } diff --git a/src/query.ts b/src/query.ts index 6859c29..966a2f2 100644 --- a/src/query.ts +++ b/src/query.ts @@ -1,52 +1,58 @@ -const warnedSelectors = new Set() +const warnedByRoot = new WeakMap>() -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, } } diff --git a/test/blessing.test.ts b/test/blessing.test.ts index 0fa4d92..d5b0102 100644 --- a/test/blessing.test.ts +++ b/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 { diff --git a/test/integration.test.ts b/test/integration.test.ts index a0033b5..ff2d7ea 100644 --- a/test/integration.test.ts +++ b/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() }) diff --git a/test/query.test.ts b/test/query.test.ts index 26086c6..a8d8a16 100644 --- a/test/query.test.ts +++ b/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 = `
1 @@ -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 = `
` + scopedQuery(root(), "###").first() + expect(warn).toHaveBeenCalledTimes(2) warn.mockRestore() })