-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Support disabling interaction tracing for suspense promises #16776
Support disabling interaction tracing for suspense promises #16776
Conversation
If a thrown Promise has the __reactDoNotTraceInteractions attribute, React will not wrapped its callbacks to continue tracing any current interaction(s).
4674984
to
986a614
Compare
@@ -174,7 +174,9 @@ function attachPingListener( | |||
renderExpirationTime, | |||
); | |||
if (enableSchedulerTracing) { | |||
ping = Schedule_tracing_wrap(ping); | |||
if (thenable.__reactDoNotTraceInteractions !== true) { | |||
ping = Schedule_tracing_wrap(ping); |
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.
The ping part is a bit weird because it means that means that if we’re only suspended on this promise for the first commit I believe it will decrement and complete the interaction before we commit.
I think this only happens if this also happened to cause a “Delayed” suspended state (bad loading) since otherwise we don’t suspend the commit and it’ll continue until it commits.
However fixing this is tricky since we’d have to keep track of this promise and call it once we commit to detach.
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.
The ping part is a bit weird because it means that means that if we’re only suspended on this promise for the first commit I believe it will decrement and complete the interaction before we commit.
Hm... I don't think this is the case, but maybe you can point out what I'm overlooking? We only decrement in finishPendingInteractions
, which runs after commit.
Not-wrapping isn't the same as decrementing.
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.
Another way to put it is, if this is ok, then it should be ok to remove this wrapper in all cases.
Because this ping is only relevant when there is an in progress render. If the in progress render keeps its own interactions alive then we never need this.
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.
Hm...yeah, maybe wrapping the ping listener is never necessary.
If a thrown Promise has the
__reactDoNotTraceInteractions
attribute, React will not wrapped its callbacks to continue tracing any current interaction(s).Example usage: