Grok repo idiomaticity review

[jakeboone02]

Code review findings

Review of main (package svelte-querybuilder @ 0.1.1) against the stated goal: idiomatic Svelte 5 UI on @react-querybuilder/core, with query/DOM parity and a Svelte-shaped API.

Packaging / dependency

• @react-querybuilder/core is resolved via a pkg.pr.new URL (...@3d5d992), not a stable npm version range. Unsuitable for consumers and for reproducible installs once published more widely. Pin to a released version (or an explicit range) and document the upgrade path.
• Package version is 0.1.1 while the public surface (controls, props, re-exports of core, history API) is large. No explicit stability / 1.0 criteria in README or docs.

Types and any

• Remaining any / loose bridges:
  • getSubQueryBuilderProps return typed as any
  • action handlers with optional context?: any
  • Control.svelte props typed as any
  • several as unknown as casts where core string identifiers meet stricter generics
• Prefer narrower types or localized generics so public and internal call sites stay checkable without casts.

Maintainability

• RuleComponents.svelte concentrates many getter-backed prop bags plus conditional control markup in one file. Prop-bag construction could be moved to small helpers (same pattern as withCommonProps) to reduce density without changing behavior.
• Empty HTML comment joiners between sibling controls are required for DOM text parity with React fixtures. They are documented in AGENTS.md and comments; still easy to “clean up” in a formatter pass. Consider a short CONTRIBUTING note or a conformance failure message that points at this constraint.

Reactivity edge cases

• createValueEditorReset is the main $effect-style path (timing for value reset on operator/type change). Keep dedicated tests on that path so behavior does not regress relative to core.
• Documented constraint: rebuilding query as a new object every read (e.g. query={{ ...q }}) reverts edits. Worth a short “reactivity gotchas” subsection (stable query reference, getter context, getter-backed control props) aimed at advanced consumers and custom controls.

Core alignment / future-proofing

• Compile-time guard accounts for core control keys not implemented here (dragHandle, ruleGroupBodyElements, ruleGroupHeaderElements). A CI check against core’s exported key list would make upgrades fail loudly when core adds keys.
• enableDragAndDrop is forced false; root always emits data-dnd="disabled". Consistent with non-goals; no change needed unless the non-goal list changes.

Tooling

• check:exports (attw + specifier lint) and deletion of test artifacts from dist are present. Keep both in the release path so Node ESM consumers and published tarballs stay clean.
• SSR is gated via examples/sveltekit (test:ssr). Treat as a hard CI requirement; do not drop when iterating on the library package alone.

Docs

• docs/differences-from-react-querybuilder.md covers intentional API divergences. Gaps relative to implementation detail:
  • stable query reference requirement (already in README warning; easy to miss)
  • why control props are getter-backed
  • context-as-getter requirement for nested / subquery builders
• No explicit list of what would block a 1.0 (API freeze, core version policy, remaining non-goals).

Suggested follow-ups (priority)

1. Replace pkg.pr.new core dependency with a stable npm version/range.
2. Tighten or document remaining any / cast sites.
3. Split prop-bag construction out of RuleComponents (optional refactor).
4. Add reactivity gotchas + 1.0 / core-pin policy to docs.
5. Optional CI: assert implemented control keys against core’s controlKeys / controlKind.