Repository navigation
feat: add correctness/stale-prop-derivation — flag prop-derived values frozen at init - #267
Conversation
Adds the stalePropDerivations fact to ComponentFacts, plumbed through parseComponentFacts (and empty for parseModuleFacts/emptyComponentFacts): a generalized collectPropNames(program, includeBindable) replaces collectNonBindableProps, scopeIntroducedNames now also tracks each-index, snippet, and await value/error bindings, and two new shadow-aware walkers (refsNamesEagerly, collectFragmentRefs) find top-level const/let bindings that eagerly capture a prop at init and are referenced in the template. Required updating every other ComponentFacts fixture/pin (core rule tests and two cli e2e tests) to include the new required field.
Add the rule via componentRule factory with optional fix support. Extends component-rule.ts to handle fix option (matching kit-module-rule pattern). Registers in four places: rules/index.ts (import, allRules, re-export) and core/index.ts re-export. Updates unmutated-state recommendation to mention $derived for prop/state computations.
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesStale prop derivation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Component
participant Parser
participant Rule
participant Reporter
Component->>Parser: Parse props, bindings, and template references
Parser->>Rule: Provide stalePropDerivations facts
Rule->>Reporter: Emit line diagnostic and $derived fix
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-07-22-stale-prop-derivation-design.md (1)
68-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hard-coded registration counts in guides. Export layout changes will make “four places” and “exactly 5” stale; describe the required registration categories instead.
docs/superpowers/specs/2026-07-22-stale-prop-derivation-design.md#L68-L68: replace the fixed location/hit counts with registration categories.docs/superpowers/plans/2026-07-22-stale-prop-derivation.md#L21-L21: remove the exact grep-count requirement.As per coding guidelines, “Do not hard-code rule counts or ID ranges in READMEs or guides; refer to rule categories instead.”
🤖 Prompt for 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. In `@docs/superpowers/specs/2026-07-22-stale-prop-derivation-design.md` at line 68, Replace fixed registration-location and grep-hit counts in docs/superpowers/specs/2026-07-22-stale-prop-derivation-design.md at lines 68-68 with the required registration categories. In docs/superpowers/plans/2026-07-22-stale-prop-derivation.md at lines 21-21, remove the exact grep-count requirement and describe the applicable categories instead.Source: Coding guidelines
🤖 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 `@packages/core/src/component-parse.ts`:
- Around line 447-455: Update collectFragmentRefs in
packages/core/src/component-parse.ts (lines 447-455) so EachBlock and AwaitBlock
header expressions are traversed with the incoming scope, then apply their
context/index/value/error bindings only while traversing the corresponding
bodies; add the same-name each-header regression in
packages/core/test/stale-prop-derivation-parse.test.ts (lines 40-48), asserting
the outer candidate is reported.
In `@packages/core/src/component.ts`:
- Around line 97-101: Make ComponentFacts.stalePropDerivations optional to
preserve compatibility with downstream object literals, and update internal
collectors consuming this field to default missing values to an empty array
before use.
In `@packages/core/src/rules/correctness/stale-prop-derivation.ts`:
- Around line 17-20: Remove the hard-coded Fix.snippet from the fix metadata in
the stale-prop derivation rule, or generate it using the actual offending
binding and expression for each result. Update componentRule so every penalized
finding receives only a valid, context-specific fix rather than the shared
color/type transformation.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-07-22-stale-prop-derivation-design.md`:
- Line 68: Replace fixed registration-location and grep-hit counts in
docs/superpowers/specs/2026-07-22-stale-prop-derivation-design.md at lines 68-68
with the required registration categories. In
docs/superpowers/plans/2026-07-22-stale-prop-derivation.md at lines 21-21,
remove the exact grep-count requirement and describe the applicable categories
instead.
🪄 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
Run ID: 0354b665-880a-4ac5-8839-7b2ac278d2f8
⛔ Files ignored due to path filters (1)
packages/action/dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (24)
.changeset/stale-prop-derivation.mddocs/src/content/docs/ja/rules/correctness/stale-prop-derivation.mddocs/src/content/docs/rules/correctness/stale-prop-derivation.mddocs/superpowers/plans/2026-07-22-stale-prop-derivation.mddocs/superpowers/specs/2026-07-22-stale-prop-derivation-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/component-rule.tspackages/core/src/rules/correctness/stale-prop-derivation.tspackages/core/src/rules/correctness/unmutated-state.tspackages/core/src/rules/index.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/stale-prop-derivation-parse.test.tspackages/core/test/stale-prop-derivation.test.ts
…tale-prop refs
{#each items as items} and {#await p then p} hid an outer prop/derived
candidate of the same name in the block's header expression, which
actually evaluates before the block's own bindings exist. Walk the
header expression under the incoming (unshadowed) scope, and only the
rest of the block under the block's introduced scope.
The fix snippet hard-coded a color/type example while findings name arbitrary bindings, risking an agent applying the wrong transformation. Drop snippet/lang and keep only the description.
1 participant
Summary
Second of three rules from the Svelte best-practices survey. The official guidance ("Treat props as though they will change") shows this exact do/don't pair:
The plain form renders correctly on first mount and silently stops tracking the parent afterwards — a stale-UI bug nothing in the compiler or svelte-check catches.
correctness/stale-prop-derivation(warning) flags it.How it works — conservative by design
All four must hold: the initializer references a
$props()prop in an eager position (references inside functions/arrows/getters stay reactive — verified against compiled runes output — and don't count); the initializer is call-free (no calls/new/await, which structurally exempts$state(initial)capture,$derived, and service construction); the binding is never written or escaped (reusing the unmutated-state machinery, incl.bind:); and it is rendered in the template (eager positions again — inline-handler bodies don't count,{#snippet}bodies do), shadow-aware.Along the way,
scopeIntroducedNameslearned{#each}indexes,{#snippet}parameters, and{#await}bindings (additive — existing consumers only get more conservative, which fixed two latent shadowing misattributions in unmutated-state/prop-mutation probes), andcorrectness/unmutated-state's recommendation now points at$derivedfor prop-computed state instead of steering users into this rule's anti-pattern.Review process
The design spec itself went through an adversarial review before implementation (it caught that closures/getters compile to reactive call-time reads, corrected two reuse claims against the real code, and added the
$bindablecoverage). Then 3 tasks via subagent-driven development with per-task reviews, and a final whole-branch review running 39 empirical probes against the built dist over realistic component shapes (uncontrolled inputs,+page.sveltedata, context/dispatcher patterns, whole-object props, spread escapes, TS destructures, directive usage, suppression) — zero false positives, READY TO MERGE.Verification
pnpm build/pnpm typecheck/pnpm lintpass (2 pre-existing warnings inmeta-object.test.tsonly)pnpm test: core 621, cli 694, vite 162, action 15, mcp 21 — all greendocs-linksgate green; changeset (minor × core/cli/vite/mcp);packages/action/distrebuilt via full workspace build🤖 Generated with Claude Code
Summary by CodeRabbit
correctness/stale-prop-derivationrule to flag prop-derived top-level values that become stale.$derived.correctness/stale-prop-derivation, including detection criteria, fixes, limitations, and configuration.