Speedometer icon indicating copy to clipboard operation
Speedometer copied to clipboard

NewsSite-Next workload is generating keys for <li> element on the fly

Open lingyuncai opened this issue 1 year ago • 3 comments

Since the data doesn't contain an id field for each item, the workload is now generating keys for each <li> element dynamically while rendering, using uuidv4(). https://github.com/WebKit/Speedometer/blob/3150f4117389931ba36871c4a0ab4e289d273965/resources/newssite/news-next/src/components/article/article-content.jsx#L23-L32

This will cause keys to never match up between renders, leading to these elements being recreated every time, which defeats the purpose of keys and slows the performance [1].

Besides, this implementation seems inconsistent with that of NewsSite-Nuxt, which uses item.id (which is undefined) as the key binding directly. https://github.com/WebKit/Speedometer/blob/3150f4117389931ba36871c4a0ab4e289d273965/resources/newssite/news-nuxt/components/atoms/ArticleContent.vue#L29-L34

[1] https://react.dev/learn/rendering-lists#rules-of-keys

lingyuncai avatar Aug 29 '24 02:08 lingyuncai

PR to address this issue: #423

lingyuncai avatar Aug 29 '24 02:08 lingyuncai

@lingyuncai - thanks for the issue and your solution. I opened a separate pr, where I added the unique ids to the actual data files. In that way we don't even need to generate them at any point in the app.

https://github.com/WebKit/Speedometer/pull/426

flashdesignory avatar Aug 31 '24 11:08 flashdesignory

Hi, @flashdesignory

Thanks for your new PRs, they indeed solve this issue in a more clean way and it looks great!

Once these PRs are merged, do you know which version of Speedometer will include these changes?

lingyuncai avatar Sep 02 '24 01:09 lingyuncai

completed with https://github.com/WebKit/Speedometer/pull/447

flashdesignory avatar Jan 29 '25 20:01 flashdesignory