-
-
Notifications
You must be signed in to change notification settings - Fork 650
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow rerender test #2796
base: main
Are you sure you want to change the base?
Allow rerender test #2796
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit e824ccd. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
|
||
export const MatchInner = React.memo(function MatchInnerImpl({ | ||
export const MatchInner = function MatchInnerImpl({ | ||
matchId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we cannot remove these memos as it kills a lot of render optimizations (as you saw you had to change the render count test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please find a way that does not remove the memos
This changes so Tanstack router will support rerenders in tests using the renderHooks from testing library.
import { renderHook } from '@testing-library/react'
We heavily depend on the rerender tests to ensure no state changes are done in hooks, to avoid infinite rerender issues that are really hard to find and fix, so would really appriciate getting this issue fixed, I hope this solution is sufficient.