Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of `@observable` fields of `Store`s; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

fzyzcjy · 2022-07-12T13:02:07Z

Close #857
Close #858
Close #859
Close #855
Close #864

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
Run the set:versions command using npm or yarn. You can find this command in the package.json file in the root directory
Include the necessary reviewers for the PR
Update the docs if there are any API changes or additions to functionality

…stem for performance

netlify · 2022-07-12T13:02:14Z

✅ Deploy Preview for mobx canceled.

Name	Link
🔨 Latest commit	`1803928`
🔍 Latest deploy log	https://app.netlify.com/sites/mobx/deploys/6319c5e7db1fa0000e17006a

fzyzcjy · 2022-07-12T13:03:12Z

p.s. I may not merge this before PRs like #842 is merged, since otherwise my master branch will be destoryed

codecov · 2022-07-12T13:03:22Z

Codecov Report

Merging #844 (74ea2e5) into master (13b7e6d) will decrease coverage by 0.15%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##           master     #844      +/-   ##
==========================================
- Coverage   98.96%   98.81%   -0.15%     
==========================================
  Files          55       55              
  Lines        1924     1945      +21     
==========================================
+ Hits         1904     1922      +18     
- Misses         20       23       +3

Flag	Coverage Δ
flutter_mobx	`100.00% <ø> (ø)`
mobx	`98.33% <86.36%> (-0.20%)`	⬇️
mobx_codegen	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
mobx/lib/src/core/atom.dart	`93.87% <0.00%> (-6.13%)`	⬇️
...rc/api/observable_collections/observable_list.dart	`100.00% <100.00%> (ø)`
...src/api/observable_collections/observable_map.dart	`100.00% <100.00%> (ø)`
...src/api/observable_collections/observable_set.dart	`100.00% <100.00%> (ø)`
mobx/lib/src/core/action.dart	`100.00% <100.00%> (ø)`
mobx/lib/src/core/atom_extensions.dart	`100.00% <100.00%> (ø)`
mobx/lib/src/core/computed.dart	`100.00% <100.00%> (ø)`
mobx/lib/src/core/context.dart	`97.18% <100.00%> (+0.01%)`	⬆️
mobx/lib/src/core/observable.dart	`100.00% <100.00%> (ø)`
mobx/lib/src/core/reaction.dart	`98.57% <100.00%> (+0.04%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13b7e6d...74ea2e5. Read the comment docs.

amondnet

Please add a changelog and version bump for the mobx package.
Need some more unit tests in places where Codecov has flagged.

    test('bypass observable system', () {
      final list = ObservableList<int>();
      var count = -1;

      expect(list.nonObservableInner.length, 0);

      final d = autorun((_) {
        count = list.nonObservableInner.length;
      });

      expect(count, equals(0));

      list.add(20);
      expect(count, equals(0));
      d();
    });

…n be properly disposed mobxjs#857

fzyzcjy · 2022-09-04T08:42:58Z

Add onDispose callback to Computed, such that old values can be properly disposed

feature implemented, test added, and passed

…erformance"

fzyzcjy · 2022-09-04T08:46:38Z

@amondnet tests added

fzyzcjy · 2022-09-05T13:56:01Z

test reproduced #855

…ds of `Store`s mobxjs#855

merge from upstream

fzyzcjy · 2022-09-06T00:34:56Z

Revert "dispose"-related features according to #860

This reverts commit 63d57ef.

This reverts commit 20fe5d3.

This reverts commit 263155b. # Conflicts: # mobx/test/computed_test.dart

…alues can be properly disposed mobxjs#857" This reverts commit bbd9020.

… error mobxjs#864

…reaction it is mobxjs#864

amondnet

LGTM

ghenry · 2022-12-19T11:40:46Z

Hi all,

If this is in a "LGTM" state, is it just awaiting for @pavanpodila to approve or can someone else help with the load?

Thanks.

amondnet · 2022-12-19T16:28:11Z

@fzyzcjy @ghenry
This PR has solved many issues( #857, #858, #859, #855, #864). If you bump the version and write a chagelog, I'll merge it.

amondnet · 2023-01-27T17:12:28Z

Hi, @fzyzcjy
I add the missing tests, bump the version and write the changelog. #894

* add `nonObservableInner`, such that users can bypass observability system for performance * chore: format code * feat: Add `onDispose` callback to `Computed`, such that old values can be properly disposed #857 * test: add tests to `disposeValue` #857 * test: add tests for "Allow users to bypass observability system for performance" * feat: add `Computed.dispose` #859 * test: add tests for Computed.dispose * test: reproduce #855 * fix: Avoid unnecessary observable notifications of `@observable` fields of `Store`s #855 * Revert "test: add tests for Computed.dispose" This reverts commit 63d57ef. * Revert "feat: add `Computed.dispose` #859" This reverts commit 20fe5d3. * Revert "test: add tests to `disposeValue` #857" This reverts commit 263155b. # Conflicts: # mobx/test/computed_test.dart * Revert "feat: Add `onDispose` callback to `Computed`, such that old values can be properly disposed #857" This reverts commit bbd9020. * format code and minor change to code * add Observable.nonObservableValue * fix: Reaction lacks toString, so cannot see which reaction causes the error #864 * feat: Add StackTrace to reactions in debug mode to easily spot which reaction it is #864 * fix linter errors * add toString and debugCreationStack * test: add atom_test --------- Co-authored-by: fzyzcjy <ch271828n@outlook.com> Co-authored-by: fzyzcjy <5236035+fzyzcjy@users.noreply.github.com>

fzyzcjy added 2 commits July 12, 2022 20:59

add nonObservableInner, such that users can bypass observability sy…

880ee28

…stem for performance

Merge remote-tracking branch 'origin/master'

5a58d06

fzyzcjy requested a review from amondnet July 30, 2022 08:46

amondnet requested changes Aug 2, 2022

View reviewed changes

fzyzcjy force-pushed the master branch from 0d1f5a1 to 5a58d06 Compare September 4, 2022 06:27

fzyzcjy added 2 commits September 4, 2022 16:30

chore: format code

e1df03a

feat: Add onDispose callback to Computed, such that old values ca…

bbd9020

…n be properly disposed mobxjs#857

fzyzcjy changed the title ~~Allow users to bypass observability system for performance~~ Allow users to bypass observability system for performance; Add onDispose callback to Computed, such that old values can be properly disposed Sep 4, 2022

test: add tests to disposeValue mobxjs#857

263155b

test: add tests for "Allow users to bypass observability system for p…

396cd93

…erformance"

fzyzcjy mentioned this pull request Sep 4, 2022

Allow users to bypass observability system for performance #858

Closed

fzyzcjy added 2 commits September 4, 2022 16:53

feat: add Computed.dispose mobxjs#859

20fe5d3

test: add tests for Computed.dispose

63d57ef

amondnet self-requested a review September 5, 2022 02:28

test: reproduce mobxjs#855

ada24f7

fzyzcjy and others added 2 commits September 5, 2022 21:57

fix: Avoid unnecessary observable notifications of @observable fiel…

3b93039

…ds of `Store`s mobxjs#855

Merge pull request #4 from mobxjs/master

919f203

merge from upstream

fzyzcjy mentioned this pull request Sep 6, 2022

Add @computedDisposable, which automatically disposes the (heavy) computed data, such as an ui.Image or other heavyweight resources #860

Open

Revert "test: add tests for Computed.dispose"

305bd4e

This reverts commit 63d57ef.

fzyzcjy added 3 commits September 6, 2022 08:36

Revert "feat: add Computed.dispose mobxjs#859"

3912572

This reverts commit 20fe5d3.

Revert "test: add tests to disposeValue mobxjs#857"

290f8c3

This reverts commit 263155b. # Conflicts: # mobx/test/computed_test.dart

Revert "feat: Add onDispose callback to Computed, such that old v…

424ec02

…alues can be properly disposed mobxjs#857" This reverts commit bbd9020.

fzyzcjy added 2 commits September 6, 2022 13:58

format code and minor change to code

fcc222b

add Observable.nonObservableValue

637d12b

fix: Reaction lacks toString, so cannot see which reaction causes the…

e852b9b

… error mobxjs#864

fzyzcjy added 3 commits September 8, 2022 14:11

feat: Add StackTrace to reactions in debug mode to easily spot which …

75260bf

…reaction it is mobxjs#864

fix linter errors

fecb442

add toString and debugCreationStack

1803928

amondnet approved these changes Oct 12, 2022

View reviewed changes

amondnet requested a review from pavanpodila October 12, 2022 03:42

amondnet added 2 commits December 20, 2022 01:28

Merge branch 'master' into master

015d35f

Merge branch 'master' into master

74ea2e5

amondnet mentioned this pull request Jan 27, 2023

gh-844 #894

Merged

7 tasks

amondnet closed this Jan 30, 2023

amondnet added a commit to amondnet/mobx.dart that referenced this pull request Feb 14, 2023

Merge branch 'master' into mobxjsgh-844

e64e87d

amondnet mentioned this pull request Apr 12, 2023

feat: Adds custom equals for creating observables. #907

Merged

7 tasks

amondnet mentioned this pull request Nov 2, 2023

Avoid unnecessary observable notifications of Iterable or Map fields #951

Merged

7 tasks

amondnet mentioned this pull request Dec 4, 2023

Avoid unnecessary observable notifications of Iterable or Map fields #964

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of `@observable` fields of `Store`s; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of `@observable` fields of `Store`s; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

fzyzcjy commented Jul 12, 2022 •

edited

Loading

netlify bot commented Jul 12, 2022 •

edited

Loading

fzyzcjy commented Jul 12, 2022

codecov bot commented Jul 12, 2022 •

edited

Loading

amondnet left a comment

fzyzcjy commented Sep 4, 2022

fzyzcjy commented Sep 4, 2022

fzyzcjy commented Sep 5, 2022

fzyzcjy commented Sep 6, 2022

amondnet left a comment

ghenry commented Dec 19, 2022

amondnet commented Dec 19, 2022

amondnet commented Jan 27, 2023

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of @observable fields of Stores; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of @observable fields of Stores; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

Conversation

fzyzcjy commented Jul 12, 2022 • edited Loading

Pull Request Checklist

netlify bot commented Jul 12, 2022 • edited Loading

✅ Deploy Preview for mobx canceled.

fzyzcjy commented Jul 12, 2022

codecov bot commented Jul 12, 2022 • edited Loading

Codecov Report

amondnet left a comment

Choose a reason for hiding this comment

fzyzcjy commented Sep 4, 2022

fzyzcjy commented Sep 4, 2022

fzyzcjy commented Sep 5, 2022

fzyzcjy commented Sep 6, 2022

amondnet left a comment

Choose a reason for hiding this comment

ghenry commented Dec 19, 2022

amondnet commented Dec 19, 2022

amondnet commented Jan 27, 2023

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of `@observable` fields of `Store`s; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

Allow users to bypass observability system for performance; Avoid unnecessary observable notifications of `@observable` fields of `Store`s; Fix Reaction lacks toString, so cannot see which reaction causes the error; Add StackTrace to reactions in debug mode to easily spot which reaction it is #844

fzyzcjy commented Jul 12, 2022 •

edited

Loading

netlify bot commented Jul 12, 2022 •

edited

Loading

codecov bot commented Jul 12, 2022 •

edited

Loading