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

How integrate with Meteor Tracker example? #84

Closed
bySabi opened this issue Dec 31, 2015 · 95 comments
Closed

How integrate with Meteor Tracker example? #84

bySabi opened this issue Dec 31, 2015 · 95 comments

Comments

@bySabi
Copy link

bySabi commented Dec 31, 2015

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:

var afiliado = mobservable.observable(function() {
    var afiliado;
    Tracker.autorun(function() {
      // HARDCODE MongoID for test only
      afiliado = Orgz.collections.Afiliados.findOne({_id: 'EbPM2uWJhbd8rZP8P'});
    });
    return afiliado;
});

ViewPersona = mobservableReact.observer(React.createClass({
/*
  mixins: [ReactMeteorData],
  getMeteorData() {
    Meteor.subscribe("afiliados", this.props.id);
    return {afiliado: Orgz.collections.Afiliados.findOne(this.props.id)};
  },
*/
  render() {
  //  return <ViewPersona_ afiliado={this.data.afiliado} />;
    return <ViewPersona_ afiliado={afiliado} />;
  }
}));

ViewPersona_ = React.createClass({
  render() {
    return <AfiliadoTabs afiliado={this.props.afiliado} />;
  }
});

"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!!!

@bySabi
Copy link
Author

bySabi commented Jan 1, 2016

Hi @mweststrate

Is working with a few "mobviously" :-) sorry!, changes ...
on line: return <ViewPersona_ afiliado={afiliado()} />;

var afiliado = mobservable.observable(function() {
    var afiliado;
    Tracker.autorun(function() {
      // HARDCODE MongoID for test only
      afiliado = Orgz.collections.Afiliados.findOne({_id: 'EbPM2uWJhbd8rZP8P'});
    });
    return afiliado;
});

ViewPersona = mobservableReact.observer(React.createClass({
/*
  mixins: [ReactMeteorData],
  getMeteorData() {
    Meteor.subscribe("afiliados", this.props.id);
    return {afiliado: Orgz.collections.Afiliados.findOne(this.props.id)};
  },
*/
  render() {
  //  return <ViewPersona_ afiliado={this.data.afiliado} />;
    return <ViewPersona_ afiliado={afiliado()} />;
  }
}));

ViewPersona_ = React.createClass({
  render() {
    return <AfiliadoTabs afiliado={this.props.afiliado} />;
  }
});

A few questions:
1- This Tracker/moboservable have sense?
2- You see any problem with this solution?
3- observable function afiliado return must be:
return afiliado ? afiliado : null; is working with return afiliado

I going to deep more on this Meteor/mobservable affair. Any advice??

Thanks.

@bySabi
Copy link
Author

bySabi commented Jan 1, 2016

it is 'half' working, Tracker.autorun(function() { ... is not retriggered on findOne changes.
Any idea??

@bySabi
Copy link
Author

bySabi commented Jan 1, 2016

Well. mobservable observe changes on 'afiliado' reference but I need it observe on afiliado object fields.
My ignorance make me treat afiliado like a plain Object and isnot.
I have to conclude this is not way??
Any advice are welcome ...

@mweststrate
Copy link
Member

Hi @bySabi

Happy new year as well! I'm not a meteor expert, so I'm wondering, does the findOne in autorun return the same instance every time or a new object? If the same object is returned each time it will probably suffice to make the affiliado object observable itself so that changes to it are picked up, assuming that they are plain objects further themselves (sorry, last time with meteor is a few years ago, I should pick it up again I guess ;-)). Probably affiliado = observable(...findOne...) will do the trick in that case.

An issue of your current solution might be that your Tracker.autorun subscription is never disposed. (should be done on componentWillUnmount).

I think your example can be simplified to:

var afiliado = mobservable.observable(); // observable reference
var autorunner = Tracker.autorun(function() {
      // HARDCODE MongoID for test only
      afiliado(Orgz.collections.Afiliados.findOne({_id: 'EbPM2uWJhbd8rZP8P'}));
});

ViewPersona = mobservableReact.observer(React.createClass({
  render() {
    return <ViewPersona_ afiliado={afiliado()} />;
  },
  componentWillUnmount() {
     autorunner.stop();
  }
}));

ViewPersona_ = React.createClass({
  render() {
    return <AfiliadoTabs afiliado={this.props.afiliado} />;
  }
});

I think it would be really nice if the Tracker.autorun could be skipped, I'm gonna tinker about whether that is possible :)

edit: use setter

@bySabi
Copy link
Author

bySabi commented Jan 2, 2016

Hello @mweststrate

I don´t know, yet, is findOne return the same reference each time. If we make this code work sure would know it.

I try your suggestions but fail with:
Uncaught Error: [mobservable.observable] Please provide at least one argument.

Thanks for the componenWillUnmount part this will next step to do is first step get solved.

On meteor community we are a little confused on what to choose. Many developers migrate from TFRP based architecture of Blaze templates to React but is not clear what architecture choose cause Flux & sons overlap with Meteor client-side cache/livequery/...
I don´t really like Redux on Meteor, is a wonderful solution, well done and easy. But after a half year with Tracker TFRP I feel Flux it is a downgrade. I'm a Solo developer, a one man shop, that need all the magic behind that I can have.
I´m sure that you and mobservable will really welcome on Meteor community but before that we need show some working example to involved them.
Is this work my next step is move https://github.com/meteor/simple-todos-react to mobservable.

I can make a repo with all is needed for test Meteor and mobservable if you wanna jump to the wagon.

@mweststrate
Copy link
Member

Sorry, listing should start with observable(null). A test repo for meteor
and mobservable would be nice indeed!

On Sat, Jan 2, 2016 at 12:16 PM, bySabi notifications@github.com wrote:

Hello @mweststrate https://github.com/mweststrate

I don´t know, yet, is findOne return the same reference each time. If we
make this code work sure would know it.

I try your suggestions but fail with:
Uncaught Error: [mobservable.observable] Please provide at least one
argument.

Thanks for the componenWillUnmount part this will next step to do is
first step get solved.

On meteor community we are a little confused on what to choose. Many
developers migrate from TFRP based architecture of Blaze templates to React
but is not clear what architecture choose cause Flux & sons overlap with
Meteor client-side cache/livequery/...
I don´t really like Redux on Meteor, is a wonderful solution, well done
and easy. But after a half year with Tracker TFRP I feel Flux it is a
downgrade. I'm a Solo developer, a one man shop, that need all the magic
behind that I can have.
I´m sure that you and mobservable will really welcome on Meteor community
but before that we need show some working example to involved them.
Is this work my next step is move
https://github.com/meteor/simple-todos-react to mobservable.

I can make a repo with all is needed for test Meteor and mobservable if
you wanna jump to the wagon.


Reply to this email directly or view it on GitHub
#84 (comment)
.

@bySabi
Copy link
Author

bySabi commented Jan 2, 2016

Good! .. it is working! reactively on findOne changes. :-)
but ... it throw a exception on changes:

dnode.js:207 [mobservable.view '<component>#.1.render()'] There was an uncaught error during the computation of ComputedObservable[<component>#.1.render() (current value:'undefined')] function reactiveRender() { 

I will do a test repo in a couple of hours.

I stay on touch, this promise ...

@mweststrate
Copy link
Member

I think after that exception another exception is logged with the actual error that did occur

@bySabi
Copy link
Author

bySabi commented Jan 2, 2016

Only have one, the above.

@mweststrate
Copy link
Member

hmm then the exception is eaten somewhere, the above warning comes from a finally clause. I'll await the test repo :)

@bySabi
Copy link
Author

bySabi commented Jan 2, 2016

Hi @mweststrate
here is the repo: https://github.com/bySabi/simple-todos-react-mobservable

I try migrate to mobservable without luck. The original is update a little.
mobservable stuff is on: client/App.jx

I you wanna test original implementation just rename: client/App.jsx.ReactMeteorData to client/App.jsx

@luisherranz
Copy link
Member

@bySabi there you go man:
bySabi/simple-todos-react-mobservable#1

I have tried to modify your example as less as possible.

@mweststrate
Copy link
Member

@bySabi sorry, didn't find the time yet to dive into this,
@luisherranz nice, tnx a lot! A generic patterns starts to appear already in your PR :)

Something like:

const state = observable({
   tasks: []
})
syncWithMeteor(state, 'tasks', () => Tasks.find({}, {sort: {createdAt: -1}}).fetch())

function syncWithMeteor(obj, attr, func) {
   tracker.autorun(() => {
      mobservable.extendObservable(obj, { [attr]: func() })
   });
}

@luisherranz
Copy link
Member

I don't know if you had time to read the endless conversation with James (sorry! 😅), you don't need to if you don't want to, don't worry.

The thing is I have fallen in love with Mobservable. It's vastly superior to Tracker and I am going to make an adapter so people coming from Meteor (who already know what TRP and its benefits but have switched from Blaze to React due to the latest Meteor changes) can swap Tracker with Mobservable.

I have some ideas on how to make the integration. An obvious one could be a wrapper like the one in your last comment, but there are more options.

  • Overwrite the Tracker library with a new one which is linked internally with Mobservable with the goal of a transparent integration. That way things like Mongo Cursors or reactive variables (Session, Meteor.userId()...) would return Mobservable observables instead. Tracker autoruns would be converted to Mobservable autoruns and so on...

I understand how Tracker works really well but I have some questions about Mobservable. I'd love to have a conversation about the internals of Mobservable, to see if that idea is feasible. In Tracker, the dependencies (Tracker.Dependency) are disconnected from the actual data, so Tracker doesn't know about the data, per se. Therefore, I am not sure if that approach would work.

  • Another option is to make the most common reactive stuff compatible one at a time. This is probably more work but it will work quite transparently as well, and I am sure this can be done. Jagi's Astronomy package hooks into collections and when you do a query (Posts.find()) it returns an Astronomy class instead of a cursor, so I could hook in the same he is doing and return a mobservable observable instead.

If you have 5 minutes this week for this let me know and we can meet in your discord channel 👍

@bySabi
Copy link
Author

bySabi commented Jan 11, 2016

Great! idea! .. reimplement Tracker API with mobservable.
If you need a beta tester ....

@mweststrate I think that me and the JS community in general need a new article from you. Right now everybody, me include, is obssesed with purity, inmutability, side-effects paranoia and that´s why flux pattern buy us on first place.
But I see a little burocracy on this pattern, say a component can´t take any decision. It must ask someone(emit a 'action') cause we cannot trust it for mutate data, even if the data is simple like a boolean var.
We need a pattern based on transparent reactivity than enable us mutate data in a predictive way without needed for thirty party trusted reducers.
Maybe you can write another Medium article explaining your thought about and put a example mobservable vs redux. And stop a little with the inmutability madness.
I´m sure many people on JS community glad you.

@dcworldwide
Copy link

@luisherranz glad to know i'm not alone in this kind of thinking. Like sabi said, happy to review your work if and when you get to it.

@luisherranz
Copy link
Member

Thanks @dcworldwide!

@mweststrate I want to start with this but I'm having a kind of a hard time getting the feature/2 mobservable branch to load on meteor.

  • First of all, Meteor support for npm packages is quite new (1.3 is in beta right now) so it doesn't work with local npm packages.
  • If I clone and import directly from the dist folder, it works, but the new 2.0 API is not built there, the object doesn't have reportObserved and reportAtomChanged.
  • I can't make it work directly importing the src folder because mobservable is written in ts and the typescript meteor transpiler doesn't work there.
  • I tried to make a new build with tsc but it threw some errors, so I guess that branch is not ready to be build yet.

If you have any other idea, or you can fix the build errors, or maybe push a beta version to npm let me know :)

@bySabi
Copy link
Author

bySabi commented Jan 22, 2016

@luisherranz you can install local packages this way. Clone 'mobservable' on 'app-1.3-test/packages` and install it:

npm install packages/mobservable --save

and this work too:

npm install https://github.com/mweststrate/mobservable.git#feature/2 --save

@luisherranz
Copy link
Member

Uhm...

This throws an 406 error on npm install

"dependencies": {
  "mobservable": "https://github.com/mweststrate/mobservable/tree/feature/2"
}

And the tarball installs fine but doesn't work

"mobservable": "https://github.com/mweststrate/mobservable/tarball/feature/2"

because the libs folder is missing.

If I clone the repo on packages and run npm install packages/mobservable --save I get the same typescript compiler errors.

@mweststrate
Copy link
Member

Yeah I still have to expose the atom api from the package in the 2 branch. Is it ok that I come back to you early next week? Otherwise my family will rightfully complain about not celebrating a weekend off ;)

@luisherranz
Copy link
Member

No problem my friend :)

@ghost
Copy link

ghost commented Mar 3, 2016

Any updates on replacing tracker with mobx?

@mweststrate
Copy link
Member

Nope, I think it should be quite doable. But I would need to learn meteor first. I think it should ideally be done by someone with a lot of Meteor knowledge and supported by MDG.

Nonetheless I think it is still possible to combine MobX and Tracker, for example by wrapping stuff in MobX autorun that notifies a Tracker Dependency when needed.

@mquandalle
Copy link

Hi, I’m working on a Meteor application (Wekan) that I would like to convert to React. After studying the different possibilities for data handling, I see a lot of value in Mobx as a “better Tracker, for React”.

The transition path from Tracker to React for most reactive data structures is relatively straightforward (eg replacing Session variables by Mobx observables is trivial) however one abstraction I miss is the client side reactive Minimongo cursors. Minimongo cursors are useful if you want to display a reactive subset of a collection, which is exactly what we do in Wekan (eg display all the cards with a specific tag).

So I’m super interested in an implementation of the Tracker API on top of Mobx. I took a look at it (early draft repository here: https://github.com/mquandalle/tracker-mobx-bridge) and at least some of the abstractions seems to match reasonably well (a Tracker dependency ~= Mobx atom for instance). I don’t have a good enough understanding of the Mobx internals to decide the best equivalences between every element of the Tracker API but I’m confident that the building a bridge is doable.

@bySabi
Copy link
Author

bySabi commented Mar 11, 2016

@mquandalle I try to archive this goal too, mobTracker but stop due time constrains and lack of knowledge. You can take the name is you like it. :-)

Your intentions are good but maybe not worth the effort spent on this task cause incoming Apollo project. In my IMO, MDG probably deprecate Tracker and Minimongo.

Maybe a better effort is bring mobX to Apollo, evangelize a little :-)

@mquandalle
Copy link

Yes, having the client side reactive cache of Apollo based on MobX is an attractive idea. We sure should evangelize that!

On the meantime I realize that the only high level feature that I’m missing from a MobX based “store” is the possibility to have cursor on a set of data. What is cool with a Cards.find({ author: 'authorId' }) cursor is that the observer is only recalled when a card that we care about is inserted, modified, or deleted. I guess I could just use a _.filter on a MobX array but that’s neither as efficient nor elegant. Are you aware of any good way to solve this problem @mweststrate? Is there somethink like a “Collection” build on top of MobX?

@mweststrate
Copy link
Member

@mquandalle I have to dive deeper in to this, but a short question: Are you primarily interested on cursors that tracks remote collections, or local connections (a filter seems to suggest the latter?). For these kind of advanced patterns atoms for lifecycle management and transformers for smart map/reduce like patterns might be key.

Edit: Wekan looks cool :)

@mquandalle
Copy link

I was thinking about local collections (that turns out to be synced with the server, but the syncing mechanism should be considered as an independent part that doesn’t interfere with it).

So if I have on my client collection a list of todo items:

let todoItems = observable([
  { id: "1", title: "abc", completed: false },
  { id: "2", title: "def", completed: true },
  { id: "3", title: "efg", completed: false },
]);

and I want to keep track reactively of the items that are not completed yet (items 1 and 3 in the above example) to display them in a view. What is the canonical way to do so? The good thing with a cursor is that it creates an atom that gets invalidated iff a document matching its selector gets modified.

@mweststrate
Copy link
Member

in that case I would go for todoItems.filter(todo => !todo.completed). I wouldn't worry to much about performance as it will only observe mutations to the array itself and all the relevant completed attributes. But if your collections are really large, you can improve this by utilizing a transformer that memoizes the completed state:

const completedChecker = createTransformer(todo => !todo.completed);
const completedItems = todoItems.filter(completedChecker);

@darko-mijic
Copy link

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.

@jmaguirrei
Copy link

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 };

@markshust
Copy link

@jmaguirrei thanks, should the subscribe be within the autorun?

@jmaguirrei
Copy link

@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

@jmaguirrei
Copy link

@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.

mobxjs/mobx-react#94

@markshust
Copy link

@jmaguirrei I think what you are experienced was a concern of mine, please see the last comment at meteor-space/tracker-mobx-autorun#5 (comment)

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.

@jmaguirrei
Copy link

@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.

@markshust
Copy link

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.

@jmaguirrei
Copy link

Until yesterday I was convinced that just List would be rendered but the thing that @andykog pointed out in mobxjs/mobx-react#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:

  • Keep your observables as decoupled as possible
  • Dont pass observables from stores to childs (unless reusable component) just import them in the child.

@markshust
Copy link

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

@andykog
Copy link
Member

andykog commented Aug 1, 2016

Yep, sorry for that. For some reason, I was sure, that forceUpdate results in skipping shouldComponentUpdate on children as well. Shame on me.

@jmaguirrei
Copy link

@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.

@andykog
Copy link
Member

andykog commented Aug 1, 2016

@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

@jmaguirrei
Copy link

@andykog If I understand correctly, there are 2 ways then:

  1. Observe in the parent Component and pass the data to the pureRender Child.
  2. Observe in both (Child component will automatically be pure rendered).

Is that correct? If so, whats the benefits of the first way over the second?

@andykog
Copy link
Member

andykog commented Aug 1, 2016

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.

@andykog
Copy link
Member

andykog commented Aug 1, 2016

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

@markshust
Copy link

@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 :)

@andykog
Copy link
Member

andykog commented Aug 1, 2016

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.

@jmaguirrei
Copy link

@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".

@markshust
Copy link

@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.

@andykog
Copy link
Member

andykog commented Aug 1, 2016

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.

@darko-mijic
Copy link

@markoshust and @jamiewinder, here is the initial draft of ported Mantra Sample Blog:
darko-mijic/mantra-sample-blog-app#1

There is only one thing I miss from Redux and that is redux-form.

@jamiewinder
Copy link
Member

@darko-mijic - FYI, I think you're intending to reference someone other than me? :)

@darko-mijic
Copy link

Sorry about that ;)

@timeyr
Copy link

timeyr commented Aug 3, 2016

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.

@darko-mijic
Copy link

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

@jmaguirrei
Copy link

@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):

  • Agreement => observes MeetingStore.myMeeting_agreements
  • Commitment => observes MeetingStore.myMeeting_commitments
  • etc

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?

@darko-mijic
Copy link

@jmaguirrei: I will take a look at your post tomorrow.

BTW, have you seen new API with live query support?
meteor-space/tracker-mobx-autorun#10

https://github.com/meteor-space/tracker-mobx-autorun/blob/87a7f7f4e30b3370e4600c0473c3c9974827c1e3/README.md

@jmaguirrei
Copy link

@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

@jmaguirrei
Copy link

@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:

  1. MeetingStore.myMeeting_agenda
  2. MeetingStore.myMeeting_agreements
  3. MeetingStore.myMeeting_commitments
  4. MeetingStore.myMeeting_comments

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests