Skip to content

Conversation

@nightscape
Copy link

Generated Dart structs with collection fields (Map, List, Set) were using identity comparison (==) instead of deep equality. This caused equality checks to fail even when collection contents were identical.

Fix:

  • Use DeepCollectionEquality for collection fields in operator== and hashCode
  • Export DeepCollectionEquality from flutter_rust_bridge_for_generated_common.dart
  • Handle Optional types wrapping collections

Changes

Fixes #the-issue-number-here

Procedure and Checklist

In order to quickly iterate and avoid slowing down development pace by making CI pass, only the following simplified steps are needed, and I (fzyzcjy) will handle the rest of CI / moving the tests currently (will have more automation in the future).

  • Implement the feature / fix the bug.
  • Add tests in frb_example/dart_minimal.
  • Make dart_minimal's CI green.

Utility commands

  • Run codegen: cargo run --manifest-path /path/to/flutter_rust_bridge/frb_codegen/Cargo.toml -- generate
  • Run tests: ./frb_internal test-dart-native --package frb_example/dart_minimal

Generated Dart structs with collection fields (Map, List, Set) were using
identity comparison (==) instead of deep equality. This caused equality
checks to fail even when collection contents were identical.

Fix:
- Use DeepCollectionEquality for collection fields in operator== and hashCode
- Export DeepCollectionEquality from flutter_rust_bridge_for_generated_common.dart
- Handle Optional types wrapping collections
Copilot AI review requested due to automatic review settings December 31, 2025 01:04
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes a critical bug in generated Dart struct equality checking where collection fields (Map, List, Set) were using identity comparison (==) instead of deep equality comparison. This caused equality checks to fail even when collection contents were identical.

Key Changes:

  • Modified code generation logic to use DeepCollectionEquality for collection fields in operator== and hashCode
  • Added recursive handling for Optional types wrapping collections
  • Exported DeepCollectionEquality from the common library for generated code

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
frb_codegen/src/library/codegen/generator/api_dart/spec_generator/class/ty/structure_non_freezed.rs Core logic change: Added needs_deep_equality() function to detect collection types and generate appropriate equality/hash code using DeepCollectionEquality
frb_dart/lib/flutter_rust_bridge_for_generated_common.dart Exported DeepCollectionEquality from collection package for use in generated code
frb_example/pure_dart/test/api/mirror_test.dart Added tests verifying Map and List equality works correctly for structs with identical contents
frb_example/pure_dart/lib/src/rust/api/*.dart Generated Dart code updated to use DeepCollectionEquality for List/Map/Set fields in equality and hashCode methods
frb_example/pure_dart/lib/src/rust/api/enumeration.dart Code formatting changes (unrelated to equality fix)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

delegate,
MirTypeDelegate::Map(_) | MirTypeDelegate::Set(_)
),
MirType::Optional(opt) => needs_deep_equality(&opt.inner),
Copy link

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The needs_deep_equality function does not handle MirType::Boxed types that wrap collections (List, Map, or Set). If a field has type Box<List<T>>, Box<Map<K,V>>, or Box<Set<T>>, the function will return false and use identity comparison instead of deep equality.

Consider adding a case for MirType::Boxed that recursively checks the inner type, similar to how MirType::Optional is handled.

Suggested change
MirType::Optional(opt) => needs_deep_equality(&opt.inner),
MirType::Optional(opt) => needs_deep_equality(&opt.inner),
MirType::Boxed(_) => true,

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

❌ Patch coverage is 85.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.19%. Comparing base (67bfaec) to head (f2a4601).

Files with missing lines Patch % Lines
...t/spec_generator/class/ty/structure_non_freezed.rs 85.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2956       +/-   ##
===========================================
- Coverage   98.57%   85.19%   -13.39%     
===========================================
  Files         464      464               
  Lines       19202    18976      -226     
===========================================
- Hits        18928    16166     -2762     
- Misses        274     2810     +2536     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Dec 31, 2025

Great job! However, this seems to be a breaking change, maybe we should add a flag to allow users to enable/disable it. In addition, it seems that users can use freezed to generate classes with deep equality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants