-
Notifications
You must be signed in to change notification settings - Fork 48.3k
Run Component Track Logs in the console.createTask() of the Fiber #32809
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
base: main
Are you sure you want to change the base?
Conversation
@@ -3596,7 +3596,7 @@ function dispatchSetState<S, A>( | |||
lane, | |||
); | |||
if (didScheduleUpdate) { | |||
startUpdateTimerByLane(lane); | |||
startUpdateTimerByLane(lane, 'setState()'); |
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.
omg yes
1085e97
to
7ed6610
Compare
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.
That way you can find the callsite of the first setState and therefore the "cause" of a render starting by selecting the "Update" track.
Big.
Can we do the same for render()
? I only know of render()
calls from within components for trivial trees like screen reader announcements so maybe not that important.
Is it possible to attach an Owner Stack to changes in sync external Stores?
LANES_TRACK_GROUP, | ||
'primary-dark', | ||
); | ||
// TODO: Ideally this would use the debugTask of the startTransition call perhaps. |
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.
Right now it's using the one from the setState
call that was wrapped in startTransition
, right?
If I had to choose, I'd usually prefer the deeper one. Does createTask
include the async stack? If not, you could set a breakpoint at the setState
and find the corresponding startTransition
that way if it's async. If the transition function is sync, having the deeper setState
task is already perfect IMO.
The task for startTransition
could leave you hanging if the transition function has multiple setState
(maybe even in separate if-else).
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.
Yea. I was thinking about maybe starting to log the Event and Action entries inside the setState
call itself instead of waiting for the render to start. That way it would have the stack trace of the setState
even in the profiling builds which would not have createTask
enabled if the console.timeStamp
call itself had it natively. The downside is that the "Update" track would not have it since it gets logged at the end time.
However, we can also just use createTask
in profiling builds just for this case since it's just once per setState
and cheap when DevTools is not open.
@@ -1861,6 +1863,7 @@ function subscribeToStore<T>( | |||
// read from the store. | |||
if (checkIfSnapshotChanged(inst)) { | |||
// Force a re-render. | |||
startUpdateTimerByLane(SyncLane, 'updateSyncExternalStore()'); |
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.
Nice
08c00c7
to
f30c5a6
Compare
f30c5a6
to
748ad35
Compare
Except for errors in DEV where we use the properties extension.
If we use the same start/end time we get the same effect.
…Task This ensures that we can get the owner stack of the currently rendering component when it emits its profiling information.
We then run the "Update" entries in that task. That way you can find the callsite of the first setState and therefore the "cause" of a render starting.
Ideally we'd show the whole failed subtree including the errored Component. We have the metrics for it. We just need to stash the Fibers somewhere. In the meantime we just change the stack trace of the error boundary entry.
There's no API called this we just call it callback() but to give some hint.
748ad35
to
973f98f
Compare
Stacked on #32736.
That way you can find the owner stack of each component that rerendered for context.
In addition to the JSX callsite tasks that we already track, I also added tracking of the first
setState
call before rendering.We then run the "Update" entries in that task. That way you can find the callsite of the first setState and therefore the "cause" of a render starting by selecting the "Update" track.
Unfortunately this is blocked on bugs in Chrome that makes it so that these stacks are not reliable in the Performance tab. It basically just doesn't work.