Closed bySabi closed 8 years ago
Then, in a large app, you should manually implement shouldResuscribe on every component that uses a portion of a Mongo record (I think almost every component in the app)...
@markoshust I think that's the whole point of using observables at all! Not worring about fine-grained dependencies. Not doing manually things that a piece software can do.
See https://mobxjs.github.io/mobx/refguide/observer-component.html
Characteristics of observer components
- Observer only subscribe to the data structures that were actively used during the last render. This means that you cannot under-subscribe or over-subscribe. You can even use data in your rendering that will only be available at later moment in time. This is ideal for asynchronously loading data.
- You are not required to declare what data a component will use. Instead, dependencies are determined at runtime and tracked in a very fine-grained manner.
- Usually reactive components have no or little state, as it is often more convenient to encapsulate (view) state in objects that are shared with other component. But you are still free to use state.
- @observer implements shouldComponentUpdate in the same way as PureRenderMixin so that children are not re-rendered unnecessary.
- Reactive components sideways load data; parent components won't re-render unnecessarily even when child components will.
- @observer does not depend on React's context system.
That's the point also to make Meteor Data observable ("load it into a Mobx Store") so the Store will react to changes in Mongo, but the observers of the Store will only react if their "contracts" says to....
@mweststrate @DominikGuzei do you agree?
I get what you are saying... but it really depends on your use-case :) I see situations where you can use both approaches. I could be a bit side-tracked right now, as I'm building a CMS with editable forms, so I specifically "don't" want Meteor to react with updates, as the forms are editable.
That said, it should be fairly simple to pipe the data into mobx and then use that as the single source of truth. See https://github.com/mobxjs/mobx/issues/84#issuecomment-223747251 for example :)
@jmaguirrei you made the important observation here. Mobx is all about giving developers their dream back: simple state management where you don't need a master thesis to understand how it is composed by 1 million different functions. At the same time, ultra-fast and fine grained updates without any manual work as you already said. If you use Mobx in strict mode you even get the last and important feature of redux: immutable state that can only be updated by Mobx actions. For me redux is dead, the only (sad) reason im still using it in some places is because of the great work on libraries like redux-form.
By the way: we released the Mobx / Tracker integration code i posted above as a meteor package ready to use in your projects: https://github.com/meteor-space/tracker-mobx-autorun
Please give us feedback on the documentation etc. 😉
@markoshust You are right, it depends on your case.
In my case my app needs to be fully reactive and was getting really slow because of my lack of knowledge to implement shoulComponentUpdate. I was (or I think I was) following all best practices in React, almost all stateless components and one component for the whole state. But a month ago I get to the point I cant type in an input box without noticing a really annoing delay. Every component was 'tied' to the app component in a way, so the whole was rendered again and again.
That's why I refactor everything, and adopted Mobx principles.
@DominikGuzei I will definitely check it out! I am also going to be using strict mode (didn't heard about it until today), if this is the best way to get the benefits from Mobx.
The thing maybe you could help me to ask is:
Is it true that if I define correctly my Mobx Domain Store (for example Conversations synced with Meteor backend), who takes some data from other Stores (the UI StateStore or another Domain Store), computes some fields, and returns an object with the relevant data to the UI (nothing more nothing less than the data that is logical part of this domain), denormalized but not duplicated, the Observer Components will allways re-render only if the relevant portion of the store has changed?
If the answer is Yes, what can I be doing wrong in the example code I posted today? Can you check that and tell me if you see something that should be better implemented?
Thanks Dominik. Hello everyone, tracker-mobx-autorun
is solution that is tested on a relatively big client project. Reduction in boilerplate code is amazing. We started out using Tracker.Component but we quickly replaced it with this as we saw it as an anti-pattern. MobX is really awesome and performant. Currently we have hybrid Redux/MobX solution on that project, since almost all forms are redux-form based.
We have aslo ported mantra-sample-blog to this ui stack to showcase usage of this library on a real project. We will make that public this week.
@darko-mijic Yes, I gave it a re-read now and I am wondering 2 things:
1) Its clear I will always use computed values that are based on observables "primitive" values. I think I was achieving this using extendObservable on get object properties, for example:
// Open Profiles
get officeProfile() {
if (this.allPeople && openProfileId) {
const
profile = _.find(this.allPeople, { _id: openProfileId }),
style = profileStyle({ cursorX, cursorY, openProfileId });
return _.assign(profile, { style });
}
},
Now I dont know if I this field is computed or not, how can I check that? Or should I return
return computed(_.assign(profile, { style }))
instead?
2) I only have observer on the container component because the "dumb" component does nothing with data, only pass the data to the JSX markup (please see my code above). Should I still be including observer on dumb components?
@jmaguirrei I think I was randomly seeing lags in inputs as well, it must be do to react-komposer. I think I'll need to rethink a few things of my decided implementation to move to more idiomatic mobx... you are all convincing me this is probably the way to go.
@markoshust Check if that input is deeply nested in the component hierarchy and/or it mutates state that have an impact on the top level component.
I think the component should be as decoupled as possible. One thing is to be in a deep level in the tree, that's inevitable, but another is to be "deep and tight" (props => props => props).
In the Mobx architecture, almost all my components are parameterless functions:
const User = () => {
const
actions = _.pick(OfficeActions, [ 'onToggleUser' ]),
data = _.pick(UsersStore, [ 'allPeople' ]),
ready = UsersStore.ready;
if (ready) {
return <_User_ actions={actions} data={data} />;
}
return false;
};
export default observer(User);
As you see, the component itself talks to the store(s) and pick the data they need and renders its dumb component (_User_
).
I am still not 100% sure that's the best (performant and mantainable) way to go, but make sense and it will make 100% sense when I see React Perf Wasted Time = 0
@jmaguirrei pretty sure the issue is I'm not using mobx idiotmatically. this composeWithMobx container composer works great, however like those who pointed this out before me, it's not taking advantage of mobx-react's observer capabilities and diff matching. I think I may still use composeWithMobx in a few places, however it's causing extra, unnecessary renders because it appears to re-render on every state change, because it doesn't use the same logic that is in mobx-react. like those who said before, the whole reason i'm using mobx is to ditch the extra work (such as the resubscribe/shouldComponentUpdate stuff), so I see no reason not to switch this to idiomatic mobx and just use the suggested tools for this architecture.
@markoshust What do you mean when you say "idiomatically"? Can you give me an example?
@darko-mijic Cant wait to see this!
We have aslo ported mantra-sample-blog to this ui stack to showcase usage of this library on a real project. We will make that public this week.
@darko-mijic can't wait to see this too! please post up here once that is ready.
@jmaguirrei just meaning using the official/suggested tools "the mobx way". packages such as react-komposer
are great, but mobx is magic, and should use magic implementations to take full advantage of it's tech.
FYI there appears to be some significant interest in meteor + mobx (+ mantra), and it's been a super long time since I've done any screencasts. So there's something on the way as soon as I can carve out a few ;)
Looking forward to that @markoshust !
No worries, I will post a link here @markoshust and @jamiewinder. It would be nice to discuss it and hear feedback. My port will be Mantra-free, Mantra is boilerplate generator in my opinion.
Here is an example Store with https://github.com/meteor-space/tracker-mobx-autorun and the more idiomatic Mobx I was able to done!:
Though I am still having unnecesary re-renders, still looking to get the best use of Mobx observables
import {
mobx,
Messages,
SubsManager,
autorun,
} from '/lib/imports/client.js';
import { myConsole } from '/imports/dev/functions/myConsole.js';
import { State } from '/imports/state/state.js';
import { myMessages } from './functions/myMessages.js';
const { observable, computed, action, useStrict } = mobx;
useStrict(true);
/*
***************************************************************************************************
M E S S A G E S S T O R E
***************************************************************************************************
*/
class Store {
@observable myMessages;
@computed get isInConversationReady() {
return this.myMessages.length > 0 && State['Chat_isInConversation'];
}
}
const
consoleActive = true,
MessagesStore = new Store(),
messagesSub = new SubsManager();
/* ------------------------------------------------------------------------------------------------
myMessages
------------------------------------------------------------------------------------------------ */
autorun(() => {
messagesSub.subscribe('messages', { conversation_id: State['Chat_conversation_id'] });
action('MESSAGES_STORE: myMessages', (messages) => {
MessagesStore.myMessages = _.filter(
myMessages({
Messages: messages,
people_ids: State['Chat_conversationPeople'],
selectedTab: State['Chat_selectedTab'],
myUser_id: Meteor.userId(),
}),
item => item.agendaNum === State['Chat_selectedAgendaNum'] || State['Chat_selectedTab'] !== 3
);
myConsole(MessagesStore, 'MESSAGES_STORE', 'green', consoleActive);
})(
State['Chat_conversation_id'] ?
Messages.find({ conversation_id: State['Chat_conversation_id'] }, { sort: { date: 1 }}).fetch() :
[]
);
}).start();
export { MessagesStore };
@jmaguirrei thanks, should the subscribe be within the autorun?
@markoshust I had the same wondering and according to this example, it should be inside:
https://github.com/meteor-space/tracker-mobx-autorun
I think if it's outside will not react to state changes. If subscription does not depend on state, it could be outside. Also because I am using Subsmanager, I think there is no over subscription
@markoshust Also see this, it's a pattern to get not "over reactivity", I will be refactoring today according to this and monitoring results with React Perf.
@jmaguirrei I think what you are experienced was a concern of mine, please see the last comment at https://github.com/meteor-space/tracker-mobx-autorun/issues/5#issuecomment-236375354
I'm worried that over-subscribing to data will cause a lot of listeners that never get cleaned up. When a component mounts, the autorun should be started, and when it unmounts, the autorun should be stopped.
I think there should be start/stop functionality on mounting/unmounting... no? The subscribe should be in the start function, not within the autorun. Technically, on any state change, it is creating a new subscriber (whether it's a performance issue or not is another question, perhaps not, but I think technically it should be outside of the autorun). I'm also using SubsManager but this should be setup in a manner which works with or without.
I think you should really be destructuring your store object like so in your contianer:
const { a, b } = store;
<List a={a} />
<List2 b={b} />
instead of passing the entire store to your presentational components. You should be setting up your presentation components to not reference Store, so they just work off props and be used if store is used or not.
@markoshust In the "old" way of getting data from Meteor with the react-meteor-data package (https://atmospherejs.com/meteor/react-meteor-data) the subscription is also inside the function. I think your concerns are valid, but maybe Meteor handle this? (just guessing...)
In respect to passing the store to components, I will only use that pattern for reusable components.
The problem with:
class MyContainer extends React.Component {
render() {
const { observableA, observableB } = store;
return (
<div>
<List a={observableA} />
<List2 b={observableB} />
</div>
);
}
}
Is that if observableA
change then MyContainer
, List
and List2
are re-rendered. Same thing if observableB
changes.
On the other hand, if you just pass the store or (better) dont pass anything but let the components to import the data they need, no "sided" renders will take effect.
Hmm, that doesn't seem correct. If observableA is passed in, it should just render List. That is the whole idea of the observer
function. Let me test things out. and I'll report back.
You're right about react-meteor-data
-- it is inside the function. I guess it's not a problem because of that. I still think we need a way to stop the autoruns/subscriptions on a cleanup/componentWillUnmount function.
Until yesterday I was convinced that just List would be rendered but the thing that @andykog pointed out in https://github.com/mobxjs/mobx-react/issues/94, is that with this pattern the Parent (MyContainer) observes A & B, so if A changes it will re-render, and if he re-render then all it's child are re-rendered too.
Today I changed my components to import what they need and I am finally getting 0 wasted time. Only pass not observable data as props.
Another thing that it's important to be aware is that you should also "granularize" your observables, because when they are invalidated it generates reactions in all observables (naturally), so if they are grouping things, they will cause unnecesary re-renders also.
So, rule of thumb:
He's wrong -- I put up a dummy repo to reproduce this. Feel free to clone it out and test: https://github.com/markoshust/mobx-render-test
Yep, sorry for that. For some reason, I was sure, that forceUpdate
results in skipping shouldComponentUpdate
on children as well. Shame on me.
@markoshust You are right... I was getting unnecesary re-rendering because I wasn't using observer() in Item1 & Item2 child components... I was using it before, but I remove them because "I wanted not to re-render so better don observe here...." The thing is totally in the other way. If you observe in all your components, then you avoid unnecesary re-renders.
If you remove them from your example, you will see that they always re-renders.
So, I am optimist again, just observe "everything" and there should be no problem.
Thanks.
@jmaguirrei, you can apply pureRenderMixin, or something like https://www.npmjs.com/package/pure-render-decorator instead of making observer. This happens just because @observer
already implement pureRenderMixin for you
@andykog If I understand correctly, there are 2 ways then:
Is that correct? If so, whats the benefits of the first way over the second?
whats the benefits of the first way over the second
@jmaguirrei, the benefit is that child component can be “dumb”. It can know nothing about your stores and depend only on props.
This article is targeting redux, but I think the concept is suitable for mobx too https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0#.b5oufte81
@jmaguirrei yep, i think the mobx approach is "observer everwhere" and you won't have any issues. it also won't have performance problems on components not using mobx.
@andykog it appears mobx-react's observer does a bit more -- there is some mobx code in there? https://github.com/mobxjs/mobx-react/blob/master/src/observer.js#L146 -- I'm not sure, I'm completely stupid about pureRenderMixin and the different approaches :)
yep, i think the mobx approach is "observer everwhere" and you won't have any issues. it also won't have performance problems on components not using mobx.
I like making UI components completely not aware of stores. That way they can be reused later, even in non-mobx app. So I wouldn't say “observer everwhere” is always a good idea.
it appears mobx-react's observer does a bit more -- there is some mobx code in there
that 3 lines are just checking if update is scheduled. If yes, no need to update ringt now, because it will happen anyway in a moment. The pureRendering implementation is the rest of the method.
@andykog @markoshust For some reason at React doesnt use PureRender by default. So if we decorate all dumb components with this maybe can loose reactivity.
For now I am confident about importing stores (to container components) instead of passing props, because I feel more control over the data flow.
We'll see. I am developing a large app, so I am sure I will encounter some more "rocks in the road".
@andykog thanks for the implementation info. I was wondering how to rid my code of observer calls all over the place. I'll test this out.
For some reason at React doesnt use PureRender by default
@jmaguirrei, React can't use PureRender by default becouse then it will fail when one of the props is some complex tree that was changed deeply inside. Then it needs to make deep equality check, resulting in problems with performance. Or some of the props may not be a plain object, but some class that can't be compared at all. So when you implemen PureRender by yourself you are taking the responsibility not to make such sneaky things.
So if we decorate all dumb components with this maybe can loose reactivity
Why? We won't loose reactivity for sure :-D
thanks for the implementation info. I was wondering how to rid my code of observer calls all over the place. I'll test this out.
@markoshust, you can simply replace it with pure render decorator, but I doubt that you'll notice some considerable difference in performance.
@markoshust and @jamiewinder, here is the initial draft of ported Mantra Sample Blog: https://github.com/darko-mijic/mantra-sample-blog-app/pull/1
There is only one thing I miss from Redux and that is redux-form.
@darko-mijic - FYI, I think you're intending to reference someone other than me? :)
Sorry about that ;)
Slightly off topic, but may I ask you how you have your stores subscribe to Meteor collections? Do you fetch all data beforehand, i.e. all documents in the collection with all fields? Because your store is unaware of the concrete data requirement of the react components.
Data can be filtered on publication level by specifying document fields that will be published. You can also transform and map data fetched from publications in autoruns.
In most of my project i am using event sourcing which gives me the ability to generate fully denormalized collections based on view requirements using projections. That makes things much easier. The only source of truth in event sourced system is the commit store which holds immutable stream of domain events. Projections are than used to generate fully denormalized view cache. Here is an example of one simple projection: https://github.com/meteor-space/projection-rebuilding-example/blob/feature/ui/source/imports/application/projections/transactions-projection.js
@darko-mijic @timeyr I am having more reactions than needed, I think maybe you already dealed with that.
I have a collection named Meetings in Meteor (Mongo) with this structure:
// In the server
// Collection
const Meetings = new Mongo.Collection('meetings');
// Schema
const MeetingsSchema = new SimpleSchema({
_id: {type: String},
title: {type: String},
date: {type: Date},
tags: {type: Object},
'tags.main': {type: [ String ]},
'tags.related': {type: [ String ]},
people: {type: Object},
'people.owner': {type: [ String ]},
'people.members': {type: [ String ]},
'people.watchers': {type: [ String ]},
agenda: {type: [ Object ], optional: true},
'agenda.$.title': {type: String},
'agenda.$.status': {type: String},
'agenda.$.time': {type: Object, optional: true},
'agenda.$.time.total': {type: Number},
'agenda.$.time.remaining': {type: Number},
'agenda.$.content': {type: Object, optional: true, blackbox: true},
agreements: {type: [ Object ], optional: true},
'agreements.$.content': {type: Object, optional: true, blackbox: true},
'agreements.$.likes': {type: [ String ]},
'agreements.$.dislikes': {type: [ String ]},
commitments: {type: [ Object ], optional: true},
'commitments.$.people': {type: [ String ]}, // 0: responsible, 1: revisor
'commitments.$.content': {type: Object, optional: true, blackbox: true},
'commitments.$.dueDate': {type: Date},
comments: {type: [ Object ], optional: true},
'comments.$.people_id': {type: String},
'comments.$.content': {type: Object, optional: true, blackbox: true},
status: {type: String},
company_id: {type: String, index: 1},
});
Meetings.attachSchema(MeetingsSchema);
Meteor.publish('meeting', function ({ _id }) {
return Meetings.find({ _id });
});
I am subscribing to the whole document only when user had selected 1 meeting_id. So I am using autorun from https://github.com/meteor-space/tracker-mobx-autorun to observe changes and update the Store:
// Meeting Store
class Store {
// @observable mymeetings;
@observable myMeeting_core;
@observable myMeeting_people;
@observable myMeeting_agenda = [];
@observable myMeeting_agreements = [];
@observable myMeeting_commitments = [];
@observable myMeeting_comments = [];
}
const
consoleActive = true,
MeetingStore = new Store(),
meetingSub = new SubsManager();
/* ------------------------------------------------------------------------------------------------
myMeeting
------------------------------------------------------------------------------------------------ */
autorun(() => {
meetingSub.subscribe('meeting', { _id: State['Meeting_meeting_id'] });
action('MEETINGS_STORE: myMeeting', () => {
if (meetingSub.ready() && State['Meeting_meeting_id']) {
const myMeetingInfo = myMeeting({
meeting: Meetings.find({ _id: State['Meeting_meeting_id']}).fetch()[0],
allPeople: CoreStore.allPeople,
});
MeetingStore.myMeeting_core = _.pick(
myMeetingInfo, [ '_id', 'title', 'date', 'tags', 'status' ]
);
MeetingStore.myMeeting_people = _.pick(
myMeetingInfo, [ 'zOwner', 'zMembers', 'zWatchers', 'zCommentators', 'zCommitters' ]
);
MeetingStore.myMeeting_agenda = myMeetingInfo.agenda;
MeetingStore.myMeeting_agreements = myMeetingInfo.agreements;
MeetingStore.myMeeting_commitments = myMeetingInfo.zCommitments;
MeetingStore.myMeeting_comments.replace(myMeetingInfo.zComments);
}
myConsole(MeetingStore, 'MEETINGS_STORE', 'pink', consoleActive);
})();
}).start()
Each field is an array of objects and has its correspondent component (container):
The thing is either Agenda, Agreements, Commitments or Comments has a change (I use a terminal to update a document directly in Mongo), the whole Store is reacting, for example:
Change content in agreement number (2) => Agenda, Agreements, Commitments, Comments are re-rendered... That's not OK.
// Comments.jsx
const Comments = () => {
const { myMeeting_core, myMeeting_comments } = MeetingStore;
if (myMeeting_core && myMeeting_comments) {
const
actions = {
onAddMeetingMember: MeetingActions.onAddMeetingMember,
onDelMeetingMember: MeetingActions.onDelMeetingMember,
},
data = {
meeting_id: myMeeting_core._id,
zComments: myMeeting_comments,
};
console.log('Comments Render');
return <_Comments_ actions={actions} data={data} />;
}
return false;
};
export default observer(Comments);
I think if I split the document into separate collections, I can solve that, but it looks wrong to force that.
So, what am I doing wrong? Should I use different collections in Mongo to achieve that? Or use different projections for each field in the store, remaining same collection?
@jmaguirrei: I will take a look at your post tomorrow.
BTW, have you seen new API with live query support? https://github.com/meteor-space/tracker-mobx-autorun/pull/10
@darko-mijic Yes, I knew about it today hoping to help me with my issue. If you think this could help in my case, I can give it a try tomorrow. Still I'll appreciate your feedback first about my code :) Thanks
@darko-mijic I think the thing is that in this piece of code:
autorun(() => {
meetingSub.subscribe('meeting', { _id: State['Meeting_meeting_id'] });
action('MEETINGS_STORE: myMeeting', () => {
if (meetingSub.ready() && State['Meeting_meeting_id']) {
const myMeetingInfo = myMeeting({
meeting: Meetings.find({ _id: State['Meeting_meeting_id']}).fetch()[0],
allPeople: CoreStore.allPeople,
});
MeetingStore.myMeeting_core = _.pick(
myMeetingInfo, [ '_id', 'title', 'date', 'tags', 'status' ]
);
MeetingStore.myMeeting_people = _.pick(
myMeetingInfo, [ 'zOwner', 'zMembers', 'zWatchers', 'zCommentators', 'zCommitters' ]
);
MeetingStore.myMeeting_agenda = myMeetingInfo.agenda;
MeetingStore.myMeeting_agreements = myMeetingInfo.agreements;
MeetingStore.myMeeting_commitments = myMeetingInfo.zCommitments;
MeetingStore.myMeeting_comments.replace(myMeetingInfo.zComments);
}
myConsole(MeetingStore, 'MEETINGS_STORE', 'pink', consoleActive);
})();
}).start()
The variable myMeetingInfo
changes after any part of the document in Mongo changes (for example a content in agreement number 2), so autorun fires the mobx action that updates all observable values in the store:
But, in practice observables 1) 3) 4) has not changed.
So I think 2 ways to solve this:
1) Mobx detects that new value equals old value, so not observer (mobx-react) has nothing to deal. 2) I have to split the code or the collection to have independent observers. This is much boilerplate, but doable.
So, thinking about option 1) there is already a way to achieve this? Do you agree with the approach?
Hello @mweststrate.
I'm trying to use/integrate mobservable with Meteor.
Commented code is tested and working. This a sample of current proof of concept code:
"mobviously" I don´t get a plain object on
afiliado
var, just:ComputedObservable[[m#1] (current value:'undefined')]
is possible do this ? what is the way?
Thanks for your time and happy new year!!!