feat(state): harden viewmodel management#200
Conversation
📝 WalkthroughWalkthroughThis PR adds ChangesViewModel mode and selector/identity refactor
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Widget
participant AsyncViewModelBase
participant Repository
Widget->>AsyncViewModelBase: load()
AsyncViewModelBase->>AsyncViewModelBase: emit AsyncLoading
AsyncViewModelBase->>Repository: loadData()
Repository-->>AsyncViewModelBase: data or error
AsyncViewModelBase->>Widget: emit AsyncData/AsyncFailure
sequenceDiagram
participant Scope as *ViewModelScope
participant Instance as _*ViewModelInstance
participant Inherited as _*ViewModelInherited
participant Selector as *ViewModelSelector
Scope->>Scope: didChangeDependencies computes identity
Scope->>Scope: recreate owned ViewModel if identity changed
Scope->>Instance: wrap with viewModel
Instance->>Inherited: wrap with viewModel + state
Selector->>Inherited: watch and derive selected value
Inherited-->>Selector: notify on state or identity change
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
examples/shopping_app/lib/core/view_models/app_view_model.dart (1)
90-98: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winParallelize independent repository calls in
loadData().
getProductsPageandgetCategoriesare independent calls but are awaited sequentially, doubling the home-page load latency unnecessarily.⚡ Proposed fix using Future.wait
`@override` Future<HomePageData> loadData() async { - final products = await args.repository.getProductsPage(limit: 6); - final categories = await args.repository.getCategories(); + final results = await Future.wait([ + args.repository.getProductsPage(limit: 6), + args.repository.getCategories(), + ]); + final products = results[0] as List<Product>; + final categories = results[1] as List<String>; return HomePageData( featuredProducts: products, categories: categories, ); }🤖 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 `@examples/shopping_app/lib/core/view_models/app_view_model.dart` around lines 90 - 98, `AppViewModel.loadData()` is awaiting `getProductsPage` and `getCategories` one after the other even though they are independent. Update `loadData()` to start both repository calls together and await them in parallel using `Future.wait`, then build `HomePageData` from the two results. Use the existing `args.repository`, `getProductsPage`, and `getCategories` symbols to keep the change localized.crates/dust_state_plugin/src/plugin/emit/templates/scope_class.jinja (1)
69-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate identity-recreation logic across
didChangeDependenciesanddidUpdateWidget.The "compute identity, compare to
_identity, recreate owned view model" logic is repeated (with slightly different shapes) in both lifecycle methods. Extracting a shared private helper (e.g._syncOwnedViewModel({bool notify})) would reduce duplication and the risk of the two paths drifting (as already happened with thenotifydefault inconsistency).🤖 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/dust_state_plugin/src/plugin/emit/templates/scope_class.jinja` around lines 69 - 100, Duplicate the owned view model sync logic in didChangeDependencies and didUpdateWidget by extracting the shared “compute widget.identity, compare against _identity, and recreate owned model” flow into a private helper such as _syncOwnedViewModel. Use that helper from both lifecycle methods, preserving the existing external-value branch and allowing the caller to control notify so the two paths stay consistent and don’t drift again.crates/dust_state_plugin/src/plugin/emit/render.rs (1)
424-437: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a non-nullable
data_typetest forrender_async_builder.The only test covers an already-nullable
data_type("Profile?"). The non-nullable branch (previousData as {data_type}cast,data_type?type) inprevious_data_type/previous_data_argumentis untested, even though it's the more common case and exercises the cast-generation logic.♻️ Suggested additional test
#[test] fn async_builder_uses_nullable_previous_data_type_once() { let source = render_async_builder("ProfileViewModelBuilder", "ProfileViewModel", "Profile?"); assert!(source.contains("Profile? previousData")); assert!(source.contains("data(context, previousData)")); assert!(!source.contains("Profile??")); assert!(!source.contains("previousData as Profile?")); } + + #[test] + fn async_builder_casts_previous_data_for_non_nullable_type() { + let source = + render_async_builder("ProfileViewModelBuilder", "ProfileViewModel", "Profile"); + + assert!(source.contains("Profile? previousData")); + assert!(source.contains("data(context, previousData as Profile)")); + }🤖 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/dust_state_plugin/src/plugin/emit/render.rs` around lines 424 - 437, Add a new `render_async_builder` test in the existing `tests` module alongside `async_builder_uses_nullable_previous_data_type_once` that uses a non-nullable `data_type` (for example `Profile`) so the `previous_data_type` and `previous_data_argument` paths that generate `data_type?` and `previousData as {data_type}` are covered. Verify the rendered source contains the nullable parameter type and cast, and that it does not emit double-nullability or the nullable-only form; keep the test focused on `render_async_builder` output and the `ProfileViewModelBuilder` / `ProfileViewModel` symbols to locate the behavior.
🤖 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/dust_state_plugin/src/plugin/validate.rs`:
- Around line 45-53: The validation logic in validate.rs uses let-chains in the
mode_source and subscription checks, which are not supported by the declared
Rust version. Refactor the conditional logic in the relevant validation blocks
inside the validate function to avoid `if let ... && ...` chaining, using nested
conditionals or equivalent pre-1.88-compatible patterns instead. Keep the
existing diagnostics behavior unchanged while ensuring the code compiles on the
workspace’s MSRV.
---
Nitpick comments:
In `@crates/dust_state_plugin/src/plugin/emit/render.rs`:
- Around line 424-437: Add a new `render_async_builder` test in the existing
`tests` module alongside `async_builder_uses_nullable_previous_data_type_once`
that uses a non-nullable `data_type` (for example `Profile`) so the
`previous_data_type` and `previous_data_argument` paths that generate
`data_type?` and `previousData as {data_type}` are covered. Verify the rendered
source contains the nullable parameter type and cast, and that it does not emit
double-nullability or the nullable-only form; keep the test focused on
`render_async_builder` output and the `ProfileViewModelBuilder` /
`ProfileViewModel` symbols to locate the behavior.
In `@crates/dust_state_plugin/src/plugin/emit/templates/scope_class.jinja`:
- Around line 69-100: Duplicate the owned view model sync logic in
didChangeDependencies and didUpdateWidget by extracting the shared “compute
widget.identity, compare against _identity, and recreate owned model” flow into
a private helper such as _syncOwnedViewModel. Use that helper from both
lifecycle methods, preserving the existing external-value branch and allowing
the caller to control notify so the two paths stay consistent and don’t drift
again.
In `@examples/shopping_app/lib/core/view_models/app_view_model.dart`:
- Around line 90-98: `AppViewModel.loadData()` is awaiting `getProductsPage` and
`getCategories` one after the other even though they are independent. Update
`loadData()` to start both repository calls together and await them in parallel
using `Future.wait`, then build `HomePageData` from the two results. Use the
existing `args.repository`, `getProductsPage`, and `getCategories` symbols to
keep the change localized.
🪄 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
Run ID: d7a2185a-687a-4d1d-86da-5e3ce969ae74
📒 Files selected for processing (56)
crates/dust_driver/tests/driver_tests/pub_workspace.rscrates/dust_driver/tests/driver_tests/state_outputs.rscrates/dust_driver/tests/driver_tests/state_outputs/snapshots/async_profile_view_model.dart.snapshotcrates/dust_state_plugin/src/plugin/analysis.rscrates/dust_state_plugin/src/plugin/emit.rscrates/dust_state_plugin/src/plugin/emit/render.rscrates/dust_state_plugin/src/plugin/emit/state_fields.rscrates/dust_state_plugin/src/plugin/emit/templates/aspect_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/aspect_selector.jinjacrates/dust_state_plugin/src/plugin/emit/templates/async_builder_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/base_async_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/base_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/context_extension.jinjacrates/dust_state_plugin/src/plugin/emit/templates/inherited_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/instance_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/listener_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/proxy_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/proxy_field_getter.jinjacrates/dust_state_plugin/src/plugin/emit/templates/scope_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/selector_class.jinjacrates/dust_state_plugin/src/plugin/emit/templates/view_model_output.jinjacrates/dust_state_plugin/src/plugin/model.rscrates/dust_state_plugin/src/plugin/parse.rscrates/dust_state_plugin/src/plugin/validate.rscrates/dust_state_plugin/tests/state_plugin_tests/analysis.rscrates/dust_state_plugin/tests/state_plugin_tests/emission/base.rscrates/dust_state_plugin/tests/state_plugin_tests/emission/imports.rscrates/dust_state_plugin/tests/state_plugin_tests/emission/workspace.rscrates/dust_state_plugin/tests/state_plugin_tests/validation.rsdocs/README.mddocs/state-management-design.mddocs/usage/state.mdexamples/benchmark_project/lib/pages/benchmark_shell.dartexamples/benchmark_project/lib/state/benchmark_view_model.g.dartexamples/shopping_app/lib/core/view_models/app_view_model.dartexamples/shopping_app/lib/core/view_models/app_view_model.g.dartexamples/shopping_app/lib/features/auth/view_models/auth_view_model.g.dartexamples/shopping_app/lib/features/cart/view_models/cart_view_model.dartexamples/shopping_app/lib/features/cart/view_models/cart_view_model.g.dartexamples/shopping_app/lib/features/checkout/view_models/checkout_view_model.dartexamples/shopping_app/lib/features/checkout/view_models/checkout_view_model.g.dartexamples/shopping_app/lib/features/demo_cart/view_models/demo_cart_api_view_model.g.dartexamples/shopping_app/lib/features/orders/view_models/order_tracking_view_model.g.dartexamples/shopping_app/lib/features/orders/view_models/orders_view_model.g.dartexamples/shopping_app/lib/features/product_detail/view_models/product_detail_view_model.g.dartexamples/shopping_app/lib/features/products/view_models/products_view_model.g.dartexamples/shopping_app/lib/features/support/view_models/shopping_chat_view_model.g.dartexamples/shopping_app/lib/features/wishlist/view_models/wishlist_view_model.g.dartexamples/shopping_app/lib/main.dartexamples/shopping_app/test/app_view_model_test.dartexamples/shopping_app/test/state_selector_test.dartpackages/dust_flutter/lib/src/state/annotations.dartpackages/dust_flutter/lib/src/state/view_model.dartpackages/dust_flutter/lib/state.dartpackages/dust_flutter/test/async_state_test.dartpackages/dust_flutter/test/view_model_test.dart
💤 Files with no reviewable changes (5)
- crates/dust_state_plugin/src/plugin/emit/templates/view_model_output.jinja
- crates/dust_state_plugin/src/plugin/emit/templates/proxy_field_getter.jinja
- crates/dust_state_plugin/src/plugin/emit/templates/aspect_class.jinja
- crates/dust_state_plugin/src/plugin/emit/state_fields.rs
- crates/dust_state_plugin/src/plugin/emit/templates/aspect_selector.jinja
| if let Some(mode_source) = annotation.mode_source.as_deref() | ||
| && !is_valid_mode_source(mode_source) | ||
| { | ||
| diagnostics.push(Diagnostic::error(format!( | ||
| "view model `{}` mode `{mode_source}` must be `ViewModelMode.sync` or `ViewModelMode.async`", | ||
| class.name | ||
| ))); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: check declared Rust edition / MSRV for the dust_state_plugin crate and workspace.
fd Cargo.toml | xargs grep -n 'edition\|rust-version'
fd rust-toolchain.toml rust-toolchain | xargs cat 2>/dev/nullRepository: y3l1n4ung/dust
Length of output: 2833
🌐 Web query:
Rust let chains stabilized 1.88 2024 edition if let chains stable release notes
💡 Result:
Yes, "let chains" were stabilized in Rust 1.88.0, which was released on June 26, 2025 [1][2]. This feature is available exclusively in the Rust 2024 edition [1][3]. It allows the use of &&-chaining with let statements inside if and while expressions, enabling the intermixture of let statements with boolean expressions [4][3]. The patterns used within these let sub-expressions can be either refutable or irrefutable [4][3]. The feature was stabilized in this specific edition because it depends on changes to the if let temporary scope, which were implemented to ensure more consistent drop behavior [1]. If you are using Rust 1.88.0 or later and encounter an error indicating that the feature is still unstable, you must ensure your project is set to use the 2024 edition [5].
Citations:
- 1: https://blog.rust-lang.org/2025/06/26/Rust-1.88.0/
- 2: https://github.com/rust-lang/rust/releases/tag/1.88.0
- 3: https://chromium.googlesource.com/external/github.com/rust-lang/rust/+/refs/tags/1.88.0/RELEASES.md
- 4: Stabilize let chains in the 2024 edition rust-lang/rust#132833
- 5: Tracking issue for eRFC 2497, "if- and while-let-chains, take 2" rust-lang/rust#53667
🌐 Web query:
site:doc.rust-lang.org let chains stabilized stable rust 1.88 2024 edition
💡 Result:
The "let chains" feature is officially stabilized as part of the Rust 2024 Edition [1][2][3]. In Rust 2024, you can chain let expressions using the && operator within the condition operands of if and while statements [1][4]. This allows you to mix let expressions with other boolean expressions in a single condition, where each pattern binding remains in scope for subsequent condition operands and the block that follows [5][6]. Because this feature relies on changes to how if let rescoping works—which is specific to the 2024 Edition—it is edition-gated [1][4]. While the query mentioned Rust 1.88, note that the stabilization is tied to the 2024 Edition rather than a specific version number alone, and it represents a true extension of valid Rust syntax [1][3].
Citations:
- 1: https://doc.rust-lang.org/edition-guide/rust-2024/let-chains.html
- 2: https://doc.rust-lang.org/stable/releases.html
- 3: https://doc.rust-lang.org/stable/edition-guide/rust-2024/let-chains.html
- 4: https://doc.rust-lang.org/beta/edition-guide/rust-2024/let-chains.html
- 5: https://doc.rust-lang.org/beta/reference/expressions/if-expr.html
- 6: https://doc.rust-lang.org/1.88.0/reference/expressions/loop-expr.html
Avoid let-chains or bump the MSRV crates/dust_state_plugin/src/plugin/validate.rs:45-53,81-89 uses if ... && let ... chains, but the workspace declares rust-version = "1.85", so this won’t compile on the supported toolchain.
🤖 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/dust_state_plugin/src/plugin/validate.rs` around lines 45 - 53, The
validation logic in validate.rs uses let-chains in the mode_source and
subscription checks, which are not supported by the declared Rust version.
Refactor the conditional logic in the relevant validation blocks inside the
validate function to avoid `if let ... && ...` chaining, using nested
conditionals or equivalent pre-1.88-compatible patterns instead. Keep the
existing diagnostics behavior unchanged while ensuring the code compiles on the
workspace’s MSRV.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/shopping_app/lib/core/view_models/app_view_model.g.dart (1)
438-452: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSuppress stale init failures after ViewModel replacement.
Line 439 guards before
init()starts, but if the scope swaps/disposes thisviewModelwhileawait viewModel.init()is pending, the catch block still reports a stale failure. Recheckmountedand identity beforeFlutterError.reportError.Proposed fix
scheduleMicrotask(() async { if (!mounted || !identical(_viewModel, viewModel)) return; try { await viewModel.init(); } catch (error, stackTrace) { + if (!mounted || !identical(_viewModel, viewModel)) return; FlutterError.reportError( FlutterErrorDetails(🤖 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 `@examples/shopping_app/lib/core/view_models/app_view_model.g.dart` around lines 438 - 452, The scheduled init flow in AppViewModelScope still reports failures for a ViewModel that has been replaced or disposed while await viewModel.init() is pending. In the scheduleMicrotask callback, add the same mounted and identical(_viewModel, viewModel) guard again inside the catch block before calling FlutterError.reportError, so only the currently active ViewModel can surface init errors. Use the existing scheduleMicrotask, viewModel.init, and FlutterError.reportError locations to apply the stale-error suppression consistently.
🤖 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.
Outside diff comments:
In `@examples/shopping_app/lib/core/view_models/app_view_model.g.dart`:
- Around line 438-452: The scheduled init flow in AppViewModelScope still
reports failures for a ViewModel that has been replaced or disposed while await
viewModel.init() is pending. In the scheduleMicrotask callback, add the same
mounted and identical(_viewModel, viewModel) guard again inside the catch block
before calling FlutterError.reportError, so only the currently active ViewModel
can surface init errors. Use the existing scheduleMicrotask, viewModel.init, and
FlutterError.reportError locations to apply the stale-error suppression
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01ed308d-c081-459c-9001-a9517148bd00
📒 Files selected for processing (8)
crates/dust_driver/tests/driver_tests/state_outputs/snapshots/async_profile_view_model.dart.snapshotcrates/dust_state_plugin/src/plugin/emit/templates/async_builder_class.jinjadocs/state-management-design.mddocs/usage/state.mdexamples/shopping_app/lib/core/view_models/app_view_model.g.dartexamples/shopping_app/test/state_scope_lifecycle_test.dartpackages/dust_flutter/lib/src/state/view_model.dartpackages/dust_flutter/test/async_state_test.dart
✅ Files skipped from review due to trivial changes (3)
- docs/state-management-design.md
- crates/dust_driver/tests/driver_tests/state_outputs/snapshots/async_profile_view_model.dart.snapshot
- docs/usage/state.md
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/dust_state_plugin/src/plugin/emit/templates/async_builder_class.jinja
- packages/dust_flutter/test/async_state_test.dart
Problem
Dust state management was still too easy to misuse:
argsversusstateruleSolution
This PR hardens the state-management stack end to end:
Docs And Lint Rules Applied
missing-docs-in-private-itemspressure.Validation
cargo fmt --all -- --checkcargo clippy -p dust_state_plugin --all-targets -- -D warnings./scripts/rust/test.sh(574 passed,1 skipped)./scripts/dart/test.sh./scripts/flutter/test.sh./scripts/dart/test.sh --examples./scripts/flutter/test.sh --examplestarget/debug/dust check --root examples/shopping_app --fail-fasttarget/debug/dust check --root examples/shopping_app --db --fail-fastgit diff --check origin/main...HEADRisk
Medium. This changes generated state-management API output and Flutter runtime behavior, but the branch includes exact snapshots, plugin tests, runtime tests, and shopping-app widget regressions.
Follow-up
State-management milestone follow-up remains tracked in the linked issues until review confirms what can close.
Refs #186
Refs #187
Refs #188
Refs #189
Refs #190
Refs #191
Refs #192
Refs #193
Refs #194
Refs #195
Summary by CodeRabbit
New Features
ViewModelModewith async ViewModels, including async lifecycle state andinvalidateSelf().Bug Fixes
Documentation
Tests