3 ms·
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 ra
by kinlan 11y ago
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).
- insin 11y agoIs he using shouldComponentUpdate() [1] to avoid diffing existing photos when new ones are added? [1] https://facebook.github.io/react/docs/advanced-performance.html#avoiding-reconciling-the-dom https://facebook.github.io/react/docs/advanced-performance.h...
- megaman821 11y agoIt 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.
- d4n3 11y agoRight, 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.
- latortuga 11y agoBecause 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.
- d4n3 11y agoPureRenderMixin 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.
- insin 11y agoThis 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/08893a046b5289b07efa539377dacf0d7e91b160 https://github.com/insin/react-hn/commit/08893a046b5289b07ef...
- tobz 11y agoMaybe 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?