WebKit / Speedometer

An open source repository for the Speedometer benchmark
Other
614 stars 73 forks source link

Avoid key update for every re-render of ArticleContent with the same content #423

Closed lingyuncai closed 2 months ago

lingyuncai commented 2 months ago

This is to address #422.

Use useMemo() to avoid uuidv4() call on the same content, which introduced unnecessary re-renders of the same <li> element.

julienw commented 2 months ago

This looks reasonable to me, but I wonder if the id prop could be generated instead wherever we create the items in the first place. I'd like @flashdesignory to chime in here.

Also you mention in #422 that nuxt has a similar issue where id is undefined. I believe this could be solved the same way.

flashdesignory commented 2 months ago

@julienw - you are correct, the id should be coming from the data itself. Looks like I didn't add an id to the article content.

I'll update the data and open a pr

Thanks @lingyuncai for surfacing the issue

lingyuncai commented 2 months ago

Hi @julienw and @flashdesignory, thanks for the review!

Storing the id prop in locally generated data is indeed a more clean way to achieve the goal, while it also unifies the behavior across the Next/Nuxt workloads.

Thanks again for the effort on this :)

rniwa commented 2 months ago

Is this PR now obsolete given https://github.com/WebKit/Speedometer/pull/426 and https://github.com/WebKit/Speedometer/pull/427?

lingyuncai commented 2 months ago

Yes, closing now.