Handling structures within components

[natew]

Not sure what to title this one. I have the following setup, using react-router's asyncProps branch which passes in async fetched data to components as props:

var ArticlePage = React.createClass({
  statics: {
    getAsyncProps: (params) => GetStores(params, ['article'])
  },

  getInitialState: () => ({ version: 0 }),

  componentWillReceiveProps(nextProps) {
    if (nextProps.article) {
      this.structure = Immstruct({ article: nextProps.article[0].data });
      this.structure.on('next-animation-frame', () => {
        this.setState({ version: ++this.state.version });
      });
    }
  },

  render() {
    if (!this.structure) return <span />;
    var article = window.articleCursor = this.structure.cursor().get('article');
    return ArticleComponent(`AP-${article.get('id')}-${this.state.version}`, article);
  }
});

module.exports = Component('Article', article => {

  //... render comments

});

var Comment = Component('Comment', function(cursor) {
  var comment = cursor.data;

  var toggleOpened = (e) => {
    e.stopPropagation();
    comment.update('closed', closed => !closed);
  };

  var classes = { closed: comment.get('closed') };

  return (
    <div className={cx(classes)} onClick={toggleOpened}>
      // ... comment content
    </div>
  );
});

Now, using the version state in ArticlePage, when the comment is toggled the entire ArticlePage HTML is swapped out on the page (rather than just the individual comment). BUT, if I remove the version tracking in ArticlePage, nothing is updated on page when I toggle comments. It seems like it needs the key to be unique in order to re-render.

I've tried a few different things here but am stuck... what is the right way to have just the single comment HTML update and not have the entire ArticlePage HTML get swapped out by React?

[mikaelbr]

Initial thought here is that by doing setState() in ArticlePage will trigger a re-render as that is how React works, and thus the render-function is executed and Article should also get shouldComponentUpdate as the cursor has changed (a sub-part of the cursor that is – as Comment is a part of that cursor), but it "probably" won't re-render to the DOM as the render-output hasn't changed.

I think the easiest way for us to check this out is if you could create something like jsfiddle.net or similar, so we could see what is happening.

Also, while Omniscient don't encourage using setState as this can lead to "impure" components, we are quite pragmatic and see that in some cases setState could make sense.

[torgeir]

Yes, this.setState changes the state which ought to trigger a possible re-render of whole subtree of components down from ArticlePage. Before each of the subcomponents are possibly-re-rendered however, a shouldComponentUpdate call will prevent Articles and Comments whose data have not changed from actually having their render methods called (so even the whole react-virtual-DOM-diffing-thing can be skipped). Only your updated Comment whose data is actually changed should have its render called.

You can track down these chains of events by enabling debug logging with omniscient calling

var component = require('omniscient');
component.debug();

(By render I mean React's "render of the virtual in memory DOM", which is then diffed, to make the least amount of actual DOM-operations to make your change visible.)

[natew]

@torgeir Yep, thats what I was hoping would happen, but I'm seeing the entire tree get re-rendered. Is that because I'm changing the key on the top level Article component?

@mikaelbr I am running some errands but I will definitely put together a jsfiddle later today unless I can fix this first.

On another note, the debug stuff is pretty cool. Has a lot of potential too, would be cool to be able to filter which are shown (maybe component.debug('comment-*')) as well as see into whats happening with the shouldupdate, though I could see that adding some overhead so maybe not.

[torgeir]

No I don't think so, but its kinda difficult to deduce from the example code. As mentioned a jsbin, fiddle or similar, or a repo with the example we can clone so we can reproduce it would be useful

Regarding the debug stuff, I'm liking the filter idea. Don't think the overhead would be an issue though, as you probably shouldnt be running it with debug once your done debugging :)

[torgeir]

@natew Oh, you're right, the key, that might very well be the reason, yes.

[natew]

@torgeir Problem being, if I take out the key from the top level, nothing else is rendered from below! Anyway, about to head out, I'll have a fiddle this afternoon.

[torgeir]

If you take out the setState, yes, because its the only thing in your example code that actually triggers a re-render. Note that in our other examples we always listen for swap and explicitly call React.renderComponent(c, el) again.

[natew]

Well, setup a demo and it worked perfectly haha :) https://github.com/natew/omniscient-test

So must be coming from something else. Checking it out...

[torgeir]

Noticing that your demo is no longer changing the key https://github.com/natew/omniscient-test/blob/master/index.js#L31, that might well be the issue :)

[mikaelbr]

Yeah. Changing this:

return ArticleComponent(`AP-${article.get('id')}-${this.state.version}`, article);

to this:

return Article(`AP-${article.get('id')}`, article, statics);

Should have made all the difference. A new key = a new element, and should be rendered in the DOM. This is how React diffs the dom

[natew]

Yea, problem being for some reason on the stack I am actually using it doesn't actually update the DOM at all if I don't add the version. But something strange is going on there, just glad I know it's not omniscient causing it now.

[torgeir]

@natew this might be of interest https://github.com/omniscientjs/omniscient/blob/03b951e5d770d74a5c6890adf79722399b09b2d9/component.js#L68-L93 :)

[natew]

Awesome :)