Avoid gratuitous re-rendering of components using settings or theme #4619
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
If an app has components surrounding PaperProvider that occasionally refresh, then components that use the theme provided by PaperProvider will refresh even if they don't need to. E.g. { animation: { scale: 1 } } changes every time because object equality is violated, even though the theme object contents are the same from render to render.
The theme is unlikely to change often, so memoizing it should not have much memory overhead.
Related issue
n/a
Test plan
An app's appearance should continue to follow theme changes, but the react-devtools profiler should not show components re-rendering because of theme (or settings) changes when there haven't been any, even if a PaperProvider parent is re-rendered.
One can also use a hook like the following to look for changes to a theme fetched with useTheme():
export const useTraceUpdate = (v: any) => {
const prev = React.useRef(v)
React.useEffect(() => {
// Compare v and prev, output differences, etc.
prev.current = v
}, [v])
}
useTraceUpdate(theme)
Passes lint and test