Skip to content

Conversation

@MrFlashAccount
Copy link
Contributor

@MrFlashAccount MrFlashAccount commented Dec 1, 2025

This PR removes the unnecessary memo wrapper, that adds overhead by doubling the comparison of the props:

  1. Inside the memo func
  2. Inside the React component

Note: rerendering of the individual rows is fine - if nothing changed, React will immediately return with the cached result

@MrFlashAccount MrFlashAccount marked this pull request as ready for review December 1, 2025 14:18
@krausest krausest merged commit ca2c258 into krausest:master Dec 14, 2025
@krausest
Copy link
Owner

Sorry, seems like select row is hit badly by your PR... (left new results, right old results). The other benchmarks are slightly faster, but select is significantly worse.

I suggest rolling back the change. Ok?

Screenshot 2025-12-15 at 07 39 51

@MrFlashAccount
Copy link
Contributor Author

MrFlashAccount commented Dec 15, 2025

@krausest that's strange. At least on my pc, the difference is negligible. If you don't mind, I'd like to ping the react core team according the need to use manual memo for list items.

UPD: at least in docs, if you already using the compiler, you don't need any extra memoization on top of it (except a few edge-cases):

https://react.dev/learn/react-compiler/introduction#after-react-compiler

UPD2: reproduced on my machine with a very large slowdown.

@MrFlashAccount
Copy link
Contributor Author

@krausest waiting for a confirmation from the core team. I think, in case the memo here is not needed with the compiler (as per docs), we can ignore the perf degradation, as it's intended. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants