Repository navigation
fix(core): count named props beside a rest element in $props() - #404
Conversation
countProps() treated any $props() destructure containing a rest element
as uncountable, returning 0 even for `let { a, b, c, ...rest } = $props()`.
architecture/prop-count treats propCount === 0 as not analyzable, so any
component with a rest element beside named props was invisible to the
rule regardless of how many named props it had (huntabyte/shadcn-svelte:
488 of 637 $props() components affected).
Count the named Property entries and drop the rest-element bail. The
named count is a lower bound, and the rule's predicate is strictly
propCount > max, so a lower bound can only add missed findings, never a
false positive. A bare `...rest` with no named props, a non-destructured
$props(), and multiple $props() calls all stay 0 as before.
Re-measured the 10-repo corpus from the 2026-07-25 threshold
recalibration with the fix applied: per-repo p90 median moves from 6 to
6.5. MAX_PROPS stays 6.
Fixes #381
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe prop counter now counts named props in ChangesProp count behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.changeset/count-props-beside-rest.md:
- Line 7: Update the threshold statements in both prop-count rule pages to
report the remeasured 6.5 median while explicitly preserving MAX_PROPS at 6 as
the chosen threshold, and revise the changeset wording if needed so all three
files consistently describe the same measurement and decision.
- Line 5: Update the release note to describe the threshold using the configured
max option, stating that findings occur when propCount exceeds max and that max
defaults to 6, rather than presenting 6 as the universal limit.
🪄 Autofix
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: f554acdf-4db2-4847-aa1e-f9bd15a22484
📒 Files selected for processing (6)
.changeset/count-props-beside-rest.mddocs/src/content/docs/ja/rules/architecture/prop-count.mddocs/src/content/docs/rules/architecture/prop-count.mdpackages/core/src/component-parse.tspackages/core/test/architecture-rules.test.tspackages/core/test/component-parse.test.ts
|
|
||
| `architecture/prop-count` now counts named props destructured alongside a rest element (`let { a, b, ...rest } = $props()`) instead of treating the whole destructure as uncountable and staying silent. The named count is a lower bound on the true prop count, and the rule only flags `propCount > 6`, so this can only surface findings on previously invisible components — never a false positive. A bare rest element with no named props (`let { ...rest } = $props()`) and a non-destructured `$props()` are still not counted. | ||
|
|
||
| Re-measured the per-repo p90 median across the same 10-repo corpus from the 2026-07-25 threshold recalibration with the fix applied: 6.5 (was 6). `MAX_PROPS` stays 6. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve the threshold-measurement conflict.
This line reports a change from 6 to 6.5 after the July 25, 2026 remeasurement. docs/src/content/docs/rules/architecture/prop-count.md Line 12 and docs/src/content/docs/ja/rules/architecture/prop-count.md Line 12 still say that 6 is the median and that the survey did not move it.
Align all three files. If 6.5 is the measured median but MAX_PROPS remains 6, state that threshold decision explicitly in both rule pages.
🤖 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 @.changeset/count-props-beside-rest.md at line 7, Update the threshold
statements in both prop-count rule pages to report the remeasured 6.5 median
while explicitly preserving MAX_PROPS at 6 as the chosen threshold, and revise
the changeset wording if needed so all three files consistently describe the
same measurement and decision.
…easurement CodeRabbit review on PR #404: the rule doc pages still stated only the original 6-is-stable measurement while the changeset already recorded the 6.5 re-measurement, and the changeset described the rule as always flagging propCount > 6 when max is configurable. Update both rule doc pages (en/ja) to state the re-measured 6.5 without presenting it as the new threshold, and reword the changeset to reference the max option instead of a hardcoded 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed in e5a880a — both doc pages now record the re-measured 6.5 alongside the deliberately-unchanged default of 6, and the changeset describes the threshold via the configurable |
1 participant
Fixes #381.
Summary
countPropstreated any$props()destructure containing a rest element as uncountable, andarchitecture/prop-counttreatspropCount === 0as "not analyzable" — solet { a, …, ...rest } = $props()with any number of named props was invisible to the rule. Attribute-forwarding codebases are where the rule was most blind (huntabyte/shadcn-svelte: 77% of$props()components uncountable, per the issue).Per the issue's reasoning: the named count is a lower bound and the predicate is strictly
propCount > max, so counting named props beside a rest element can never false-positive — it only removes silence. A barelet { ...rest } = $props(), a non-destructured$props(), and multiple$props()calls stay 0 as before.Changes
countProps: drop the rest-element bail; the constraint comment now states the lower-bound contract.parseComponentFacts(6 named + rest passes, 7 flagged).@svelte-vitals/coreminor (new findings on previously silent components).Corpus re-measurement — threshold decision needed
Re-ran the 2026-07-25 recalibration measurement (same 10 repos, fresh
--depth 1clones, built core) with the fix applied. 917 components became newly countable:Pre-fix, the two corroborating statistics agreed at 6; post-fix they read 6.5 / 7. The shift is structural, not noise: rest-heavy component libraries moved up (bits-ui 9→10, threlte 7→9, xyflow 12→14) while forwarding-wrapper repos stayed flat or dropped. Per-repo p90s: 7, 10, 4, 6, 4, 1, 4, 9, 11, 14.
This PR ships with
MAX_PROPS = 6unchanged — the changeset records the re-measured median. Two defensible calls, maintainer's pick:Verification
pnpm -r typecheck/pnpm test(core 1306, cli 846, vite 209) /pnpm lintall green. Measurement script and clones were scratch-only, not committed.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
$props()expressions.Documentation
Tests