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.39k stars 311 forks source link

Feature Request: Make the entire store readonly #678

Open AlaaEldeenYsr opened 3 years ago

AlaaEldeenYsr commented 3 years ago

Our project structure depends on immutable state management. All the mobx stores in the project should be immutable, All the observables in the mobx stores should be mutated only from inside the stores, In that case I give readonly annotation for every single observable, I think it would be more much easy if we give only one readonly annotation for the store to make all the observables inside immutable without giving the annotation for every single observable.

Ascenio commented 2 years ago

Sounds nice, but there's a catch. What if the class has private dependencies which it doesn't want to expose? There should be a way to avoid generating getters for them.

benfgit commented 2 years ago

One approach could be, if a public variable is annotated with @observable in a @readonly store, then it will be exposed to outside. If the variable is private, it will be available for in-store use only. When you add this to the current @readonly solution, it might be confusing. Developers should know about what happens when:

When Mobx stores considered as state objects for Flutter apps, observable's should not be mutated directly. Default behaviour should be read-only and mutations should be perfromed by action's.

When stores are read-only by default (and @readonly is deprecated), the total cases will be reduced to a fair amount. Only exception will be to make an observable read-write outside of the store. That could be handled by an exceptional annotation such as @observable(readonly: false). When this applied, 3 possibilities occurs:

Current codegen supports mutability and treats immutability as exception. It needs to be reversed and immutability should be imposed by codegen, docs and examples.

Ascenio commented 2 years ago

I do think readonly should be the default behavior, but that would break so much code. That would be a breaking change similar to bloc's latest changes. In similar way, we could setup a issue for discussion and see if people agree with the changes before anything else.

If we don't come to do that, I like the idea of using @readonly in the store and annotate public fields with @observer, which in most cases should be no or few fields. What do you think @pavanpodila?

benfgit commented 2 years ago

I agree. We should discuss it further and it will be a breaking change.

@observable should be equivalent to Stream<T>. It should be open for subscription but closed for mutation. Right now it's more likely equivalent of StreamController<T>.