Repository navigation
feat: add performance/state-raw — suggest $state.raw for reassign-only object state - #276
Conversation
…acts Adds ComponentFacts.rawableStates plus the isPlainStateCall predicate and collectAliasRefs/collectFragmentAliasRefs/collectEachContextTaint collectors, wired into parseComponentFacts. Fixes two defects surfaced by the parse tests: collectAliasRefs was walking a VariableDeclarator's own `id` as a self-reference (skip it, like the existing MemberExpression.property / Property.key skips), and collectEachContextTaint scanned the EachBlock node itself instead of its body, which self-shadowed the context binding via scopeIntroducedNames and hid item-level mutations.
collectAliasRefs skipped a VariableDeclarator's whole `id` subtree to avoid
treating the candidate's own binding as a self-reference, but that also
silenced real aliasing references hiding inside the pattern: destructuring
default values (`const { a = obj } = x`) and computed keys
(`const { [obj]: a } = x`). Add collectPatternAliasRefs, a small
pattern-only walk (ObjectPattern/ArrayPattern/RestElement/AssignmentPattern)
that skips bound Identifiers but hands AssignmentPattern.right and computed
Property.key back to collectAliasRefs for normal reference tracking.
The collectAliasRefs and collectEachContextTaint code blocks in the plan still had the two bugs the Task 2 implementation fixed (VariableDeclarator.id silencing destructuring-default/computed-key aliases, and collectEachContextTaint self-shadowing the each-context binding by scanning the EachBlock node instead of its body). Replace both blocks with the corrected versions so Task 3/4 don't copy the buggy code forward.
…ntFacts fixture tsc --noEmit failed because the state-raw core change added a required ComponentFacts field that this fixture never picked up.
…te-raw
`{#each obj.items as item}` + `bind:value={item.text}` never tainted `obj`
because collectEachContextTaint only matched a bare Identifier each-expression,
so an editable nested list was told to go raw. Resolve MemberExpression each
expressions through their root object too.
`use:action={obj}` (and `transition:`/`animate:` params) hand the candidate to
arbitrary code that may mutate it or rely on its reactivity — add a
state-raw-only directive-escape collector (verified against svelte 5 modern-AST
type names: UseDirective/TransitionDirective/AnimateDirective) without touching
the shared collectTemplateEscapes, which correctness/unmutated-state relies on.
Condition 5 now covers a member path of the candidate; condition 4's escape
list now covers use:/transition:/animate: directive expressions (a new-rule-only
collector — the shared template-escape collector stays unchanged for
unmutated-state). Notes the use:action={state} escape gap for unmutated-state
as a deliberate follow-up, not changed on this branch.
Rebuild bundled action dist after the state-raw taint fixes in packages/core.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesRaw state performance rule
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SvelteFile
participant CoreParser
participant ComponentFacts
participant StateRawRule
SvelteFile->>CoreParser: parse component script and template
CoreParser->>ComponentFacts: compute rawableStates
ComponentFacts->>StateRawRule: provide candidate bindings
StateRawRule->>SvelteFile: emit info diagnostic and $state.raw fix
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/plans/2026-07-22-state-raw.md`:
- Line 45: Repair the doc comment near the component-parse recorder signature by
removing the nested backticks around the kinds identifier and retaining a single
code span for kinds within the surrounding text. Preserve the documented
classification values and the unchanged acc union contract.
- Around line 480-485: Update the wiring snippet around collectAliasRefs and
collectFragmentAliasRefs to invoke collectDirectiveEscapes for the program’s
directive expressions, ensuring use:, transition:, and animate: references
disqualify candidates. Add a regression test covering directive-based escapes so
the plan’s implementation contract is verified.
- Around line 426-429: Update the EachBlock handling around expr and ctxNames to
resolve the root object of member expressions such as obj.items before checking
names membership and shadowing; use that resolved root for tainting while
preserving the existing Identifier behavior and avoiding shadowed roots.
- Line 20: Replace the hard-coded registration-count assertions with structural
registration-site checks. In docs/superpowers/plans/2026-07-22-state-raw.md
lines 20 and 631, use a checklist naming each required registration location
instead of exact grep or match counts; in
docs/superpowers/specs/2026-07-22-state-raw-design.md lines 67-68, describe the
registration locations directly without fixed hit counts or ID ranges.
In `@docs/superpowers/specs/2026-07-22-state-raw-design.md`:
- Line 8: Update the inline Markdown in the introductory Svelte best-practices
quote so `$state` and the following text are separated by a space, and add
spacing before the `$state.raw` code span. Keep `$state.raw` entirely within its
own inline-code span without changing the quoted guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 303259bc-3328-4436-b8e4-7c35a5521991
⛔ Files ignored due to path filters (1)
packages/action/dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (22)
.changeset/state-raw.mddocs/src/content/docs/ja/rules/performance/state-raw.mddocs/src/content/docs/rules/performance/state-raw.mddocs/superpowers/plans/2026-07-22-state-raw.mddocs/superpowers/specs/2026-07-22-state-raw-design.mdpackages/cli/test/malformed-svelte.test.tspackages/cli/test/suppression-e2e.test.tspackages/core/src/component-collect.tspackages/core/src/component-parse.tspackages/core/src/component.tspackages/core/src/index.tspackages/core/src/rules/index.tspackages/core/src/rules/perf/state-raw.tspackages/core/test/architecture-rules.test.tspackages/core/test/bundle-rules.test.tspackages/core/test/component-collect.test.tspackages/core/test/component-rule.test.tspackages/core/test/correctness-rules.test.tspackages/core/test/security-kit-rules.test.tspackages/core/test/security-rules.test.tspackages/core/test/state-raw-parse.test.tspackages/core/test/state-raw-rule.test.ts
Summary
Last of the three rules from the Svelte best-practices survey:
performance/state-raw(info) suggests$state.rawfor object/array-literal$statebindings that are only ever reassigned — Svelte's own guidance for large reassign-only objects (API responses, canonically), whose deep-proxy overhead taxes every property access with no consumer.Design: never suggest a behavior-breaking change
An info-level suggestion that would BREAK the component if followed is worse than silence, so the conditions are strict — a candidate survives only when nothing could depend on deep reactivity:
correctness/unmutated-state, which requires zero writes)delete, member updates, method calls) and no escapes (call arguments, component props,bind:,use:/transition:/animate:directive params)list = [...list, x]stays a qualifying reassign, whileobj = (cache = obj),const inner = obj, helperreturn obj, inline-handler stores, and destructuring defaults (const { a = obj } = x) all disqualify{#each}blocks over the candidate or a member path of it ({#each obj.items as item}+bind:value={item.text}— an editable list must stay deeply reactive)Under the hood,
collectStateWrites/collectTemplateEscapesgained an optional reassign/mutate/escape classifier with the existing union contract untouched (verified byte-identical against a merge-base build), sounmutated-stateandstale-prop-derivationare unaffected.Review process
The design spec went through two adversarial review rounds before implementation (they added the aliasing conditions, the each-context taint, and the plain-
$statepredicate). During implementation the Task 2 agent found and fixed two bugs in the plan's own collector code (declarator-id self-reference; each-block self-shadowing), upheld by its reviewer. The final whole-branch review ran 39+ empirical probes against the built dist and caught two more behavior-breaking holes — member-path each blocks and directive escapes — both fixed on-branch with pinned tests and re-probed.Verification
oxlint+oxfmt --checkclean; docs build green (153 pages)docs-linksgate green; changeset (minor × core/cli/vite/mcp);packages/action/distrebuilt🤖 Generated with Claude Code
Summary by CodeRabbit
performance/state-rawrule to detect$stateobject/array bindings that are reassigned but never mutated, escaped, aliased, or edited in list contexts, recommending$state.raw.performance/state-raw, including detection criteria, limitations, examples, and how to disable the rule.