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 8, 2021
1 parent ad60746 commit a9ee0aa
Show file tree
Hide file tree
Showing 18 changed files with 251 additions and 11 deletions.
14 changes: 14 additions & 0 deletions packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap
View file
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
4 changes: 3 additions & 1 deletion packages/react-devtools-shared/src/backend/legacy/renderer.js
View file
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
View file
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
View file
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
View file
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
View file
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
8 changes: 8 additions & 0 deletions packages/react-devtools-shared/src/devtools/views/ButtonIcon.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type IconType =
| 'search'
| 'settings'
| 'error'
| 'strict-mode-non-compliant'
| 'suspend'
| 'undo'
| 'up'
Expand Down Expand Up @@ -121,6 +122,9 @@ export default function ButtonIcon({className = '', type}: Props) {
case 'error':
pathData = PATH_ERROR;
break;
case 'strict-mode-non-compliant':
pathData = PATH_STRICT_MODE_NON_COMPLIANT;
break;
case 'suspend':
pathData = PATH_SUSPEND;
break;
Expand Down Expand Up @@ -269,6 +273,10 @@ const PATH_VIEW_DOM = `
3-1.34 3-3-1.34-3-3-3z
`;

const PATH_STRICT_MODE_NON_COMPLIANT = `
M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm1 15h-2v-2h2v2zm0-4h-2V7h2v6z
`;

const PATH_VIEW_SOURCE = `
M9.4 16.6L4.8 12l4.6-4.6L8 6l-6 6 6 6 1.4-1.4zm5.2 0l4.6-4.6-4.6-4.6L16 6l6 6-6 6-1.4-1.4z
`;
Expand Down
9 changes: 9 additions & 0 deletions packages/react-devtools-shared/src/devtools/views/Components/Element.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@
--color-expand-collapse-toggle: var(--color-component-name-inverted);
}

.SelectedElement.StrictModeNonCompliantElement {
background-color: var(--color-warning-background);
color: var(--color-text-selected);
}
.InactiveSelectedElement.StrictModeNonCompliantElement {
background-color: var(--color-error-background);
color: var(--color-text-selected);
}

.KeyName {
color: var(--color-attribute-name);
}
Expand Down
6 changes: 6 additions & 0 deletions packages/react-devtools-shared/src/devtools/views/Components/Element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export default function Element({data, index, style}: Props) {
depth,
displayName,
hocDisplayNames,
isStrictModeNonCompliant,
key,
type,
} = ((element: any): ElementType);
Expand All @@ -126,6 +127,10 @@ export default function Element({data, index, style}: Props) {
className = styles.HoveredElement;
}

if (isStrictModeNonCompliant) {
className += ' ' + styles.StrictModeNonCompliantElement;
}

return (
<div
className={className}
Expand All @@ -146,6 +151,7 @@ export default function Element({data, index, style}: Props) {
{ownerID === null ? (
<ExpandCollapseToggle element={element} store={store} />
) : null}

<DisplayName displayName={displayName} id={((id: any): number)} />
{key && (
<Fragment>
Expand Down
5 changes: 5 additions & 0 deletions packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,8 @@
font-style: italic;
border-left: 1px solid var(--color-border);
}

.StrictModeNonCompliant {
margin-right: 0.125rem;
color: var(--color-warning-background);
}
Loading

0 comments on commit a9ee0aa

Please sign in to comment.