Skip to content

Grok repo idiomaticity review #3

Description

@jakeboone02

Scope

Review of the current 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.

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.

Opportunities for improvement

1. Package naming and dependency stability

  • Published name is temporarily @react-querybuilder/vue due to npm name conflict with vue-query-builder.
  • Core is currently resolved via a pkg.pr.new URL rather than a published version.
  • Before a real release: pin @react-querybuilder/core to a published version and decide the final package name so the rename is only a name change (exports map and peer range stay stable).

2. Size and structure of useQueryBuilder

packages/vue-querybuilder/src/composables/useQueryBuilder.ts is ~625 lines and combines:

  • Manager construction, seeding, and subscription
  • Structural reconfigure watcher and equality gate
  • Option-list / resolver derivation
  • Schema assembly
  • Controlled-mode write-back and feedback-loop guard

Suggested split:

  • Extract manager lifecycle (construct, seed, reconfigure, subscribe) into a focused composable (e.g. useQueryManager).
  • Keep schema/resolvers assembly and controlled write-back as separate helpers.

This would clarify the boundary between the Vue reactivity layer and core logic.

3. Controlled-mode feedback loop

The Object.is + manager.signatureOf() guard correctly prevents loops when a parent holds the query in reactive() and spreads it back. Gaps:

  • Behavior when a parent deep-clones the query is not documented.
  • No explicit path for consumers who intentionally want every new object reference to win (e.g. deep clone then push).

Document the signature comparison (or expose it) and note that a genuine deep clone is treated as a change only when the signature differs; otherwise consumers should call manager.setQuery themselves.

4. Typing of replacement controls

ControlComponent is intentionally unparameterized because Vue’s Component<P> does not model “parent always passes the full bag; consumer may declare a subset.” Slot props are exactly typed; the component form is loose.

Possible middle ground: keep the unparameterized map type, and add small helper types (ValueEditorControl, ActionElementControl, etc.) that authors can use when defining replacement SFCs so they get autocomplete for the props they care about without fighting the map.

5. Boolean prop defaults and wrapper components

Every boolean prop is given an explicit undefined default so Vue’s boolean casting does not turn an omitted prop into false and override inherited context. Wrapper authors who re-declare these props can easily forget the pattern and break inheritance.

Document a short “Writing a wrapper around QueryBuilder” note (or a small helper) that restates the requirement.

6. Public vs internal surface

RuleComponents, RuleGroupHeader, RuleGroupBody, and RuleSubQuery are internal and not exported. Accessors (useSchema, useCurrentRule, …) are public. Make the boundary explicit in docs and in the export surface so internals are not treated as stable API.

7. Additional Vue-native surface (low priority)

  • No first-class Pinia / store integration (external manager is the escape hatch).
  • Minimal “idiomatic <script setup> + v-model:query + a few slots” recipe could be expanded.
  • A typed helper for prop forwarding when building higher-order components would reduce friction for wrappers.

8. Minor code notes

  • QueryBuilder.vue uses as unknown as when merging slots into props; a small typed merge helper would remove the cast.
  • ValueSelector multi-select is driven via the selected attribute on each <option> rather than a value binding (required to avoid Vue stringifying and clearing the selection). A one-line comment in the component would help future readers.
  • The live() closure pattern for function props is sound; a short note on when a function prop must be stable vs when an inline arrow is acceptable would help consumers.

Summary of priorities

  1. Stabilize package name and core dependency for release.
  2. Thin useQueryBuilder by extracting manager lifecycle.
  3. Document / clarify controlled-mode signature behavior and deep-clone cases.
  4. Improve typing helpers for authors of replacement controls.
  5. Document boolean-default / wrapper pattern and public/internal boundary.

No change to the overall architecture (QueryManager + shallowRef + slots + provide/inject + DOM parity) is required; the items above are refinements toward a clearer, more idiomatic Vue API surface.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions