Current behavior for excluding Component render with unchanged props from Components track #34822
Conversation
| }); | ||
|
|
||
| // @gate __DEV__ && enableComponentPerformanceTrack | ||
| it('includes spans for Components with no prop changes', async () => { |
There was a problem hiding this comment.
actually doesn't at the moment but prob should?
github-actions
bot
added
the
React Core Team
| }, | ||
| end: 21000, | ||
| start: 16000, | ||
| }, | ||
| ], | ||
| ]); | ||
| expect(getConsoleTimestampEntries()).toEqual([ | ||
| ['Render', 16000, 31000, 'Blocking', 'Scheduler ⚛', 'primary-dark'], |
There was a problem hiding this comment.
15s in the Scheduler track but only 5s are accounted for (in <Left> where props changed). The other 10s of <Right> are omitted. Ideally they would show with a hint that no props changed and the Component should be memoized. Or refactored since a large amount of time spent in a Component with no props changed hints at some expensive work in its impl that couldn't be memoized.
|
Comparing: ead9218...abffa5c Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
eps1lon
force-pushed
the
sebbie/components-track-unchanged-props
branch
from
1ee4714 to
abffa5c
Compare
|
This is a refactoring bug introduced by #34370 It used to be that we fell through to the By getting rid of the early return we'd have to copy paste the else case to all the ifs. |
hoxyq
left a comment
hoxyq
left a comment
There was a problem hiding this comment.
Oops, sorry about the missing branch!
Are you working on a fix for it, or I could take a look, if you want me to?
That'd be nice. I'm not actively working on a fix. |
4 participants
If we rerender with the same props, the render time will not be accounted for in the Components track. The attached test reproduces the behavior observed in https://codesandbox.io/p/sandbox/patient-fast-j94f2g:
