From dd45378685e1a4c0b98872b96340f3585de9c5f7 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 4 Apr 2022 12:14:00 -0400 Subject: [PATCH] Temporarily disable "long nested update" warning DevTools' timeline profiler warns if an update inside a layout effect results in an expensive re-render. However, it misattributes renders that are spawned from a sync render at lower priority. This affects the new implementation of useDeferredValue but it would also apply to things like Offscreen. It's not obvious to me how to fix this given how DevTools models the idea of a "nested update" so I'm disabling the warning for now to unblock the bugfix for useDeferredValue. --- .../src/__tests__/preprocessData-test.js | 12 +++++++++--- .../src/import-worker/preprocessData.js | 5 ++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/preprocessData-test.js b/packages/react-devtools-shared/src/__tests__/preprocessData-test.js index 72086a684cd41..880607ff1d768 100644 --- a/packages/react-devtools-shared/src/__tests__/preprocessData-test.js +++ b/packages/react-devtools-shared/src/__tests__/preprocessData-test.js @@ -1463,7 +1463,9 @@ describe('Timeline profiler', () => { expect(event.warning).toBe(null); }); - it('should warn about long nested (state) updates during layout effects', async () => { + // This is temporarily disabled because the warning doesn't work + // with useDeferredValue + it.skip('should warn about long nested (state) updates during layout effects', async () => { function Component() { const [didMount, setDidMount] = React.useState(false); Scheduler.unstable_yieldValue( @@ -1523,7 +1525,9 @@ describe('Timeline profiler', () => { ); }); - it('should warn about long nested (forced) updates during layout effects', async () => { + // This is temporarily disabled because the warning doesn't work + // with useDeferredValue + it.skip('should warn about long nested (forced) updates during layout effects', async () => { class Component extends React.Component { _didMount: boolean = false; componentDidMount() { @@ -1654,7 +1658,9 @@ describe('Timeline profiler', () => { }); }); - it('should not warn about deferred value updates scheduled during commit phase', async () => { + // This is temporarily disabled because the warning doesn't work + // with useDeferredValue + it.skip('should not warn about deferred value updates scheduled during commit phase', async () => { function Component() { const [value, setValue] = React.useState(0); const deferredValue = React.useDeferredValue(value); diff --git a/packages/react-devtools-timeline/src/import-worker/preprocessData.js b/packages/react-devtools-timeline/src/import-worker/preprocessData.js index 258eef931a382..110b4e8923fb2 100644 --- a/packages/react-devtools-timeline/src/import-worker/preprocessData.js +++ b/packages/react-devtools-timeline/src/import-worker/preprocessData.js @@ -1124,7 +1124,10 @@ export default async function preprocessData( lane => profilerData.laneToLabelMap.get(lane) === 'Transition', ) ) { - schedulingEvent.warning = WARNING_STRINGS.NESTED_UPDATE; + // FIXME: This warning doesn't account for "nested updates" that are + // spawned by useDeferredValue. Disabling temporarily until we figure + // out the right way to handle this. + // schedulingEvent.warning = WARNING_STRINGS.NESTED_UPDATE; } } });