Any plans to support Om-like reference cursors?

[skevy]

https://github.com/swannodette/om/wiki/Advanced-Tutorial#reference-cursors

Supporting these would prevent the long chain of passing props (or in omniscient's case, cursors) down long component trees. It would be really cool to have something like these. Morearty.js has something like this.

Wondering about your thoughts here.

[mikaelbr]

Hi! You can do this with immstruct and Immutable.js as well, you can create a cursor based on a path, and immstruct will fix so these cursors doesn't get stale (and thus avoid losing data).

Let's say you have a file with your structure:

var immstruct = require('immstruct');

module.exports = immstruct({
  /* My large Structure */
});

And in some component

var structure = require('./myStructure.js');

var SomeSubComponent = component(({cursor}) => (<div>{cursor.deref()}</div>));

var SomeComponent = component(({cursor}) => {
  var newCursor = structure.cursor(['path', 'to', 'someWhere']);
  return <SomeSubComponent cursor={newCursor} />;
});

Or are you thinking about setting a cursor on a specific component? Something like this:

var structure = require('./myStructure.js');

var SomeSubComponent = component({
  cursor: structure.cursor(['path', 'to', 'someWhere'])
}, ({cursor}) => (<div>{cursor.deref()}</div>));

var SomeComponent = component(({cursor}) => {
  return <SomeSubComponent />;
});

Something like that could possibly work, but it defeats the purpose of having re-usable and composable components.

[skevy]

This is a contrived example, but bear with me:

Imagine this hypothetical large structure for a quiz application:

{
    navigation: { ... },
    q_and_a: { ... },
    results: { ... },
    statistics: { ... },
}

Also imagine that in your first example, SomeComponent was called ResultsHandler and was passed a cursor to the "results" part of your structure, and SomeSubComponent was actually called StatisticsWidget and needed to be embedded in the "results page", but only needed to know about general statistics of the quiz.

In this case, you have two different components pointing to completely separate parts of the structure, but one is embedded in the other. Because of how shouldComponentUpdate works, if your "results" cursor doesn't change but "statistics" does, our StatisticsWidget component would not update.

This is what something like a reference cursor would aim to solve. Taken from the link I mentioned, the problem is that:

In many applications you have some component several levels deep in the render tree. The question is, how do we get data to this component without having to pass data to everyone in between the root of the application and where the component actually lives?

Perhaps something like your second example could work. And I'm not sure that I agree that it would defeat the purpose of having re-usable and composable components. If a component knew only about the data it needed, didn't need to receive data from it's parent and didn't need to pass data down to components below it, it would be very composable because it could essentially just live completely on its own and be "put" anywhere. Right?

[skevy]

Another thought:

It seems like, after talking to David Nolen, how this is implemented in Om may not be possible in Omniscient using the same methods.

What Om does is, in it's render function, loops through all components, checks their shouldComponentUpdate, and if it should update, forceUpdates. Then, in a second render pass, it takes an components not already updated by the initial "normal" cursor pass and then checks components that have reference cursors, checks their shouldComponentUpdate, and forceUpdates.

This is, of course, different than what's going on with immstruct swap and Omniscient. In omniscient, we're just blindly rendering the whole tree, relying on shouldComponentUpdate and React to do it's job, where as Om has done it more manually, and perhaps, intelligently.

Not sure what we could do about this if this functionality is something that could be added to omniscient. I'll put some thought into it.

[torgeir]

I see what you're getting at, and as of today this won't really work without both components being passed a cursor to the piece of state that encompasses them both (results and statistics), or by modifying shouldComponentUpdate of the outer one.

While I absolutely agree that something similiar could be useful in some scenarios, I'm also kind of reluctant, as it complicates reasoning about what renders when. Are we not in this way effectively relying on "some global state" to also trigger re-renders of some sub component? :) In some cases this is probably exactly what you'd want, though.

I haven't really dug into Om, but is it tracking individual components to accomplish this - to re-render only parts of the component tree that rely on this data without starting from the top? Or could we use a mixin that forceUpdates a component down the line when something changes? Following up the advanced tutorial, e.g.

var SubView = component([ forceUpdateOnChange(items) ], () => ...)
var MainView = component(({someOtherCursor}) =>
  <div>
    <SubView />
    ...
  </div>);

We've been on to vaguely similar ideas with immstruct for reacting to changes of these kinds of structures omniscientjs/immstruct#19 and omniscientjs/immstruct#18

[skevy]

Short answer - yes, I think a mixin can work, at least for a naive implementation, but I don't think we'd be able to get the same performance improvements that Om get's when dealing with reference cursors.

I played around with this a bit last night. Basically, I just had mixin (like you proposed) that registered the component with a cursor on mount, and then a second listener on structure swap that would handle this "second-render pass". It works. But, what we wouldn't be doing with this method is preventing a component that may have been updated by the first render pass from being rendered again, which Om DOES do (and that's why the reference cursor implementation that they do essentially "costs nothing").

To do that, I think, I'm not sure we would be able to implement anything like that without keeping track of every component (Om does do this...specifically, you can see what it's doing here, if I'm reading the source right (I am by no means a clojure expert): https://github.com/swannodette/om/blob/master/src/om/core.cljs#L1139-L1158)

[skevy]

Perhaps @swannodette may have insight into why he chose to implement this in Om, and why he feels like it's a good decision and allows for easier, not more complicated, reasoning about UI. Though admittedly, he may not have much more to say than what he's already written. :)

I think with this library being called "omniscient", and obviously borrowing many of the ideas from Om, we should definitely consider implementing reference cursors. If we can figure out how to do it efficiently (and with less cruft than Morearty's "context", as most of that functionality already exists in immstruct), I think it would be worthwhile to add it to the library.

[swannodette]

@skevy as was discussed in the React Conference hallway chats the Om approach isn't actually so different from the high level ideas behind Relay - you want to bind a component to some collection of information in the global application state without needing to pass it in from above. This is just a natural organization thing in my opinion. In Om people are not using Reference Cursors for everything, just in the places where it's more natural for the application design.

As to why it was implemented in the way it was. Om treats the application component tree like an AST, there's a first pass, and there is a second pass. Nothing happens in the second pass if it was already addressed in the first pass. The Om implementation also permits some further simple optimizations we haven't gotten to yet like sorting components by render depth (again allowing components further up the hierarchy to render first and eliminating later redundant work).

[torgeir]

@skevy I might very well not be grokking this, but why would we need another pass? :)

(..) what we wouldn't be doing with this method is preventing a component that may have been updated by the first render pass from being rendered again, which Om DOES do (..)

Will not the default shouldComponentUpdate prevent these kind of re-renders, as data you are passed have not changed? If your component forceRenders when data other than what it is passed is changed, this is the only time you will actually re-render, and you'd want to as what you depend on has changed

[mikaelbr]

I can get why you some time would want to do this, and your example makes sense, @skevy. But it would essentially tightly bind a component to a specific data.

I think it's worth trying out the naive approach. I suspect that the performance wouldn't be too bad, and perhaps, as @torgeir says, the shouldComponentUpdate will prevent double rendering. Might be wrong, but a quick proof of concept might smoke this out pretty easily.

[skevy]

I suppose the render pass thing would only really matter if you were doing rAF updates, because if we're NOT doing rAF, then multiple renders would happen anyway, which in that case you're correct, the default shouldComponentUpdate would prevent weirdness. If it's batched, however, with rAF (as Om is, if I'm not mistaken), then it would matter. A possible use case that highlights the issue when using rAF is below:

var structure = immstruct({
   a: { "one": "...", "two": "..." },
   b: { "one": "...", "two": "..." }
})

var component1 = component(function({ cursor }) {
    return <div>{cursor.deref()}</div>
})

var component2 = component([forceUpdateOnChange(structure.cursor(["b", "one"]))], function({cursor}) {
    return <div>{cursor.deref()}, {this.refcursors[0].deref()}</div>
})

var appComponent = component(function({cursor}) {
    return (<div>
        <component1 cursor=cursor.get("a") />
        <component2 cursor=cursor.get("a") />
    </div>)
})

function render() {
    React.render(<appComponent.jsx cursor={structure.cursor()} />, document.body)
}

structure.on('next-animation-frame', function(newData, oldData, path) {
    render()
    refCursorPass(path)  //A mythical method that doesn't exist yet, but that would loop through registered cursors and call force update on any paths that have partial matches to the updated path
});

structure.cursor().update(['a', 'one'], function(data) {
    return "someNewData";
})

structure.cursor().update('b', function(data) {
    return data.set("one", "someOtherNewData")
})

If rAF was happening, component2 would update twice in this scenario. Once from the first update (which causes an update because of the passed in cursor) and once from the second update, because of the cursor passed to the "forceUpdate" mixin.

[skevy]

At this point, I'm not sure if I'm making sense anymore. So if I'm not, please let me know. :-p

[skevy]

And @mikaelbr, it DOES indeed bind a specific component to a piece of data - that would be the point. As @swannodette says, it's not that you would ALWAYS want to do this, but that sometimes it's better for the overall application design.

[mikaelbr]

Yeah, absolutely, I get that. But my point is (I was trying to convey – poorly), it could seem as if this would be the exception not the "render norm", and thus, maybe the performance loss isn't too much of an issue.

[mikaelbr]

Something like refCursorPass(path) could work as well. We can easily have an overview of components in a omniscient instance, so it would be something like: component.refCursorPass(path). I still think you can also maybe do the same thing as a mixin, avoiding refCursorPass(path). I think @torgeir is trying out some different approaches now?

[skevy]

@mikaelbr Ok. Well, either one works for me to be perfectly honest.

So, if we do a ref cursor pass, we need a place to register an object with keys of paths and values of components that our bound to reference cursors. Where do you guys think we could store that?

If we do the mixin approach (which honestly I think I like better), we could have the component itself, on componentWillMount, listen to paths in the structure (that are specified through mixin arguments), and then forceUpdate on itself. But that would require a change to immstruct, am I right?

[mikaelbr]

@skevy the approach you talk about with componentWillMount is the one we plan on trying out, and what I've thought of as the most feasable solutions. As far as I can tell it would require immstruct to have this : omniscientjs/immstruct#18 but for demonstrative purposes it is possible to just do an if check in an on swap listener. I really think doing this as a mixin would be the path of least surprises, and require the least amount of work, for the end user.

[skevy]

100% agree. Do you guys want me to implement or do you want to handle?

[mikaelbr]

I think @torgeir is half way through implementing it already, but as it's late evening here he went to bed and planned on continuing tomorrow. But we're always accepting suggestions!

[skevy]

Alright. Sounds good. Looking forward to seeing what you guys come up with! And I'm very excited to use it. Haha

[dashed]

I'm really liking the conversation on this, and I've been lightly experimenting on stuff with regards to pretty much the concerns @skevy had.

A mixin in tandem with a wrapper around React.createClass (like omniscient) would be very feasible.

Here's I would do it. I haven't tested this, but I modified it from existing code that instead use store semantics (b/c I'm tired of thinking of trees :P). It requires a library that I call immstructor, which wraps around immstruct by providing some useful utilities: https://gist.github.com/Dashed/e38d4184bc239268737e
No official repo for it yet, but still WIP; hopefully it's useful for you.

let shouldComponentUpdate = require('omniscient').shouldComponentUpdate;

let updateMixin = {
   shouldComponentUpdate: shouldComponentUpdate
};

let alwaysRenderMixin = {
   shouldComponentUpdate: () => true
};

let structureMixin = {

    contextTypes: {
        // TODO: eventually immstruct should be instantiable, so it can be passed
        //       as a context object. May also be a map of structure instances.
        immstruct: React.PropTypes.object
    },

    structure: function(name, path) {
        let structure = this.context.immstruct(name);

        if(!path)
            return structure;

        return structure.cursor(path);
    },

    listenToCursor: function(structure, path, callback) {
        immstructor.listenTo(structure, path, callback);
    }
};


module.exports = function component(spec, alwaysRender) {

    let _render = spec.render;
    spec.render = function() {
        return _render.call(this, this.props, this.props.statics);
    };

    let mixins = alwaysRender ? [alwaysRenderMixin, structureMixin] : [updateMixin, structureMixin];
    spec.mixins = _.has(spec, 'mixins') ? mixins.concat(spec.mixins) : mixins;

    return React.createClass(spec);
};

[torgeir]

Half way I don't know, although here's an example of what I was thinking ought to work 😃 https://github.com/torgeir/omniscient-testing-issue-42/blob/master/app.js

It does not work exactly as planned though, I was expecting a loop of the following

2015-02-03 16:47:16.665main.entry.js:394 <Clock>: render
2015-02-03 16:47:17.666main.entry.js:394 <Clock>: render
2015-02-03 16:47:18.665main.entry.js:394 <Clock>: render
2015-02-03 16:47:19.662main.entry.js:394 <Clock>: shouldComponentUpdate => false

However we do get an extra render after each shouldComponentUpdate after doing the forceUpdate

2015-02-03 16:47:16.665main.entry.js:394 <Clock>: render
2015-02-03 16:47:17.666main.entry.js:394 <Clock>: render
2015-02-03 16:47:18.665main.entry.js:394 <Clock>: render
2015-02-03 16:47:19.662main.entry.js:394 <Clock>: shouldComponentUpdate => false
2015-02-03 16:47:19.667main.entry.js:394 <Clock>: render                                   <-- 

I didn't have time to track this down more atm, we'll have to investigate, but it might have something to do with this flag? https://github.com/facebook/react/blob/2aeb8a2a6beb00617a4217f7f8284924fa2ad819/src/core/ReactCompositeComponent.js#L581

Edit: This is wrong.

[torgeir]

I was too fast, this is in fact exactly how it is supposed to work; Here's the same flow with answers updating each 3 seconds and the clock ticking each 1 second spaced for clarity

Considerations:

• Each forced clock update will also trigger render of Anwers as it touches the quiz, which on swap calls render() again, but Answers won't re-render as answers didn't change
• Each answer update triggers swap and re-renders Answers as it's cursor have changed. The clock however does not actually re-render, as it's passed cursors (none) have not changed.

2015-02-03 21:36:37.797main.entry.js:112 updating clock
2015-02-03 21:36:37.797main.entry.js:423 <Answers>: shouldComponentUpdate => false
2015-02-03 21:36:37.801main.entry.js:423 <Clock>: render

2015-02-03 21:36:38.794main.entry.js:112 updating clock
2015-02-03 21:36:38.796main.entry.js:423 <Answers>: shouldComponentUpdate => false
2015-02-03 21:36:38.799main.entry.js:423 <Clock>: render

2015-02-03 21:36:39.791main.entry.js:105 updating answers
2015-02-03 21:36:39.793main.entry.js:423 <Answers>: shouldComponentUpdate => true (cursors have changed)
2015-02-03 21:36:39.794main.entry.js:423 <Answers>: render
2015-02-03 21:36:39.796main.entry.js:423 <Clock>: shouldComponentUpdate => false

2015-02-03 21:36:39.802main.entry.js:112 updating clock
2015-02-03 21:36:39.803main.entry.js:423 <Answers>: shouldComponentUpdate => false
2015-02-03 21:36:39.805main.entry.js:423 <Clock>: render

2015-02-03 21:36:40.803main.entry.js:112 updating clock
2015-02-03 21:36:40.804main.entry.js:423 <Answers>: shouldComponentUpdate => false
2015-02-03 21:36:40.806main.entry.js:423 <Clock>: render

2015-02-03 21:36:41.805main.entry.js:112 updating clock
2015-02-03 21:36:41.806main.entry.js:423 <Answers>: shouldComponentUpdate => false
2015-02-03 21:36:41.808main.entry.js:423 <Clock>: render

2015-02-03 21:36:42.792main.entry.js:105 updating answers
2015-02-03 21:36:42.794main.entry.js:423 <Answers>: shouldComponentUpdate => true (cursors have changed)
2015-02-03 21:36:42.795main.entry.js:423 <Answers>: render
2015-02-03 21:36:42.797main.entry.js:423 <Clock>: shouldComponentUpdate => false

2015-02-03 21:36:42.806main.entry.js:112 updating clock
2015-02-03 21:36:42.807main.entry.js:423 <Answers>: shouldComponentUpdate => false
2015-02-03 21:36:42.808main.entry.js:423 <Clock>: render

[dashed]

@torgeir Interesting. Have you tried updating both clock and answers at the same time? Pretty much like this: quiz.cursor().update(()=> Immutable.fromJS( newMap ));

[torgeir]

That ought to work pretty much the same, except the listener for structure.on('swap', .. would not be given the path clock. Changing this so it also forces renders when the whole strucutre changes you'd get this:

let answers = [];
setInterval(() => {
  answers.push(Math.random());
  quiz.cursor().update(() => Immutable.fromJS({ answers: answers, clock: new Date() }));
}, 10000);

2015-02-04 07:15:24.557main.entry.js:5302 <Answers>: shouldComponentUpdate => true (cursors have changed)
2015-02-04 07:15:24.558main.entry.js:5302 <Answers>: render
2015-02-04 07:15:24.561main.entry.js:5302 <Clock>: shouldComponentUpdate => false
2015-02-04 07:15:24.564main.entry.js:5302 <Clock>: render

2015-02-04 07:15:35.324main.entry.js:5302 <Answers>: shouldComponentUpdate => true (cursors have changed)
2015-02-04 07:15:35.325main.entry.js:5302 <Answers>: render
2015-02-04 07:15:35.329main.entry.js:5302 <Clock>: shouldComponentUpdate => false
2015-02-04 07:15:35.333main.entry.js:5302 <Clock>: render

2015-02-04 07:15:46.230main.entry.js:5302 <Answers>: shouldComponentUpdate => true (cursors have changed)
2015-02-04 07:15:46.231main.entry.js:5302 <Answers>: render
2015-02-04 07:15:46.233main.entry.js:5302 <Clock>: shouldComponentUpdate => false
2015-02-04 07:15:46.234main.entry.js:5302 <Clock>: render

[mikaelbr]

So here is what we need to make this work in a more stable / usable way:

1. A way to always append behaviour to componentWillMount and componentDidUnmount. Either its AOP/decoration. We need a reliable way to add the behaviour, without replacing existing behaviour.
2. Immstruct must have this: Listen for changes to specific paths immstruct#18 and support many listeners.
3. Inject cursor to component in some way, to prevent referring to the path twice (as in https://github.com/torgeir/omniscient-testing-issue-42/blob/master/app.js#L23-L25)

I think this should probably be its own module as it has a hard dependency to immstruct. This might require Omniscient to have some sort of extended mixin support, or "plugin" function system. We should look into how to make this as simple as possible and natural for the end user.

[torgeir]

1. A way to always append behaviour to componentWillMount and componentDidUnmount.

https://github.com/omniscientjs/omniscient-mixins could need some love, utils for joining mixins with before, after, around semantics etc. should probably go here

1. Inject cursor to component in some way, to prevent referring to the path twice

Listening to an existing cursor and not a path in side a structure would probably be better, but then the cursor would need to be updated on each render, no?

Edit: I'd enjoy

let Clock = component('Clock', [ forceUpdateOn(clockCursor) ], () =>
  div({},
    String(clockCursor.deref())));

[mikaelbr]

https://github.com/omniscientjs/omniscient-mixins could need some love, utils for joining mixins with before, after, around semantics etc. should probably go here

Indeed, but the difficult part is that we can't have knowledge if it exists other mixins or not, or the order of the possibly existing mixins – within a mixin. If that makes any sense? So it seems to me that there needs to be some kind of logic in omniscient.

[dashed]

@torgeir Listening to a cursor as an alternative to a keyPath is possible. However, the maintainer of immutable has different opinions of exposing this keyPath publically. Though, I could use help in arguing for it here immutable-js/immutable-js#242

[torgeir]

@mikaelbr I'm thinking this should be left to the user, but we could provide the tools to make it easily possible.

[torgeir]

@dashed I'll see what I can do, should probably come up with a decent example, maybe relate it to the reference-cursors of Om

[skevy]

I think these hooks need to live on componentDidMount instead of componentWillMount.

I ran into a scenario (in server-side rendering) where when this listener was added in componentWillMount, the app state changed between binding that handler in componentWillMount and the component actually being mounted. Something to think about.

[skevy]

Also, I'm a bit confused by this:

A way to always append behaviour to componentWillMount and componentDidUnmount. Either its AOP/decoration. We need a reliable way to add the behaviour, without replacing existing behaviour.

Doesn't React guarantee that lifecycle methods from all mixins are going to be called?

From the React docs:

A nice feature of mixins is that if a component is using multiple mixins and several mixins define the same lifecycle method (i.e. several mixins want to do some cleanup when the component is destroyed), all of the lifecycle methods are guaranteed to be called. Methods defined on mixins run in the order mixins were listed, followed by a method call on the component.

[torgeir]

Oh, that I didn't know! Certainly makes it less applicable for being part of omniscient core. Some mixin utils are probably handy anyways, though, sometimes you want to ensure the order of calls among several mixins.

[skevy]

Sure, but I don't think it matters in this case, FWIW.

[skevy]

Sorry about closing...hit the wrong button! :-p

[dashed]

If you really want to ensure order, there's something like this:

https://github.com/brigand/react-mixin
https://github.com/brigand/smart-mixin

[mikaelbr]

Hm. That's actually pretty interesting. This means very much less complexity for the reference cursor implementation. There's no need to ensure order for this mixin.

[skevy]

@mikaelbr Yah there shouldn't be any need at all. I agree.

[torgeir]

This might be of interest omniscientjs/immstruct#30 (work in progres)

[dashed]

This is as much as an immstruct issue, rather an omniscient issue. I'm not sure what can be amended to omniscient to assist with this kind of feature.

[mikaelbr]

As @dashed notes, this is no longer a Omniscient issue. This can be achieved with omniscientjs/immstruct#30 and https://github.com/omniscientjs/omniscient-mixins/blob/master/mixins/forceUpdateOn.js