Skip to content

Commit

Permalink
Advocate for StrictMode usage within Components tree
Browse files Browse the repository at this point in the history
Adds concept of subtree modes to DevTools frontend and visually styles non-StrictMode compliant components.
  • Loading branch information
Brian Vaughn committed Dec 9, 2021
1 parent f2a59df commit ca2caa6
Show file tree
Hide file tree
Showing 20 changed files with 366 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,8 @@ Object {
1,
1,
11,
0,
1,
1,
1,
4,
Expand Down Expand Up @@ -1183,6 +1185,8 @@ Object {
1,
1,
11,
0,
1,
1,
1,
4,
Expand Down Expand Up @@ -1658,6 +1662,8 @@ Object {
1,
13,
11,
0,
1,
1,
1,
4,
Expand Down Expand Up @@ -2202,6 +2208,8 @@ Object {
1,
13,
11,
0,
1,
1,
1,
4,
Expand Down Expand Up @@ -2295,6 +2303,8 @@ Object {
1,
1,
11,
0,
1,
1,
1,
1,
Expand Down Expand Up @@ -2943,6 +2953,8 @@ Object {
1,
1,
11,
0,
1,
1,
1,
1,
Expand Down Expand Up @@ -4214,6 +4226,8 @@ Object {
1,
1,
11,
0,
1,
1,
1,
1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,4 +509,15 @@ describe('Store (legacy)', () => {
expect(store).toMatchSnapshot('5: collapse root');
});
});

describe('StrictMode compliance', () => {
it('should mark all elements as strict mode compliant', () => {
const App = () => null;

const container = document.createElement('div');
act(() => ReactDOM.render(<App />, container));

expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(false);
});
});
});
48 changes: 48 additions & 0 deletions packages/react-devtools-shared/src/__tests__/store-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,54 @@ describe('Store', () => {
`);
});

describe('StrictMode compliance', () => {
it('should mark strict root elements as strict', () => {
const App = () => <Component />;
const Component = () => null;

const container = document.createElement('div');
const root = ReactDOM.createRoot(container, {unstable_strictMode: true});
act(() => {
root.render(<App />);
});

expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(false);
expect(store.getElementAtIndex(1).isStrictModeNonCompliant).toBe(false);
});

it('should mark non strict root elements as not strict', () => {
const App = () => <Component />;
const Component = () => null;

const container = document.createElement('div');
const root = ReactDOM.createRoot(container);
act(() => {
root.render(<App />);
});

expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(true);
expect(store.getElementAtIndex(1).isStrictModeNonCompliant).toBe(true);
});

it('should mark StrictMode subtree elements as strict', () => {
const App = () => (
<React.StrictMode>
<Component />
</React.StrictMode>
);
const Component = () => null;

const container = document.createElement('div');
const root = ReactDOM.createRoot(container);
act(() => {
root.render(<App />);
});

expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(true);
expect(store.getElementAtIndex(1).isStrictModeNonCompliant).toBe(false);
});
});

describe('collapseNodesByDefault:false', () => {
beforeEach(() => {
store.collapseNodesByDefault = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,9 @@ export function attach(
pushOperation(TREE_OPERATION_ADD);
pushOperation(id);
pushOperation(ElementTypeRoot);
pushOperation(0); // isProfilingSupported?
pushOperation(0); // StrictMode compliant?
pushOperation(0); // Profiling supported?
pushOperation(0); // StrictMode supported?
pushOperation(hasOwnerMetadata ? 1 : 0);
} else {
const type = getElementType(internalInstance);
Expand Down
29 changes: 29 additions & 0 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
ElementTypeRoot,
ElementTypeSuspense,
ElementTypeSuspenseList,
StrictMode,
} from 'react-devtools-shared/src/types';
import {
deletePathInObject,
Expand Down Expand Up @@ -52,6 +53,7 @@ import {
TREE_OPERATION_REMOVE,
TREE_OPERATION_REMOVE_ROOT,
TREE_OPERATION_REORDER_CHILDREN,
TREE_OPERATION_SET_SUBTREE_MODE,
TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS,
TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
} from '../constants';
Expand Down Expand Up @@ -155,6 +157,7 @@ export function getInternalReactConstants(
ReactPriorityLevels: ReactPriorityLevelsType,
ReactTypeOfSideEffect: ReactTypeOfSideEffectType,
ReactTypeOfWork: WorkTagMap,
StrictModeBits: number,
|} {
const ReactTypeOfSideEffect: ReactTypeOfSideEffectType = {
DidCapture: 0b10000000,
Expand Down Expand Up @@ -192,6 +195,18 @@ export function getInternalReactConstants(
};
}

let StrictModeBits = 0;
if (gte(version, '18.0.0-alpha')) {
// 18+
StrictModeBits = 0b011000;
} else if (gte(version, '16.9.0')) {
// 16.9 - 17
StrictModeBits = 0b1;
} else if (gte(version, '16.3.0')) {
// 16.3 - 16.8
StrictModeBits = 0b10;
}

let ReactTypeOfWork: WorkTagMap = ((null: any): WorkTagMap);

// **********************************************************
Expand Down Expand Up @@ -513,6 +528,7 @@ export function getInternalReactConstants(
ReactPriorityLevels,
ReactTypeOfWork,
ReactTypeOfSideEffect,
StrictModeBits,
};
}

Expand All @@ -534,6 +550,7 @@ export function attach(
ReactPriorityLevels,
ReactTypeOfWork,
ReactTypeOfSideEffect,
StrictModeBits,
} = getInternalReactConstants(version);
const {
DidCapture,
Expand Down Expand Up @@ -1876,7 +1893,9 @@ export function attach(
pushOperation(TREE_OPERATION_ADD);
pushOperation(id);
pushOperation(ElementTypeRoot);
pushOperation((fiber.mode & StrictModeBits) !== 0 ? 1 : 0);
pushOperation(isProfilingSupported ? 1 : 0);
pushOperation(StrictModeBits !== 0 ? 1 : 0);
pushOperation(hasOwnerMetadata ? 1 : 0);

if (isProfiling) {
Expand Down Expand Up @@ -1913,6 +1932,16 @@ export function attach(
pushOperation(ownerID);
pushOperation(displayNameStringID);
pushOperation(keyStringID);

// If this subtree has a new mode, let the frontend know.
if (
(fiber.mode & StrictModeBits) !== 0 &&
(((parentFiber: any): Fiber).mode & StrictModeBits) === 0
) {
pushOperation(TREE_OPERATION_SET_SUBTREE_MODE);
pushOperation(id);
pushOperation(StrictMode);
}
}

if (isProfilingSupported) {
Expand Down
6 changes: 6 additions & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ export const BRIDGE_PROTOCOL: Array<BridgeProtocol> = [
{
version: 1,
minNpmVersion: '4.13.0',
maxNpmVersion: '4.21.0',
},
// Version 2 adds a StrictMode-enabled and supports-StrictMode bits to add-root operation.
{
version: 2,
minNpmVersion: '4.22.0',
maxNpmVersion: null,
},
];
Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const TREE_OPERATION_REORDER_CHILDREN = 3;
export const TREE_OPERATION_UPDATE_TREE_BASE_DURATION = 4;
export const TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS = 5;
export const TREE_OPERATION_REMOVE_ROOT = 6;
export const TREE_OPERATION_SET_SUBTREE_MODE = 7;

export const LOCAL_STORAGE_DEFAULT_TAB_KEY = 'React::DevTools::defaultTab';

Expand Down
56 changes: 55 additions & 1 deletion packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
TREE_OPERATION_REMOVE,
TREE_OPERATION_REMOVE_ROOT,
TREE_OPERATION_REORDER_CHILDREN,
TREE_OPERATION_SET_SUBTREE_MODE,
TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS,
TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
} from '../constants';
Expand All @@ -33,6 +34,7 @@ import {
BRIDGE_PROTOCOL,
currentBridgeProtocol,
} from 'react-devtools-shared/src/bridge';
import {StrictMode} from 'react-devtools-shared/src/types';

import type {Element} from './views/Components/types';
import type {ComponentFilter, ElementType} from '../types';
Expand Down Expand Up @@ -72,6 +74,7 @@ type Config = {|
export type Capabilities = {|
hasOwnerMetadata: boolean,
supportsProfiling: boolean,
supportsStrictMode: boolean,
|};

/**
Expand Down Expand Up @@ -812,6 +815,20 @@ export default class Store extends EventEmitter<{|
}
};

_recursivelyUpdateSubtree(
id: number,
callback: (element: Element) => void,
): void {
const element = this._idToElement.get(id);
if (element) {
callback(element);

element.children.forEach(child =>
this._recursivelyUpdateSubtree(child, callback),
);
}
}

onBridgeNativeStyleEditorSupported = ({
isSupported,
validAttributes,
Expand Down Expand Up @@ -883,9 +900,15 @@ export default class Store extends EventEmitter<{|
debug('Add', `new root node ${id}`);
}

const isStrictModeCompliant = operations[i] > 0;
i++;

const supportsProfiling = operations[i] > 0;
i++;

const supportsStrictMode = operations[i] > 0;
i++;

const hasOwnerMetadata = operations[i] > 0;
i++;

Expand All @@ -894,15 +917,22 @@ export default class Store extends EventEmitter<{|
this._rootIDToCapabilities.set(id, {
hasOwnerMetadata,
supportsProfiling,
supportsStrictMode,
});

// Not all roots support StrictMode;
// don't flag a root as non-compliant unless it also supports StrictMode.
const isStrictModeNonCompliant =
!isStrictModeCompliant && supportsStrictMode;

this._idToElement.set(id, {
children: [],
depth: -1,
displayName: null,
hocDisplayNames: null,
id,
isCollapsed: false, // Never collapse roots; it would hide the entire tree.
isStrictModeNonCompliant,
key: null,
ownerID: 0,
parentID: 0,
Expand Down Expand Up @@ -958,9 +988,10 @@ export default class Store extends EventEmitter<{|
hocDisplayNames,
id,
isCollapsed: this._collapseNodesByDefault,
isStrictModeNonCompliant: parentElement.isStrictModeNonCompliant,
key,
ownerID,
parentID: parentElement.id,
parentID,
type,
weight: 1,
};
Expand Down Expand Up @@ -1050,6 +1081,7 @@ export default class Store extends EventEmitter<{|
haveErrorsOrWarningsChanged = true;
}
}

break;
}
case TREE_OPERATION_REMOVE_ROOT: {
Expand Down Expand Up @@ -1124,6 +1156,28 @@ export default class Store extends EventEmitter<{|
}
break;
}
case TREE_OPERATION_SET_SUBTREE_MODE: {
const id = operations[i + 1];
const mode = operations[i + 2];

i += 3;

// If elements have already been mounted in this subtree, update them.
// (In practice, this likely only applies to the root element.)
if (mode === StrictMode) {
this._recursivelyUpdateSubtree(id, element => {
element.isStrictModeNonCompliant = false;
});
}

if (__DEBUG__) {
debug(
'Subtree mode',
`Subtree with root ${id} set to mode ${mode}`,
);
}
break;
}
case TREE_OPERATION_UPDATE_TREE_BASE_DURATION:
// Base duration updates are only sent while profiling is in progress.
// We can ignore them at this point.
Expand Down
Loading

0 comments on commit ca2caa6

Please sign in to comment.