Skip to content

Commit

Permalink
Automatically Profile roots when DevTools is present (#13058)
Browse files Browse the repository at this point in the history
* react-test-renderer injects itself into DevTools if present
* Fibers are always opted into ProfileMode if DevTools is present
* Added simple test for DevTools + always profiling behavior
  • Loading branch information
bvaughn authored Jun 20, 2018
1 parent ae8c6dd commit b0f6089
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 1 deletion.
11 changes: 10 additions & 1 deletion packages/react-reconciler/src/ReactFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
} from 'shared/ReactTypeOfWork';
import getComponentName from 'shared/getComponentName';

import {isDevToolsPresent} from './ReactFiberDevToolsHook';
import {NoWork} from './ReactFiberExpirationTime';
import {NoContext, AsyncMode, ProfileMode, StrictMode} from './ReactTypeOfMode';
import {
Expand Down Expand Up @@ -345,7 +346,15 @@ export function createWorkInProgress(
}

export function createHostRootFiber(isAsync: boolean): Fiber {
const mode = isAsync ? AsyncMode | StrictMode : NoContext;
let mode = isAsync ? AsyncMode | StrictMode : NoContext;

if (enableProfilerTimer && isDevToolsPresent) {
// Always collect profile timings when DevTools are present.
// This enables DevTools to start capturing timing at any point–
// Without some nodes in the tree having empty base times.
mode |= ProfileMode;
}

return createFiber(HostRoot, null, null, mode);
}

Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiberDevToolsHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ function catchErrors(fn) {
};
}

export const isDevToolsPresent =
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ !== 'undefined';

export function injectInternals(internals: Object): boolean {
if (typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ === 'undefined') {
// No DevTools
Expand Down
11 changes: 11 additions & 0 deletions packages/react-test-renderer/src/ReactTestRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
Profiler,
} from 'shared/ReactTypeOfWork';
import invariant from 'shared/invariant';
import ReactVersion from 'shared/ReactVersion';

import * as ReactTestHostConfig from './ReactTestHostConfig';
import * as TestRendererScheduling from './ReactTestRendererScheduling';
Expand Down Expand Up @@ -510,4 +511,14 @@ const ReactTestRendererFiber = {
unstable_setNowImplementation: TestRendererScheduling.setNowImplementation,
};

// Enable ReactTestRenderer to be used to test DevTools integration.
TestRenderer.injectIntoDevTools({
findFiberByHostInstance: (() => {
throw new Error('TestRenderer does not support findFiberByHostInstance()');
}: any),
bundleType: __DEV__ ? 1 : 0,
version: ReactVersion,
rendererPackageName: 'react-test-renderer',
});

export default ReactTestRendererFiber;
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/**
* Copyright (c) 2013-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @emails react-core
*/

'use strict';

describe('ReactProfiler DevTools integration', () => {
let React;
let ReactFeatureFlags;
let ReactTestRenderer;
let AdvanceTime;
let advanceTimeBy;
let hook;
let mockNow;

const mockNowForTests = () => {
let currentTime = 0;

mockNow = jest.fn().mockImplementation(() => currentTime);

ReactTestRenderer.unstable_setNowImplementation(mockNow);
advanceTimeBy = amount => {
currentTime += amount;
};
};

beforeEach(() => {
global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = hook = {
inject: () => {},
onCommitFiberRoot: jest.fn((rendererId, root) => {}),
onCommitFiberUnmount: () => {},
supportsFiber: true,
};

jest.resetModules();

ReactFeatureFlags = require('shared/ReactFeatureFlags');
ReactFeatureFlags.enableProfilerTimer = true;
React = require('react');
ReactTestRenderer = require('react-test-renderer');

mockNowForTests();

AdvanceTime = class extends React.Component {
static defaultProps = {
byAmount: 10,
shouldComponentUpdate: true,
};
shouldComponentUpdate(nextProps) {
return nextProps.shouldComponentUpdate;
}
render() {
// Simulate time passing when this component is rendered
advanceTimeBy(this.props.byAmount);
return this.props.children || null;
}
};
});

it('should auto-Profile all fibers if the DevTools hook is detected', () => {
const App = ({multiplier}) => {
advanceTimeBy(2);
return (
<React.unstable_Profiler id="Profiler" onRender={onRender}>
<AdvanceTime byAmount={3 * multiplier} shouldComponentUpdate={true} />
<AdvanceTime
byAmount={7 * multiplier}
shouldComponentUpdate={false}
/>
</React.unstable_Profiler>
);
};

const onRender = jest.fn(() => {});
const rendered = ReactTestRenderer.create(<App multiplier={1} />);

expect(hook.onCommitFiberRoot).toHaveBeenCalledTimes(1);

// Measure observable timing using the Profiler component.
// The time spent in App (above the Profiler) won't be included in the durations,
// But needs to be accounted for in the offset times.
expect(onRender).toHaveBeenCalledTimes(1);
expect(onRender).toHaveBeenCalledWith('Profiler', 'mount', 10, 10, 2, 12);
onRender.mockClear();

// Measure unobservable timing required by the DevTools profiler.
// At this point, the base time should include both:
// The time 2ms in the App component itself, and
// The 10ms spend in the Profiler sub-tree beneath.
expect(rendered.root.findByType(App)._currentFiber().treeBaseTime).toBe(12);

rendered.update(<App multiplier={2} />);

// Measure observable timing using the Profiler component.
// The time spent in App (above the Profiler) won't be included in the durations,
// But needs to be accounted for in the offset times.
expect(onRender).toHaveBeenCalledTimes(1);
expect(onRender).toHaveBeenCalledWith('Profiler', 'update', 6, 13, 14, 20);

// Measure unobservable timing required by the DevTools profiler.
// At this point, the base time should include both:
// The initial 9ms for the components that do not re-render, and
// The updated 6ms for the component that does.
expect(rendered.root.findByType(App)._currentFiber().treeBaseTime).toBe(15);
});
});

0 comments on commit b0f6089

Please sign in to comment.