Repository navigation
fix(core): parse components with argument-less $state() instead of silently skipping them - #429
Conversation
…lently skipping them (#424) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 (4)
📝 WalkthroughWalkthroughThe parser now handles argument-less ChangesArgument-less state parsing
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 |
✅ Action performedReview finished.
|
Fixes #424.
Problem
let el = $state();— the idiomaticbind:thisdeclaration — crashed fact extraction: two scans incomponent-parse.ts(the$state.rawcandidates scan and the builtin-state candidates scan) callunwrapTs(init.arguments?.[0]), andunwrapTsdereferenced.typeon theundefinedargument. The resultingTypeErrorwas swallowed bycollectComponentFacts's deliberate never-throw fallback, so the whole file silently contributedemptyComponentFacts— invisible to every rule, with a clean Health 100/100 to show for it.Verified blast radius: only bare
$state()in.sveltecomponents.$state.raw()/$state.frozen()with no argument and argument-less$state()in.svelte.tsrunes modules never reached the crashing scans (pinned by a regression test so that stays true).Fix
unwrapTsnow toleratesundefinedvia an overload pair — one guard in the shared helper covers both current call sites and any future caller, while keeping the non-undefined callers' return type narrow. No call sites changed; the never-throw fallback incomponent-collect.tsis untouched (it remains the safety net for genuinely malformed files).Tests
$state()alongsidecount/doubled/$effect) yields normal facts —loc,effects,constableStates, plusrawableStates/nonreactiveBuiltinStatesto exercise both formerly-crashing scans.$state<HTMLDialogElement>()variant from the issue's trigger table..svelte.tsrunes module with argument-less$state()(already worked; pinned against regression).collectComponentFactsno longer falls back to empty facts for such a file (the user-visible symptom).Mutation-checked: with the
unwrapTsguard reverted, the three.sveltetests fail with the originalTypeError; the.svelte.tspin passes, as expected.Release note
Changeset (
@svelte-vitals/corepatch) states the gate movement explicitly: files previously skipped by this crash are now analyzed, so projects using this pattern may see new findings — including critical ones — and a previously green run can fail the default--fail-on criticalgate. That is the fix working.Follow-up (out of scope)
The issue's secondary point — nothing surfaces when a file is skipped by the never-throw fallback — is a real observability gap but a separate design decision (skipped-file counter / stderr note). Filing it separately keeps this a pure crash fix.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
$state()declarations so affected components are processed normally instead of being skipped.$state<T>()declarations.Tests