-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
(Philosophical question) Make dependency more explicit? #937
Comments
This reminds me of Cerebral.js, they have a Single State with arbitrary depth and you specify which parts of the state you want to listen to: import React from 'react'
import { state, sequences } from 'cerebral'
import { connect } from '@cerebral/react'
export default connect(
{
foo: state`foo`,
onClick: sequences`onClick`
},
class MyComponent extends React.Component {
render() {
return <div onClick={() => this.props.onClick()}>{this.props.foo}</div>
}
}
) Its very nice, very intuitive and specific IMO. But maybe make it optional? In small and medium projects just using the builder without worrying what store do you need is very nice. I have a store of stores with something resembling a Single State so I just do: Observer(
builder: (context) {
if (state.wallet.withdraw.isFetchingWithdrawalInfo) {
return const CustomProgressIndicator();
}
// if (state.otherStore.someObservable) ...
return Text('${state.wallet.withdraw.userBalanceShib}');
},
) I'm keeping it as simple as possible, so I would argue that if this change is introduced would be nice to make it optional. It adds too much code to my simple app. :) |
@fzyzcjy what about creating a linter rule to avoid using observables without Observer( inside a widget 🤔 |
Definitely optional - I am not going to introduce any breaking changes! And this is just a brainstorm - far from real implementation.
That will be quite hard indeed :/ It is possible to call a normal function, which calls a normal function, etc, and finally deep nested it fetches an observable. |
@fzyzcjy could this be related to that?
https://pub.dev/documentation/mobx/latest/mobx/ReactiveReadPolicy.html |
Looks like the reactive context has policies for when you can read/write observables, and looks like its possible to tell mobx to throw an error if an observable was read outside a reactive context, like an action or a reaction. |
@fzyzcjy just realized you are contributor lol, you already know all of these things. 🙈 |
Yes, I know that. That one is great, but it is caught at runtime (instead of compile time), and there is no syntax hint to show explicit dependency (whether explicit dependency is a burden or a help is debatable!). Again, this thread is a brainstorm, instead of any actual proposal to really implement something :) I just have something in my mind and thus output it here in case it is interesting for anyone. |
Got it, so Reactive Policies are enforced at runtime, that's not on the docs, good to know. Then it definitely ads more safety, probably would also make mocking tests for individual widgets very easy since you know the store dependencies beforehand right? |
We have to spend time on improving the documentation. I wrote the first version few years back and need help to revisit and make it more comprehensive. |
@pavanpodila I'm reading the codebase and trying to make sense of it, after I get an idea of how this works I'll try to do some PRs for the docs. Is there anywhere you discuss documentation stuff? |
No location yet but we can start a separate thread. Look forward to your PRs |
@fzyzcjy |
@fzyzcjy you can save the current behaviour, just rename current Observer into AutoObserver. But it would be a breaking change, you should add a migration note, that existing projects should now find-and-replace all Observers to AutoObservers in their codebases. So people can choose, and those who want to let MobX track the observables by itself will use an AutoObserver, but ones who want to subscribe to observables explicitly will use a manual Observer. |
As we know, riverpod is quite popular these days. I have been using mobx for a long time and think it is quite good, but I also see some strengths of riverpod. Therefore, I create this thread as a brainstorm :)
One thing I think interesting is its explicit dependency shown in code. It seems like they use
ref.watch(something)
, when mobx just read thesomething
directly. (I am not an expert in riverpod, so please correct me if I am wrong.)At first, I felt mobx's way is much neater and shorter, and it does give productivity. When the codebase has grown large, I sometimes also want to know what part will cause the downstream to react. In addition, IIRC once in a while it is possible to forget to put an
Observer
, and then everything does not react. For example, this issue can happen when doing refactor to extract some subtree of widgets into a new widget - and then forget to put an extra observer. Similar issues are, for example, when you have an Observer, and later refactor and add a LayoutBuilder/Builder/whatever - then everything inside thebuilder
will not be reactive without an extra Observer.So a very very naive change to mobx may be: Add
ref
as an argument toObserver(builder: (context, ref) => ...)
. For @observable, maybe enhance the codegen such that it gives some reader object that can be used likeref.watch(store.myfield)
. For @computed, maybe something similar (since again we use codegen).There do exist drawbacks for the approach. For example, people using it for small projects may feel that there are extra code to be typed.
The text was updated successfully, but these errors were encountered: