mobxjs / mobx.dart

MobX for the Dart language. Hassle-free, reactive state-management for your Dart and Flutter apps.
https://mobx.netlify.app
MIT License
2.4k stars 310 forks source link

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

Open fzyzcjy opened 2 years ago

fzyzcjy commented 2 years ago

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

pavanpodila commented 2 years ago

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 commented 2 years ago

@pavanpodila Thanks for the reply! I agree and will not write code for the @computedDisposable. However, IMHO https://github.com/mobxjs/mobx.dart/issues/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 commented 2 years ago

@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 commented 2 years ago

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

fzyzcjy commented 2 years ago

LGTM, I will revert it in #844