From d6f6b951e12e25b67f681094714a337356d2a310 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 13 Sep 2019 08:59:57 -0700 Subject: [PATCH] Support disabling interaction tracing for suspense promises (#16776) * Support disabling interaction tracing for suspense promises If a thrown Promise has the __reactDoNotTraceInteractions attribute, React will not wrapped its callbacks to continue tracing any current interaction(s). * Added optional '__reactDoNotTraceInteractions' attribute to Flow Thenable type --- .../src/ReactFiberCommitWork.js | 4 +- .../react-reconciler/src/ReactFiberThrow.js | 4 +- .../src/ReactFiberWorkLoop.js | 3 + .../__tests__/ReactProfiler-test.internal.js | 85 +++++++++++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index ffce3166eb40..0e31e58b6467 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -1512,7 +1512,9 @@ function attachSuspenseRetryListeners(finishedWork: Fiber) { let retry = resolveRetryThenable.bind(null, finishedWork, thenable); if (!retryCache.has(thenable)) { if (enableSchedulerTracing) { - retry = Schedule_tracing_wrap(retry); + if (thenable.__reactDoNotTraceInteractions !== true) { + retry = Schedule_tracing_wrap(retry); + } } retryCache.add(thenable); thenable.then(retry, retry); diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index 0e2d869b729a..b739f4056c45 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -174,7 +174,9 @@ function attachPingListener( renderExpirationTime, ); if (enableSchedulerTracing) { - ping = Schedule_tracing_wrap(ping); + if (thenable.__reactDoNotTraceInteractions !== true) { + ping = Schedule_tracing_wrap(ping); + } } thenable.then(ping, ping); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index cfa2f3e0e959..2542dcc107ec 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -204,6 +204,9 @@ const RootCompleted = 4; export type Thenable = { then(resolve: () => mixed, reject?: () => mixed): Thenable | void, + + // Special flag to opt out of tracing interactions across a Suspense boundary. + __reactDoNotTraceInteractions?: boolean, }; // Describes where we are in the React execution stack diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index ce5211dff2e4..5e962bb4702e 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -2729,6 +2729,91 @@ describe('Profiler', () => { onInteractionScheduledWorkCompleted.mock.calls[1][0], ).toMatchInteraction(highPriUpdateInteraction); }); + + it('does not trace Promises flagged with __reactDoNotTraceInteractions', async () => { + loadModulesForTracing({useNoopRenderer: true}); + + const interaction = { + id: 0, + name: 'initial render', + timestamp: Scheduler.unstable_now(), + }; + + AsyncText = ({ms, text}) => { + try { + TextResource.read([text, ms]); + Scheduler.unstable_yieldValue(`AsyncText [${text}]`); + return text; + } catch (promise) { + promise.__reactDoNotTraceInteractions = true; + + if (typeof promise.then === 'function') { + Scheduler.unstable_yieldValue(`Suspend [${text}]`); + } else { + Scheduler.unstable_yieldValue(`Error [${text}]`); + } + throw promise; + } + }; + + const onRender = jest.fn(); + SchedulerTracing.unstable_trace( + interaction.name, + Scheduler.unstable_now(), + () => { + ReactNoop.render( + + }> + + + + , + ); + }, + ); + + expect(onInteractionTraced).toHaveBeenCalledTimes(1); + expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction( + interaction, + ); + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); + expect(getWorkForReactThreads(onWorkStarted)).toHaveLength(0); + expect(getWorkForReactThreads(onWorkStopped)).toHaveLength(0); + + expect(Scheduler).toFlushAndYield([ + 'Suspend [Async]', + 'Text [Loading...]', + 'Text [Sync]', + ]); + // Should have committed the placeholder. + expect(ReactNoop.getChildrenAsJSX()).toEqual('Loading...Sync'); + expect(onRender).toHaveBeenCalledTimes(1); + + let call = onRender.mock.calls[0]; + expect(call[0]).toEqual('test-profiler'); + expect(call[6]).toMatchInteractions( + ReactFeatureFlags.enableSchedulerTracing ? [interaction] : [], + ); + + // The interaction is now complete. + expect(onInteractionTraced).toHaveBeenCalledTimes(1); + expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1); + expect( + onInteractionScheduledWorkCompleted, + ).toHaveBeenLastNotifiedOfInteraction(interaction); + + // Once the promise resolves, we render the suspended view + await awaitableAdvanceTimers(20000); + expect(Scheduler).toHaveYielded(['Promise resolved [Async]']); + expect(Scheduler).toFlushAndYield(['AsyncText [Async]']); + expect(ReactNoop.getChildrenAsJSX()).toEqual('AsyncSync'); + expect(onRender).toHaveBeenCalledTimes(2); + + // No interactions should be associated with this update. + call = onRender.mock.calls[1]; + expect(call[0]).toEqual('test-profiler'); + expect(call[6]).toMatchInteractions([]); + }); }); }); });