diff --git a/CHANGELOG.md b/CHANGELOG.md index dc026b1..b6f5801 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,4 +6,4 @@ All notable changes to `@bquery/eslint-plugin` are documented here. The format f ### Added -- Initial release: five bQuery-specific rules and a recommended flat config ([bQuery/bQuery#228](https://github.com/bQuery/bQuery/issues/228), developed in [bQuery/bQuery#278](https://github.com/bQuery/bQuery/pull/278)): `no-top-level-dollar`, `prefer-scoped-reactivity`, `no-dynamic-trusted-html`, `no-value-in-untracked` and `prefer-subpath-imports` (autofixes root imports to module subpaths from an export map generated from the published `@bquery/bquery` declaration files). +- Initial release: five bQuery-specific rules and a recommended flat config ([bQuery/bQuery#228](https://github.com/bQuery/bQuery/issues/228), developed in [bQuery/bQuery#278](https://github.com/bQuery/bQuery/pull/278)): `no-top-level-dollar`, `prefer-scoped-reactivity`, `no-dynamic-trusted-html`, `no-value-in-untracked` (reports only identifiers initialised from a bQuery signal factory, so DOM `input.value` and other non-signal `.value` reads are not flagged) and `prefer-subpath-imports` (autofixes root imports to module subpaths from an export map generated from the published `@bquery/bquery` declaration files). diff --git a/README.md b/README.md index 51324c4..16586be 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ ESLint rules for [bQuery.js](https://bquery.js.org) that turn its documented inv | [`no-top-level-dollar`](#no-top-level-dollar) | `warn` | — | `$()` calls that run on import and throw when the element is gone | | [`no-dynamic-trusted-html`](#no-dynamic-trusted-html) | `error` | suggestion | `trusted()` / `unsafeHtml()` on dynamic, unsanitized values | | [`prefer-scoped-reactivity`](#prefer-scoped-reactivity) | `warn` | suggestion | `signal()` / `computed()` / `effect()` that leak from a component | -| [`no-value-in-untracked`](#no-value-in-untracked) | `warn` | suggestion | `.value` reads in `untrack()` / `watch()` callbacks | +| [`no-value-in-untracked`](#no-value-in-untracked) | `warn` | suggestion | Signal `.value` reads in `untrack()` / `watch()` callbacks | | [`prefer-subpath-imports`](#prefer-subpath-imports) | `warn` | autofix | Imports from the root entry instead of `@bquery/bquery/` | All rules follow imports: they only fire for functions imported from `@bquery/bquery`, its module subpaths or `@bquery/bquery/full` (named, renamed or namespace imports). A local function that happens to be called `$` or `trusted` is ignored. @@ -173,7 +173,15 @@ Reading `signal.value` subscribes the current observer. Some callbacks do not su - `untrack(fn)` runs `fn` with dependency tracking off; - the callback of `watch(source, cb)`, `watchDebounce()` and `watchThrottle()` only re-runs when `source` changes. Other signals read inside it never trigger the callback (with `watch()` they only cause redundant re-evaluations of the internal effect). -A `.value` read there looks reactive but is not. The rule reports `.value` reads on identifiers directly inside such callbacks and suggests `.peek()`, which states the intent and never subscribes. Writes (`x.value = …`, `x.value++`), the callback's own parameters and locals, and reads in nested functions are ignored. +A `.value` read there looks reactive but is not. The rule reports `.value` reads of bQuery signals directly inside such callbacks and suggests `.peek()`, which states the intent and never subscribes. + +An identifier counts as a signal only when the rule can prove it: it is declared in the same file as a `const` (or a `let` / `var` that is never reassigned) and initialised with a call to a bQuery signal factory imported from `@bquery/bquery`, a module subpath or `/full` (named, renamed or namespace import): + +- `@bquery/bquery/reactive`: `signal()`, `computed()`, `linkedSignal()`, `persistedSignal()`, `readonly()`; +- `@bquery/bquery/component`: `useSignal()`, `useComputed()`, `useSlot()`; +- `useCookie()` (platform), `reducedMotionSignal()` (motion), `keyboardUserSignal()` / `focusVisible()` (a11y), `isActiveSignal()` (router), `draggablePosition()` / `sortableOrder()` (dnd). + +Everything else is ignored, because its `.value` may not belong to a signal and `.peek()` would throw: DOM elements (`input.value`), `Map` entries and plain objects, function parameters, signals imported from other modules or passed around, destructured bindings and unresolved globals. Writes (`x.value = …`, `x.value++`), the callback's own parameters and locals, and reads in nested functions are ignored too. ```ts // ❌ Bad — changing `page` does not re-run the callback @@ -196,6 +204,12 @@ watch(query, (next) => { const snapshot = untrack(() => count.peek()); ``` +```ts +// ✅ Not reported — `input` is a DOM element, not a signal +const input = document.querySelector('input'); +watch(query, () => search(input.value)); +``` + ### prefer-subpath-imports The root `@bquery/bquery` entry re-exports every module. Importing from the module subpath (`@bquery/bquery/reactive`, `@bquery/bquery/core`, …) keeps the module graph small for bundlers, dev servers and tests, and documents which module a name comes from. diff --git a/src/rules/no-value-in-untracked.js b/src/rules/no-value-in-untracked.js index d8ca53d..0d1c975 100644 --- a/src/rules/no-value-in-untracked.js +++ b/src/rules/no-value-in-untracked.js @@ -8,6 +8,13 @@ * inside it never trigger it (for `watch` they only cause redundant * re-evaluations of the internal effect). * + * Only identifiers that provably hold a bQuery signal are reported: a + * `const` (or never reassigned `let` / `var`) initialised with a call to one + * of bQuery's signal factories (`signal()`, `computed()`, `useSignal()`, …). + * Anything else — DOM elements, `Map` entries, parameters, imports from other + * modules, globals — may have a `.value` without a `.peek()`, so it is left + * alone. + * * @module @bquery/eslint-plugin/rules/no-value-in-untracked */ @@ -15,6 +22,27 @@ import { bquerySources, docsUrl, findVariable, isFunction, matchBQueryCall } fro const SOURCES = bquerySources('reactive'); +/** + * bQuery factories whose result has a tracked `.value` and an untracked + * `.peek()` (they return `Signal`, `Computed`, `LinkedSignal` or + * `ReadonlySignal`), grouped by the module that exports them. Checked against + * the published `@bquery/bquery` 1.17 declaration files. + * + * @type {Array<[Set, Set]>} + */ +const SIGNAL_FACTORIES = [ + [ + bquerySources('reactive'), + new Set(['signal', 'computed', 'linkedSignal', 'persistedSignal', 'readonly']), + ], + [bquerySources('component'), new Set(['useSignal', 'useComputed', 'useSlot'])], + [bquerySources('platform'), new Set(['useCookie'])], + [bquerySources('motion'), new Set(['reducedMotionSignal'])], + [bquerySources('a11y'), new Set(['keyboardUserSignal', 'focusVisible'])], + [bquerySources('router'), new Set(['isActiveSignal'])], + [bquerySources('dnd'), new Set(['draggablePosition', 'sortableOrder'])], +]; + /** Untracking helpers → index of the untracked callback argument. */ const CALLBACK_INDEX = new Map([ ['untrack', 0], @@ -80,6 +108,52 @@ const untrackedHelper = (context, fn) => { return match.name; }; +/** + * Unwrap TypeScript-only wrappers (`x as T`, `x!`, `x satisfies T`, `x`). + * + * @param {import('../utils.js').AnyNode} node - Expression + * @returns {import('../utils.js').AnyNode} The wrapped runtime expression + */ +const unwrap = (node) => { + let current = node; + while ( + current?.type === 'TSAsExpression' || + current?.type === 'TSNonNullExpression' || + current?.type === 'TSSatisfiesExpression' || + current?.type === 'TSTypeAssertion' + ) { + current = current.expression; + } + return current; +}; + +/** + * Whether a variable provably holds a bQuery signal: it has a single + * `const` / `let` / `var` declaration with a plain identifier, is initialised + * with a call to a bQuery signal factory (named, renamed or namespace + * import), and is never reassigned. + * + * @param {import('eslint').Rule.RuleContext} context - Rule context + * @param {import('eslint').Scope.Variable} variable - Variable to inspect + * @returns {boolean} True for confirmed signals + */ +const holdsSignal = (context, variable) => { + if (variable.defs.length !== 1) return false; + const [def] = variable.defs; + if (def.type !== 'Variable' || def.node.id.type !== 'Identifier') return false; + if (def.parent.kind !== 'const') { + // `let` / `var`: the initialiser is the only write. + if (variable.references.some((reference) => reference.isWrite() && !reference.init)) { + return false; + } + } + const init = unwrap(def.node.init); + if (init?.type !== 'CallExpression') return false; + return SIGNAL_FACTORIES.some(([sources, names]) => + Boolean(matchBQueryCall(context, init, sources, names)) + ); +}; + /** * Whether `inner` lies inside `outer` (by source range). * @@ -97,7 +171,7 @@ export default { hasSuggestions: true, docs: { description: - 'Flag `.value` reads inside `untrack()` and `watch()` callbacks, which do not subscribe, and suggest `.peek()`', + 'Flag `.value` reads of bQuery signals inside `untrack()` and `watch()` callbacks, which do not subscribe, and suggest `.peek()`', recommended: true, url: docsUrl('no-value-in-untracked'), }, @@ -129,6 +203,9 @@ export default { const variable = findVariable(sourceCode.getScope(node), name); if (!variable || variable.defs.length === 0) return; if (variable.defs.some((def) => isWithin(def.name, fn))) return; + // Only identifiers that provably hold a bQuery signal: `.peek()` does + // not exist on a DOM input, a `Map` entry or an unknown import. + if (!holdsSignal(context, variable)) return; const data = { name, helper }; context.report({ diff --git a/test/no-value-in-untracked.test.ts b/test/no-value-in-untracked.test.ts index 5032709..0112289 100644 --- a/test/no-value-in-untracked.test.ts +++ b/test/no-value-in-untracked.test.ts @@ -40,6 +40,43 @@ ruleTester.run('no-value-in-untracked', rule, { `import { untrack, signal } from '@bquery/bquery'; const s = signal(1); untrack(() => s['value']);`, + // Only confirmed bQuery signals have `.peek()`: a DOM input's `.value` + // is not a signal read (and `input.peek()` would throw). + `import { watch, signal } from '@bquery/bquery/reactive'; + const query = signal(''); + const input = document.querySelector('input'); + watch(query, () => search(input.value));`, + // Map entries, objects with a `value` field. + `import { untrack } from '@bquery/bquery/reactive'; + const fields = new Map(); + const entry = fields.get('name'); + const option = { value: 1 }; + untrack(() => [entry.value, option.value]);`, + // Parameters of an enclosing function. + `import { watch, signal } from '@bquery/bquery'; + const query = signal(''); + export function bind(field) { watch(query, () => field.value); }`, + // Imports from other modules may or may not be signals. + `import { watch, signal } from '@bquery/bquery'; + import { filter } from './state'; + const query = signal(''); + watch(query, () => filter.value);`, + // Factories that are not bQuery's, or not signal factories. + `import { untrack } from '@bquery/bquery/reactive'; + import { signal } from '@preact/signals-core'; + import { effect } from '@bquery/bquery/reactive'; + const a = signal(0); + const b = effect(() => {}); + untrack(() => a.value + b.value);`, + // A `let` that is reassigned may no longer hold the signal. + `import { untrack, signal } from '@bquery/bquery/reactive'; + let current = signal(0); + current = { value: 1 }; + untrack(() => current.value);`, + // Destructured bindings are not followed. + `import { untrack, signal } from '@bquery/bquery/reactive'; + const [first] = [signal(0)]; + untrack(() => first.value);`, ], invalid: [ { @@ -83,7 +120,7 @@ observe(query, (next) => { load(next, page.peek()); });`, }, { code: `import * as reactive from '@bquery/bquery/reactive'; -import { filter, sort } from './state'; +const filter = reactive.signal(''); const sort = reactive.computed(() => 'asc'); reactive.watchDebounce(filter, function () { render(filter.value, sort?.value); }, 100); reactive.watchThrottle(filter, () => sort.value(), 100);`, errors: [ @@ -94,7 +131,7 @@ reactive.watchThrottle(filter, () => sort.value(), 100);`, { messageId: 'usePeek', output: `import * as reactive from '@bquery/bquery/reactive'; -import { filter, sort } from './state'; +const filter = reactive.signal(''); const sort = reactive.computed(() => 'asc'); reactive.watchDebounce(filter, function () { render(filter.peek(), sort?.value); }, 100); reactive.watchThrottle(filter, () => sort.value(), 100);`, }, @@ -107,7 +144,7 @@ reactive.watchThrottle(filter, () => sort.value(), 100);`, { messageId: 'usePeek', output: `import * as reactive from '@bquery/bquery/reactive'; -import { filter, sort } from './state'; +const filter = reactive.signal(''); const sort = reactive.computed(() => 'asc'); reactive.watchDebounce(filter, function () { render(filter.value, sort?.peek()); }, 100); reactive.watchThrottle(filter, () => sort.value(), 100);`, }, @@ -120,7 +157,7 @@ reactive.watchThrottle(filter, () => sort.value(), 100);`, { messageId: 'usePeek', output: `import * as reactive from '@bquery/bquery/reactive'; -import { filter, sort } from './state'; +const filter = reactive.signal(''); const sort = reactive.computed(() => 'asc'); reactive.watchDebounce(filter, function () { render(filter.value, sort?.value); }, 100); reactive.watchThrottle(filter, () => sort.peek()(), 100);`, }, @@ -128,5 +165,82 @@ reactive.watchThrottle(filter, () => sort.peek()(), 100);`, }, ], }, + { + // Renamed factory imports and the root namespace; TS wrappers on the + // initialiser; a never-reassigned `let`. + code: `import { computed as derive, signal as createSignal, untrack } from '@bquery/bquery'; +import type { Signal } from '@bquery/bquery/reactive'; +const total = derive(() => 1); +let count = createSignal(0) as Signal; +untrack(() => total.value + count.value);`, + errors: [ + { + messageId: 'valueInUntracked', + data: { name: 'total', helper: 'untrack' }, + suggestions: [ + { + messageId: 'usePeek', + output: `import { computed as derive, signal as createSignal, untrack } from '@bquery/bquery'; +import type { Signal } from '@bquery/bquery/reactive'; +const total = derive(() => 1); +let count = createSignal(0) as Signal; +untrack(() => total.peek() + count.value);`, + }, + ], + }, + { + messageId: 'valueInUntracked', + data: { name: 'count', helper: 'untrack' }, + suggestions: [ + { + messageId: 'usePeek', + output: `import { computed as derive, signal as createSignal, untrack } from '@bquery/bquery'; +import type { Signal } from '@bquery/bquery/reactive'; +const total = derive(() => 1); +let count = createSignal(0) as Signal; +untrack(() => total.value + count.peek());`, + }, + ], + }, + ], + }, + { + // Namespace import of the full bundle; signal factories from other modules. + code: `import * as bq from '@bquery/bquery/full'; +import { useCookie } from '@bquery/bquery/platform'; +const theme = useCookie('theme'); +const page = bq.linkedSignal(() => 1, () => {}); +bq.untrack(() => [theme.value, page.value]);`, + errors: [ + { + messageId: 'valueInUntracked', + data: { name: 'theme', helper: 'untrack' }, + suggestions: [ + { + messageId: 'usePeek', + output: `import * as bq from '@bquery/bquery/full'; +import { useCookie } from '@bquery/bquery/platform'; +const theme = useCookie('theme'); +const page = bq.linkedSignal(() => 1, () => {}); +bq.untrack(() => [theme.peek(), page.value]);`, + }, + ], + }, + { + messageId: 'valueInUntracked', + data: { name: 'page', helper: 'untrack' }, + suggestions: [ + { + messageId: 'usePeek', + output: `import * as bq from '@bquery/bquery/full'; +import { useCookie } from '@bquery/bquery/platform'; +const theme = useCookie('theme'); +const page = bq.linkedSignal(() => 1, () => {}); +bq.untrack(() => [theme.value, page.peek()]);`, + }, + ], + }, + ], + }, ], });