refactor(hir): split widget_decl.rs under the 2000-line file-size cap#6531
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe changes add reactive lowering for state-dependent opacity and position animations, expose it through ChangesWidget lowering updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LoweringContext
participant try_desugar_reactive_animate
participant animateOpacityOrPosition
participant stateOnChange
LoweringContext->>try_desugar_reactive_animate: lower animation call
try_desugar_reactive_animate->>animateOpacityOrPosition: emit initial animation
try_desugar_reactive_animate->>stateOnChange: register State value subscribers
stateOnChange->>animateOpacityOrPosition: re-lower and re-invoke on change
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🤖 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 `@crates/perry-hir/src/lower/widget_decl/reactive_animate.rs`:
- Around line 21-86: The collect_state_value_reads function documents support
for object literals and assignment expressions but currently skips both. Add
recursive traversal for ast::Expr::Object property value expressions and
ast::Expr::Assign assignment operands, preserving existing deduplication and
handling nested expressions such as object member reads; add regression coverage
confirming stateOnChange subscribers are generated for these cases.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e8024c25-2570-4c12-aaa3-e68ff8ebd848
📒 Files selected for processing (3)
crates/perry-hir/src/lower/widget_decl.rscrates/perry-hir/src/lower/widget_decl/reactive_animate.rscrates/perry-hir/src/lower/widget_decl/tests.rs
Comment on lines
+21
to
+86
| /// Covers the expression shapes most commonly found in animation arguments: | ||
| /// ternaries, binary/logical ops, parens, template literals, unary, | ||
| /// assignment RHS, call args, array/object literals, and member reads. The | ||
| /// catch-all silently skips unhandled shapes — worst case, a state read | ||
| /// inside an exotic expression just won't trigger reactivity (same | ||
| /// conservative failure mode as #104's template walker). | ||
| fn collect_state_value_reads(ctx: &LoweringContext, expr: &ast::Expr, out: &mut Vec<String>) { | ||
| match expr { | ||
| ast::Expr::Member(member) => { | ||
| // `<ident>.value` where ident is a registered State. | ||
| if let ast::MemberProp::Ident(prop) = &member.prop { | ||
| if prop.sym.as_ref() == "value" { | ||
| if let ast::Expr::Ident(obj) = member.obj.as_ref() { | ||
| let name = obj.sym.to_string(); | ||
| if matches!( | ||
| ctx.lookup_native_instance(&name), | ||
| Some(("perry/ui", "State")) | ||
| ) && !out.contains(&name) | ||
| { | ||
| out.push(name); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| collect_state_value_reads(ctx, member.obj.as_ref(), out); | ||
| } | ||
| ast::Expr::Paren(p) => collect_state_value_reads(ctx, &p.expr, out), | ||
| ast::Expr::Cond(c) => { | ||
| collect_state_value_reads(ctx, &c.test, out); | ||
| collect_state_value_reads(ctx, &c.cons, out); | ||
| collect_state_value_reads(ctx, &c.alt, out); | ||
| } | ||
| ast::Expr::Bin(b) => { | ||
| collect_state_value_reads(ctx, &b.left, out); | ||
| collect_state_value_reads(ctx, &b.right, out); | ||
| } | ||
| ast::Expr::Unary(u) => collect_state_value_reads(ctx, &u.arg, out), | ||
| ast::Expr::Tpl(t) => { | ||
| for e in &t.exprs { | ||
| collect_state_value_reads(ctx, e, out); | ||
| } | ||
| } | ||
| ast::Expr::Call(c) => { | ||
| if let ast::Callee::Expr(ce) = &c.callee { | ||
| collect_state_value_reads(ctx, ce, out); | ||
| } | ||
| for a in &c.args { | ||
| collect_state_value_reads(ctx, &a.expr, out); | ||
| } | ||
| } | ||
| ast::Expr::Array(a) => { | ||
| for el in a.elems.iter().flatten() { | ||
| collect_state_value_reads(ctx, &el.expr, out); | ||
| } | ||
| } | ||
| ast::Expr::Seq(s) => { | ||
| for e in &s.exprs { | ||
| collect_state_value_reads(ctx, e, out); | ||
| } | ||
| } | ||
| ast::Expr::TsNonNull(n) => collect_state_value_reads(ctx, &n.expr, out), | ||
| ast::Expr::TsAs(a) => collect_state_value_reads(ctx, &a.expr, out), | ||
| ast::Expr::TsTypeAssertion(a) => collect_state_value_reads(ctx, &a.expr, out), | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Traverse the expression shapes promised by this collector.
Object and Assign are documented as supported but fall into _ => {}. Expressions such as ({ target: state.value }).target therefore receive only the initial animation and no stateOnChange subscriber. Add recursive handling for these shapes and regression tests, or use an exhaustive AST visitor.
🤖 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 `@crates/perry-hir/src/lower/widget_decl/reactive_animate.rs` around lines 21 -
86, The collect_state_value_reads function documents support for object literals
and assignment expressions but currently skips both. Add recursive traversal for
ast::Expr::Object property value expressions and ast::Expr::Assign assignment
operands, preserving existing deduplication and handling nested expressions such
as object member reads; add regression coverage confirming stateOnChange
subscribers are generated for these cases.
widget_decl.rs reached 2372 lines — #6513's reloadPolicy work pushed it over the CI `check_file_size.sh` cap, reddening the lint job's file-size step on every PR (previously masked behind the benchmark-freshness step). Pure mechanical split, no logic change: - widget_decl/reactive_animate.rs — collect_state_value_reads + try_desugar_reactive_animate (reactive-animation desugaring). try_desugar_reactive_animate is re-exported from widget_decl.rs as pub(crate) so lower/mod.rs's `pub(crate) use widget_decl::*` still resolves it for expr_call/prescans.rs. - widget_decl/tests.rs — the #[cfg(test)] module. widget_decl.rs drops to 1665 lines. perry-hir compiles, fmt --check is clean, the 18 moved tests pass, and check_file_size.sh is green.
proggeramlug
force-pushed
the
fix/split-widget-decl-lower
branch
from
July 17, 2026 13:32
9644471 to
0a48673
Compare
proggeramlug
pushed a commit
that referenced
this pull request
Jul 17, 2026
Version + CHANGELOG for the three fixes that green the repo-wide lint job: - #6528 (v0.5.1261): version-invariant Cargo.toml fingerprint in the public-baseline freshness gate. - #6531 (v0.5.1262): split widget_decl.rs under the 2000-line file-size cap. - #6394/#6526 (v0.5.1263): key the object cache on all codegen-affecting env vars. Bumps [workspace.package] 0.5.1260 -> 0.5.1263. The version bump no longer invalidates the benchmark freshness gate (fixed by #6528).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
crates/perry-hir/src/lower/widget_decl.rsreached 2372 lines, over the CIcheck_file_size.shcap (2000). #6513 (reloadPolicythrough all three widget backends) added +378 lines and tipped it over. The lint job's "File size limit" step was skipped while the benchmark-freshness step failed first, so it slipped onto main; once freshness passes it surfaces and reddens lint on every PR.Fix
Pure mechanical split, no logic change — into the
foo.rs+foo/submodule layout the repo already uses (cf.object_cache.rs+object_cache/object_cache_tests.rs):widget_decl/reactive_animate.rs—collect_state_value_reads+try_desugar_reactive_animate(the reactive-animation desugaring).try_desugar_reactive_animateis re-exported fromwidget_decl.rsaspub(crate)solower/mod.rs'spub(crate) use widget_decl::*still resolves it forexpr_call/prescans.rs. (pub(super)couldn't be re-exported across the new module boundary — E0364.)widget_decl/tests.rs— the#[cfg(test)]module.widget_decl.rsdrops from 2372 → 1665 lines.Verification
cargo check -p perry-hircompiles.cargo fmt -p perry-hir -- --checkclean.cargo test -p perry-hir --lib widget_decl::tests)../scripts/check_file_size.sh→ "OK: no Rust source files exceed 2000 lines."Together with #6528 (benchmark-freshness gate), this greens the repo-wide lint job.
Summary by CodeRabbit
New Features
animateOpacity/animatePosition) that automatically update when referenced state values change.Tests