forked from facebook/react
-
Notifications
You must be signed in to change notification settings - Fork 9
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
DevTools: Add post-commit hook (facebook#21183)
I recently added UI for the Profiler's commit and post-commit durations to the DevTools, but I made two pretty silly oversights:
1. I used the commit hook (called after mutation+layout effects) to read both the layout and passive effect durations. This is silly because passive effects may not have flushed yet git at this point.
2. I didn't reset the values on the HostRoot node, so they accumulated with each commit.
This commitR addresses both issues:
1. First it adds a new DevTools hook, onPostCommitRoot*, to be called after passive effects get flushed. This gives DevTools the opportunity to read passive effect durations (if the build of React being profiled supports it).
2. Second the work loop resets these durations (on the HostRoot) after calling the post-commit hook so address the accumulation problem.
I've also added a unit test to guard against this regressing in the future.
* Doing this in flushPassiveEffectsImpl seemed simplest, since there are so many places we flush passive effects. Is there any potential problem with this though?- Loading branch information
Showing
11 changed files
with
275 additions
and
43 deletions.
There are no files selected for viewing
144 changes: 144 additions & 0 deletions
144
packages/react-devtools-shared/src/__tests__/profilingHostRoot-test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| /** | ||
| * Copyright (c) Facebook, Inc. and its affiliates. | ||
| * | ||
| * This source code is licensed under the MIT license found in the | ||
| * LICENSE file in the root directory of this source tree. | ||
| * | ||
| * @flow | ||
| */ | ||
|
|
||
| describe('profiling HostRoot', () => { | ||
| let React; | ||
| let ReactDOM; | ||
| let Scheduler; | ||
| let store: Store; | ||
| let utils; | ||
| let getEffectDurations; | ||
|
|
||
| let effectDurations; | ||
| let passiveEffectDurations; | ||
|
|
||
| beforeEach(() => { | ||
| utils = require('./utils'); | ||
| utils.beforeEachProfiling(); | ||
|
|
||
| getEffectDurations = require('../backend/utils').getEffectDurations; | ||
|
|
||
| store = global.store; | ||
|
|
||
| React = require('react'); | ||
| ReactDOM = require('react-dom'); | ||
| Scheduler = require('scheduler'); | ||
|
|
||
| effectDurations = []; | ||
| passiveEffectDurations = []; | ||
|
|
||
| // This is the DevTools hook installed by the env.beforEach() | ||
| // The hook is installed as a read-only property on the window, | ||
| // so for our test purposes we can just override the commit hook. | ||
| const hook = global.__REACT_DEVTOOLS_GLOBAL_HOOK__; | ||
| hook.onPostCommitFiberRoot = function onPostCommitFiberRoot( | ||
| rendererID, | ||
| root, | ||
| ) { | ||
| const {effectDuration, passiveEffectDuration} = getEffectDurations(root); | ||
| effectDurations.push(effectDuration); | ||
| passiveEffectDurations.push(passiveEffectDuration); | ||
| }; | ||
| }); | ||
|
|
||
| it('should expose passive and layout effect durations for render()', () => { | ||
| function App() { | ||
| React.useEffect(() => { | ||
| Scheduler.unstable_advanceTime(10); | ||
| }); | ||
| React.useLayoutEffect(() => { | ||
| Scheduler.unstable_advanceTime(100); | ||
| }); | ||
| return null; | ||
| } | ||
|
|
||
| utils.act(() => store.profilerStore.startProfiling()); | ||
| utils.act(() => { | ||
| const container = document.createElement('div'); | ||
| ReactDOM.render(<App />, container); | ||
| }); | ||
| utils.act(() => store.profilerStore.stopProfiling()); | ||
|
|
||
| expect(effectDurations).toHaveLength(1); | ||
| const effectDuration = effectDurations[0]; | ||
| expect(effectDuration === null || effectDuration === 100).toBe(true); | ||
| expect(passiveEffectDurations).toHaveLength(1); | ||
| const passiveEffectDuration = passiveEffectDurations[0]; | ||
| expect(passiveEffectDuration === null || passiveEffectDuration === 10).toBe( | ||
| true, | ||
| ); | ||
| }); | ||
|
|
||
| it('should expose passive and layout effect durations for createRoot()', () => { | ||
| function App() { | ||
| React.useEffect(() => { | ||
| Scheduler.unstable_advanceTime(10); | ||
| }); | ||
| React.useLayoutEffect(() => { | ||
| Scheduler.unstable_advanceTime(100); | ||
| }); | ||
| return null; | ||
| } | ||
|
|
||
| utils.act(() => store.profilerStore.startProfiling()); | ||
| utils.act(() => { | ||
| const container = document.createElement('div'); | ||
| const root = ReactDOM.unstable_createRoot(container); | ||
| root.render(<App />); | ||
| }); | ||
| utils.act(() => store.profilerStore.stopProfiling()); | ||
|
|
||
| expect(effectDurations).toHaveLength(1); | ||
| const effectDuration = effectDurations[0]; | ||
| expect(effectDuration === null || effectDuration === 100).toBe(true); | ||
| expect(passiveEffectDurations).toHaveLength(1); | ||
| const passiveEffectDuration = passiveEffectDurations[0]; | ||
| expect(passiveEffectDuration === null || passiveEffectDuration === 10).toBe( | ||
| true, | ||
| ); | ||
| }); | ||
|
|
||
| it('should properly reset passive and layout effect durations between commits', () => { | ||
| function App({shouldCascade}) { | ||
| const [, setState] = React.useState(false); | ||
| React.useEffect(() => { | ||
| Scheduler.unstable_advanceTime(10); | ||
| }); | ||
| React.useLayoutEffect(() => { | ||
| Scheduler.unstable_advanceTime(100); | ||
| }); | ||
| React.useLayoutEffect(() => { | ||
| if (shouldCascade) { | ||
| setState(true); | ||
| } | ||
| }, [shouldCascade]); | ||
| return null; | ||
| } | ||
|
|
||
| const container = document.createElement('div'); | ||
| const root = ReactDOM.unstable_createRoot(container); | ||
|
|
||
| utils.act(() => store.profilerStore.startProfiling()); | ||
| utils.act(() => root.render(<App />)); | ||
| utils.act(() => root.render(<App shouldCascade={true} />)); | ||
| utils.act(() => store.profilerStore.stopProfiling()); | ||
|
|
||
| expect(effectDurations).toHaveLength(3); | ||
| expect(passiveEffectDurations).toHaveLength(3); | ||
|
|
||
| for (let i = 0; i < effectDurations.length; i++) { | ||
| const effectDuration = effectDurations[i]; | ||
| expect(effectDuration === null || effectDuration === 100).toBe(true); | ||
| const passiveEffectDuration = passiveEffectDurations[i]; | ||
| expect( | ||
| passiveEffectDuration === null || passiveEffectDuration === 10, | ||
| ).toBe(true); | ||
| } | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.