weixin_39949506
weixin_39949506
2020-11-29 18:08

Update keys to match row indices

Improves performance and fixes Android bug by preventing unnecessary unmounting and remounting of rows during scrolling.

When the visible region changes during scrolling, the visible rows are re-rendered with different keys due to the line:


for (let i = 0; i < size; ++i) items.push(itemRenderer(from + i, i));

https://github.com/orgsync/react-list/blob/master/react-list.es6#L444

For example, the second row will render with key={1}. After scrolling so the first row is no longer visible, the second row will rerender with key={0}. This forces React to unmount and remount the row even though the content has not changed.

This causes two problems:

  1. On Chrome Android, when scrolling, if the target node of the touch event disappears, the touch events stop firing. In practice, this means you cannot scroll more than a few pixels on Android before scrolling stops/breaks since the DOM node is replaced with a new one as soon as react-list rerenders.

  2. Unmounting and remounting unnecessarily incurs a large performance penalty when the rows have significant content in them (e.g. other react components that do things when mounted/unmounted). With this fix, the list I'm building runs significantly faster and smoother!

Further notes on the Android bug: - The react-list examples contain rows with only text, which seem to work fine on Chrome, but as soon as you add any child <div>s, this bug appears. - This bug is also reproducible if you use desktop Chrome and enable simulating touch events using the developer tools.

该提问来源于开源项目:caseywebdev/react-list

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

5条回答

  • weixin_39609423 weixin_39609423 5月前

    Feel free to use index or key for your element key, whichever works best for your use case. This change makes it impossible to use the latter.

    点赞 评论 复制链接分享
  • weixin_39949506 weixin_39949506 5月前

    Optionality is nice, but is there ever a good reason to use the current key for your element key?

    Using different keys for the same row: - reduces performance - goes against React best-practices (using the same key when rendering the same element tells react not to do extra work) - causes bugs on Android

    I'd strongly advocate for using index as the key by default. The only reason I didn't propose removing the second parameter entirely is for backwards compatibility.

    点赞 评论 复制链接分享
  • weixin_39949506 weixin_39949506 5月前

    It's probably easy to dismiss this suggestion given that it's a one-line code change, but for some background, I spent several hours debugging this issue on Android, eventually resolving to write my own version (I've done this before in the pre-react world) before stumbling upon this solution. The Android bug is incredibly tricky to resolve!

    Perhaps you can at least consider changing the docs to advise using index as the key, as this could save others some painful hours of debugging and should work for 99% of use cases (can't think of any good use cases for the current behavior)?

    点赞 评论 复制链接分享
  • weixin_39609423 weixin_39609423 5月前

    We use this component in a table with 10k rows and using the key arg is much more performance for us. There are too many factors that play into which is better to use to always recommend one. The best way is to perf test both and take the faster.

    点赞 评论 复制链接分享
  • weixin_39949506 weixin_39949506 5月前

    Solely in the interest of building the best possible list implementation =)

    • Are you able to share an example that shows the performance difference? It would be awesome to get some heuristics (weight of content, lists of certain sizes, desktop versus mobile, viewport size, etc) and provide some recommended testing methods so that people can performance test their own implementation.
    • Can you comment on the android bug?
    点赞 评论 复制链接分享

相关推荐