Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fzyzcjy opened this issue Sep 4, 2022 · 5 comments

Comments

@fzyzcjy
Copy link
Collaborator

fzyzcjy commented Sep 4, 2022

Example:

class A = _A with _$A;
class _A with Store {
  @computedDisposable
  MyHeavyObject get myObject => some_computation();
}

generates:

mixin _$AStore on _AStore, Store {
  Computed<bool>? _$myObjectComputed;

  @override
  bool get myObject =>
      (_$myObjectComputed ??= Computed(() => super.myObject,
              name: ...', disposeValue: (v) => v.dispose()) // NOTE 1
          .value;

  void dispose() {
    super.dispose();
    _$myObjectComputed.dispose(); // NOTE 2
  }
}

NOTE 1: Add this "disposeValue" automatically
NOTE 2: Add this "dispose" function

@fzyzcjy fzyzcjy changed the title Add @computedDisposable, which automatically disposes the (heavy) computed data Add @computedDisposable, which automatically disposes the (heavy) computed data, such as an ui.Image or other heavyweight resources Sep 4, 2022
@pavanpodila
Copy link
Member

I prefer not to mix lifecycle management of resources with core observables. Computed never holds a value, but only depends on other value holding observables. So fundamentally it's bringing irrelevant responsibility. Would not vote for this.

You are bringing application level concepts here. Don't think it's the right place. Can you see if you can solve at the application layer?

@fzyzcjy
Copy link
Collaborator Author

fzyzcjy commented Sep 4, 2022

@pavanpodila Thanks for the reply! I agree and will not write code for the @computedDisposable. However, IMHO #857 is still very much needed. (I guess your comment is only against @computedDisposable, not #857, but anyway I would write down some explanations)

The reason is that, sometimes the computed value is heavy. It can be a big controller that has a dispose method, a big object (memory-hungry) that needs to be disposed (freed) when no longer needed, etc. In such cases, it would be necessary to provide that disposeValue callback in #857.

Indeed, I have tried to implement it without modifying mobx source code, but all failed. (If you are interested I can post my non-working solution here).

@fzyzcjy
Copy link
Collaborator Author

fzyzcjy commented Sep 4, 2022

@pavanpodila I love mobx (and use it extensively!), but I see Riverpod has some features that are so great - this is the main reason I am adding this (and planning to add others) - to make mobx have fewer shortcomings compared with riverpod! (Surely mobx has a lot of other strengths :) )

@pavanpodila
Copy link
Member

How about using an Atom to track the usage and then dispose when there are no more observers?

@fzyzcjy
Copy link
Collaborator Author

fzyzcjy commented Sep 6, 2022

LGTM, I will revert it in #844

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

No branches or pull requests

2 participants