Hacker News Reader

Comment by bm_i | Hacker News Reader

I'd love to see the full source of the render function(s) here, we may not be seeing the full story... Are the React children having appropriate `key` attributes set as the list increases in size? etc

View on HN

I spoke with Paul, he is using `key` and said he is following best practice as per the docs. That being said, we do want the code to be open, we just have a rather heavy OSS process that can take a while to get it out (it's a nut ache at the moment).

Is he using shouldComponentUpdate() [1] to avoid diffing existing photos when new ones are added?

[1] https://facebook.github.io/react/docs/advanced-performance.h...

It seems like the end result of React's diffing algorithm is going to be an appendChild. Exactly what is happening in the vanilla JavaScript. But I don't think the diffing algorithm itself is the reason for the time disparity but rather React's rebuilding of the tree to diff against the DOM. I agree with you, shouldComponentUpdate() should massively speed up React.

Right, with lots of elements, React still needs to render and diff each component which for a lot of elements causes a lot of garbage and diffing work. ShouldComponentUpdate / PureRenderMixin avoids this altogether when props/state isn't changed so this gives a significant boost with lots of elements.

Because of the somewhat pathological benchmark design, he could probably implement a component for each picture and set shouldComponentUpdate to simply return false. With proper keys, I have a feeling the React performance would improve dramatically, perhaps as much or more as the pureRender mixin.

PureRenderMixin is basically shouldComponentUpdate with a shallow comparison of previous/next props and state, but the big win is avoiding render and diff for the component, so returning false would be about the same as PureRenderMixin.

This has been added to the end of the post:

> Did you set shouldComponentUpdate to false? Yeah that would definitely improve matters here, unless I need to update something like a “last updated” message on a per-photo basis. It seems to then become a case of planning your components for React, which is fair enough, but then I feel like that wouldn’t be any different if you were planning for Vanilla DOM updates, either. It’s the same thing: you always need to plan for your architecture.

The Vanilla DOM equivalent is just not writing any update code until you need it. I'd argue that adding...

  shouldComponentUpdate() { return false }

...is the equivalent React way of saying "this will never be updated", but the default behaviour is the opposite of Vanilla DOM because React automatically handles updates for you, regardless of when you realised you were going to need them.

The "last updated" scenario would be a much more interesting comparison with Vanilla DOM, as a React component's lifecycle gives you an obvious place to set up/tear down the necessary timeouts on an individual basis and updates happen only when they need to. I was pleasantly surprised by how trivial it was to make _every_ time/date displayed in my Hacker News clone live-update, even down to the per-second level: https://github.com/insin/react-hn/commit/08893a046b5289b07ef...

Maybe I'm not following, but: if this was a contrived example -- i.e. written specifically for the purpose of testing an idea, and not a result of internal evaluation where there are proprietary bits -- then what does "your" (I don't know who you are) OSS process have to do with it?

Presumably the process applies to releasing code written during work hours full stop.

This.

Can't release code until it goes through open sourcing process. It is pain that's all. Code will be shared.

Reply on news.ycombinator.com