Skip to content

Conversation

@ben-milanko
Copy link

Describe the changes proposed in this Pull Request.

There is an existing bug where the useDeepEquality option is not being passed to the generator.

Based on MakeObservable the default option should be useDeepEquality: true.

After looking at the generated code, this is not being passed through.


Pull Request Checklist

  • If the changes are being made to code, ensure the version in pubspec.yaml is updated.
  • Increment the major/minor/patch/patch-count, depending on the complexity of change
  • Add the necessary unit tests to ensure the coverage does not drop
  • Update the Changelog to include all changes made in this PR, organized by version
  • Run the melos run set_version command from the root directory
  • Include the necessary reviewers for the PR
  • Update the docs if there are any API changes or additions to functionality

@ben-milanko ben-milanko changed the title useDeepEquality option is not being passed to the generated code useDeepEquality option is not being passed to the generated code Dec 30, 2025
@amondnet amondnet requested review from amondnet and Copilot December 30, 2025 08:34
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 PR fixes a bug where the useDeepEquality option from the @observable annotation was not being extracted and passed to the code generator, preventing users from controlling deep equality checks in their observable setters.

Key Changes:

  • Extracts the useDeepEquality parameter from @observable annotations in the code generation pipeline
  • Adds comprehensive test coverage for useDeepEquality with true, false, and null values
  • Updates version from 2.7.4 to 2.7.5

Reviewed changes

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

Show a summary per file
File Description
mobx_codegen/lib/src/store_class_visitor.dart Added _getUseDeepEquality() method to extract the useDeepEquality parameter from annotations and pass it to ObservableTemplate
mobx_codegen/test/store_class_visitor_test.dart Added three test cases to verify useDeepEquality is correctly included in generated code when set to true, false, or null
mobx_codegen/pubspec.yaml Updated version to 2.7.5 and normalized quote style for dependency version constraints
mobx_codegen/lib/version.dart Updated version constant to match pubspec.yaml
mobx_codegen/CHANGELOG.md Documented the bug fix for version 2.7.5

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

@amondnet amondnet self-assigned this Dec 30, 2025
Copy link
Collaborator

@amondnet amondnet left a comment

Choose a reason for hiding this comment

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

Thanks for this fix! The core implementation looks good and follows the existing patterns.

However, there are a couple of issues to address:

  1. Rebase needed: The PR appears to be based on an older branch. The pubspec.yaml has SDK >=3.0.0 but current main requires >=3.7.0. Please rebase on the latest main branch.

  2. Missing late field support: The useDeepEquality parameter is only added to regular field setters but not to late field setters in observable.dart (line 70). Could you also add it there for completeness?

Let me know if you need any help with these changes!

@ben-milanko ben-milanko requested a review from amondnet January 7, 2026 00:59
@ben-milanko
Copy link
Author

@amondnet thank you for the feedback. I've made the requested changes. Let me know if you have any more comments.

I've also got a PR I'd like to raise for useDeepEquality functionality on computeds.

@amondnet
Copy link
Collaborator

amondnet commented Jan 8, 2026

@ben-milanko Could you please fix the failing test? The test data needs to be modified. If it's difficult, please grant me permission to modify the PR.

@ben-milanko
Copy link
Author

Hi @amondnet I've merged the latest changes from main, and tried to update the test data to include useDeepEquality, but am running into some formatting issues.

In this example the expected is on the left and the actual on the right. The deep equality was fixed in this example.

image

The test in question is

    test('generates for a class mixing Store', () async {
      await compareFiles('./data/valid_input.dart', './data/valid_output.dart');
    });

I'm not sure if you might be using a different formatter to me?

I've added you with write permissions on my fork if you would like to take a look.

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