Grok repo idiomaticity review (narrowed)

[jakeboone02]

Scope

Review of the vue-querybuilder codebase against the goal of an idiomatic Vue 3 library that reuses @react-querybuilder/core for all query logic and preserves DOM parity with React Query Builder.

Narrowed 2026-09-21. Five of the eight original sections were already satisfied; two are refuted. See the audit comment for the full disposition table and the reasoning behind each closure. Only the open items remain below.

Architecture (context)

• Query state is owned by QueryManager from core.
• Reactive layer: shallowRef of immutable queries, useQueryBuilder / useRule / useRuleGroup / useQueryActions / useValueEditorReset, provide/inject for config inheritance, scoped slots for every control key.
• Four driving modes: defaultQuery, query + onQueryChange, v-model:query, external manager.
• Conformance suite asserts byte-identical DOM output against upstream fixtures.
• DnD, UI-framework packages, expr/datetime UI, and async option lists are explicitly out of scope for v1.

No change to the overall architecture is required; the items below are refinements toward a clearer, more idiomatic Vue API surface.

Open

1. Typing helpers for authors of replacement controls

ControlComponent stays unparameterized — the reasoning at src/types/controls.ts:190-210 holds and is not reopened. The ergonomics gap it leaves is real, though, and ControlPropsMap already holds the data.

Add a public alias:

export type ControlProps<
  K extends ControlKey,
  F extends FullField = FullField,
  O extends string = string,
> = ControlPropsMap<F, O>[K];

• K constrained to core's ControlKey, so the port cannot drift from core's canonical key list.
• Exported publicly from src/types/controls.ts.
• docs/customization.md gains defineProps<Pick<ControlProps<'valueEditor'>, 'value' | 'handleOnChange'>>().
• Gate: a src/types/types.test-d.ts case asserting mutual assignability with ValueEditorProps. Proven red by removing a member from the target.

Type-only. No runtime code, no coverage impact.

2. Pinia / store recipe (docs only)

Extend docs/customization.md "Driving the query from outside" with a Pinia subsection holding the QueryManager in a store.

Worth demonstrating rather than leaving implied: this is exactly the case core 8.23.0 made work. QueryManager now keeps its state in a non-enumerable, symbol-keyed own property carrying __v_skip, so a manager held in a store is readable through the proxy and is not deep-proxied. On an older core the same code threw Cannot read private member #past.

Scope: a fenced example only — no Pinia dependency, no examples/demo integration. Cross-reference the proxy-safety note in docs/differences-from-react-querybuilder.md §3.

3. The as unknown as slot merge, timeboxed

src/components/QueryBuilder.vue:94 casts the merged props/slots object through unknown. The likely cause is generic variance between the component's own RG parameter and the RuleGroupTypeAny that useQueryBuilder is called with — not a slot-merge typing gap, in which case a typed merge helper removes nothing.

Timebox to one sitting:

1. Try a typed merge helper and see whether the cast collapses to a single as, or disappears.
2. If the variance survives (expected), keep the cast and replace it with a one-line comment naming RG vs RuleGroupTypeAny as the reason.

Either outcome beats an unexplained double cast. Do not spend a second sitting on it.

Accepted from a refuted item

src/composables/useQueryBuilder.ts will not be split (see the audit comment), but it gains banner comments delimiting the five phases the original review named: manager lifecycle, reconfigure watcher and equality gate, resolver derivation, schema assembly, controlled write-back. Zero behavior change.

[jakeboone02]

Audit against the current tree

Re-audited on 2026-09-21 against @react-querybuilder/vue@0.2.0 / @react-querybuilder/core@^8.23.0. Five of the eight sections are already satisfied — several by the idiomaticity work that landed in 0.2.0, which overlapped this review substantially. One is refuted. Three are open.

§	Disposition	Evidence / note
1 package name + core dep	Done, in part refuted	@react-querybuilder/core is ^8.23.0, a published semver — the pkg.pr.new specifier is gone. The name is settled: @react-querybuilder/vue is final, not temporary. The unscoped vue-querybuilder will not happen, and every claim to the contrary has been removed from AGENTS.md, README.md, and the 0.2.0 changelog entry.
2 split useQueryBuilder	Refuted	See below.
3 controlled-mode feedback loop	Done	Guard at useQueryBuilder.ts:454-466; deep-clone behavior documented at docs/differences-from-react-querybuilder.md:115-119 ("a genuine replacement (for example a deep clone) is correctly pushed through"). manager.signatureOf is already public on core's manager, so the "expose it" half needs no work.
4 typing of replacement controls	Open	See below.
5 boolean defaults / wrapper note	Done	docs/differences-from-react-querybuilder.md:287-292, including "If you write a wrapper component around QueryBuilder, do the same". Declining the helper — see §7 below.
6 public vs internal surface	Done	RuleComponents, RuleGroupHeader, RuleGroupBody, RuleSubQuery are not exported (src/components/index.ts exports only the 11 public components plus defaultControlElements). Accessors are public via src/composables/accessors.ts. Boundary stated in §6 of the differences doc and at docs/customization.md:206.
7 Pinia recipe	Open	See below.
7 prop-forwarding helper	Refuted	See below.
8 as unknown as slot merge	Open	Still at QueryBuilder.vue:94.
8 ValueSelector selected comment	Done	src/components/ValueSelector.vue:38-43, plus differences doc :281-282.
8 live() note	Done	Doc comment at useQueryBuilder.ts:195-206; consumer-facing note at differences doc :274-275.

Refuted: §2, extracting useQueryManager

src/composables/useQueryBuilder.ts is 625 lines, but the five phases named in the review are not separable. The live() closures (:195-206), the schema bag, and the controlled write-back guard (:454-466) all close over the same manager and the same merged config; an extracted lifecycle composable would either return a ~15-member bag or re-derive the merged config a second time.

More to the point, the complaint is file size, not Vue idiom. Nothing in the file is a React-ism — the one genuinely un-Vue pattern a prior review found, the parts prop drilled into the internal components, is already gone in favor of provide/inject (src/internal/parts.ts). And this file is the highest-risk thing in the repo to churn: it is what stands between four driving modes and the conformance suite.

Accepted instead: banner comments delimiting the five phases the review itself names. Zero behavior change, and it answers the real complaint, which is navigability.

Refuted: §7, typed prop-forwarding helper

v-bind="$attrs" is the answer. That helper was only ever needed because the default controls set inheritAttrs: false; that flag is gone from all of them (only slotToComponent and the four internal components keep it, each deliberately). A bespoke helper layered on working native fallthrough would be worse than the built-in and would need its own gate to stay honest.

Open, and scoped

§4 — control-author typing helpers. The ControlComponent reasoning at src/types/controls.ts:190-210 stands and is not reopened: Component<P> passes its argument through as the instance type, so it rejects the useful case (a control declaring a subset) and admits the broken one (a control declaring a prop of the wrong type). But the review's actual ask is authoring ergonomics, and ControlPropsMap already holds the data. Adding a public alias:

export type ControlProps<
  K extends ControlKey,
  F extends FullField = FullField,
  O extends string = string,
> = ControlPropsMap<F, O>[K];

K is constrained to core's ControlKey so the port cannot drift from core's canonical key list. Docs get defineProps<Pick<ControlProps<'valueEditor'>, 'value' | 'handleOnChange'>>(). Gate: a src/types/types.test-d.ts case asserting mutual assignability with ValueEditorProps, proven red by removing a member from the target. Type-only; no runtime code.

§7 — Pinia recipe, docs only. Extending docs/customization.md "Driving the query from outside" with a Pinia subsection. Worth showing rather than leaving implied, because it is exactly the case core 8.23.0 made work: QueryManager now keeps its state in a non-enumerable, symbol-keyed own property carrying __v_skip, so a manager held in a store is readable through the proxy and is not deep-proxied. On an older core the same code threw Cannot read private member #past, so anyone who tried it and gave up needs telling. A fenced example only — no Pinia dependency and no examples/demo integration.

§8 — the as unknown as cast. The review reads QueryBuilder.vue:94 as a slot-merge typing gap; the likelier cause is generic variance between the component's RG and the RuleGroupTypeAny that useQueryBuilder is called with, in which case a typed merge helper removes nothing. Timeboxed to one sitting: try the helper, and if the variance survives, keep the cast and replace it with a one-line comment naming the reason. Either outcome beats an unexplained double cast.

Narrowing the issue body to the three open items accordingly.