From ca2caa603723d1f7788c55af9a3a8813d421f1f3 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 9 Dec 2021 10:56:24 -0500 Subject: [PATCH 1/3] Advocate for StrictMode usage within Components tree Adds concept of subtree modes to DevTools frontend and visually styles non-StrictMode compliant components. --- .../__snapshots__/profilingCache-test.js.snap | 14 ++++ .../__tests__/legacy/storeLegacy-v15-test.js | 11 +++ .../src/__tests__/store-test.js | 48 +++++++++++++ .../src/backend/legacy/renderer.js | 4 +- .../src/backend/renderer.js | 29 ++++++++ packages/react-devtools-shared/src/bridge.js | 6 ++ .../react-devtools-shared/src/constants.js | 1 + .../src/devtools/store.js | 56 +++++++++++++++- .../src/devtools/views/Components/Element.css | 21 ++++++ .../src/devtools/views/Components/Element.js | 17 +++++ .../views/Components/InspectedElement.css | 5 ++ .../views/Components/InspectedElement.js | 25 ++++++- .../src/devtools/views/Components/types.js | 4 ++ .../src/devtools/views/Icon.js | 9 +++ .../views/Profiler/CommitTreeBuilder.js | 20 +++++- packages/react-devtools-shared/src/types.js | 2 + packages/react-devtools-shared/src/utils.js | 12 ++++ .../src/app/PartiallyStrictApp/index.js | 34 ++++++++++ .../react-devtools-shell/src/app/index.js | 67 ++++++++++++++----- packages/react-devtools/OVERVIEW.md | 2 + 20 files changed, 366 insertions(+), 21 deletions(-) create mode 100644 packages/react-devtools-shell/src/app/PartiallyStrictApp/index.js diff --git a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap index f07f405a85f2e..dd4717bb5cb6b 100644 --- a/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap +++ b/packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap @@ -719,6 +719,8 @@ Object { 1, 1, 11, + 0, + 1, 1, 1, 4, @@ -1183,6 +1185,8 @@ Object { 1, 1, 11, + 0, + 1, 1, 1, 4, @@ -1658,6 +1662,8 @@ Object { 1, 13, 11, + 0, + 1, 1, 1, 4, @@ -2202,6 +2208,8 @@ Object { 1, 13, 11, + 0, + 1, 1, 1, 4, @@ -2295,6 +2303,8 @@ Object { 1, 1, 11, + 0, + 1, 1, 1, 1, @@ -2943,6 +2953,8 @@ Object { 1, 1, 11, + 0, + 1, 1, 1, 1, @@ -4214,6 +4226,8 @@ Object { 1, 1, 11, + 0, + 1, 1, 1, 1, diff --git a/packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js b/packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js index fb6778521361a..9afd3344c985f 100644 --- a/packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js +++ b/packages/react-devtools-shared/src/__tests__/legacy/storeLegacy-v15-test.js @@ -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(, container)); + + expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(false); + }); + }); }); diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 53a531d51bf29..4cf491363262c 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -114,6 +114,54 @@ describe('Store', () => { `); }); + describe('StrictMode compliance', () => { + it('should mark strict root elements as strict', () => { + const App = () => ; + const Component = () => null; + + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container, {unstable_strictMode: true}); + act(() => { + root.render(); + }); + + 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 = () => ; + const Component = () => null; + + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + act(() => { + root.render(); + }); + + expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(true); + expect(store.getElementAtIndex(1).isStrictModeNonCompliant).toBe(true); + }); + + it('should mark StrictMode subtree elements as strict', () => { + const App = () => ( + + + + ); + const Component = () => null; + + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + act(() => { + root.render(); + }); + + expect(store.getElementAtIndex(0).isStrictModeNonCompliant).toBe(true); + expect(store.getElementAtIndex(1).isStrictModeNonCompliant).toBe(false); + }); + }); + describe('collapseNodesByDefault:false', () => { beforeEach(() => { store.collapseNodesByDefault = false; diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index b757d008ac294..0a1896ee322c4 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js @@ -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); diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 7aebe6e507a0f..631521791a549 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -24,6 +24,7 @@ import { ElementTypeRoot, ElementTypeSuspense, ElementTypeSuspenseList, + StrictMode, } from 'react-devtools-shared/src/types'; import { deletePathInObject, @@ -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'; @@ -155,6 +157,7 @@ export function getInternalReactConstants( ReactPriorityLevels: ReactPriorityLevelsType, ReactTypeOfSideEffect: ReactTypeOfSideEffectType, ReactTypeOfWork: WorkTagMap, + StrictModeBits: number, |} { const ReactTypeOfSideEffect: ReactTypeOfSideEffectType = { DidCapture: 0b10000000, @@ -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); // ********************************************************** @@ -513,6 +528,7 @@ export function getInternalReactConstants( ReactPriorityLevels, ReactTypeOfWork, ReactTypeOfSideEffect, + StrictModeBits, }; } @@ -534,6 +550,7 @@ export function attach( ReactPriorityLevels, ReactTypeOfWork, ReactTypeOfSideEffect, + StrictModeBits, } = getInternalReactConstants(version); const { DidCapture, @@ -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) { @@ -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) { diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index e67f6cee95423..68891e8c18d4b 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -57,6 +57,12 @@ export const BRIDGE_PROTOCOL: Array = [ { 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, }, ]; diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index 8898c27787f40..6480222e1e414 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -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'; diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index c8b252072dcb4..b68ea3d26f92e 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -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'; @@ -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'; @@ -72,6 +74,7 @@ type Config = {| export type Capabilities = {| hasOwnerMetadata: boolean, supportsProfiling: boolean, + supportsStrictMode: boolean, |}; /** @@ -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, @@ -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++; @@ -894,8 +917,14 @@ 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, @@ -903,6 +932,7 @@ export default class Store extends EventEmitter<{| hocDisplayNames: null, id, isCollapsed: false, // Never collapse roots; it would hide the entire tree. + isStrictModeNonCompliant, key: null, ownerID: 0, parentID: 0, @@ -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, }; @@ -1050,6 +1081,7 @@ export default class Store extends EventEmitter<{| haveErrorsOrWarningsChanged = true; } } + break; } case TREE_OPERATION_REMOVE_ROOT: { @@ -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. diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.css b/packages/react-devtools-shared/src/devtools/views/Components/Element.css index a21b303a3504a..06c1206f3b3e2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Element.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/Element.css @@ -43,6 +43,19 @@ --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); +} + +.SelectedElement .StrictModeNonCompliantMarker { + color: var(--color-text); +} + .KeyName { color: var(--color-attribute-name); } @@ -76,6 +89,8 @@ .ErrorIcon, .ErrorIconContrast, +.StrictMode, +.StrictModeContrast, .WarningIcon, .WarningIconContrast { height: 0.75rem !important; @@ -85,9 +100,15 @@ .ErrorIcon { color: var(--color-console-error-icon); } +.StrictMode { + color: var(--color-dimmer); +} .WarningIcon { color: var(--color-console-warning-icon); } .ErrorIconContrast, .WarningIconContrast { color: var(--color-component-name); } +.StrictModeContrast { + color: var(--color-text); +} diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.js b/packages/react-devtools-shared/src/devtools/views/Components/Element.js index 73789a9bd62ff..5e4a15b4955b2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Element.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Element.js @@ -113,6 +113,7 @@ export default function Element({data, index, style}: Props) { depth, displayName, hocDisplayNames, + isStrictModeNonCompliant, key, type, } = ((element: any): ElementType); @@ -126,6 +127,10 @@ export default function Element({data, index, style}: Props) { className = styles.HoveredElement; } + if (isStrictModeNonCompliant) { + className += ' ' + styles.StrictModeNonCompliantElement; + } + return (
) : null} + + {key && (  key=" @@ -190,6 +197,16 @@ export default function Element({data, index, style}: Props) { } /> )} + {isStrictModeNonCompliant && ( + + )}
); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.css b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.css index b6ad3b9983f22..1303d009cf01d 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.css @@ -66,3 +66,8 @@ font-style: italic; border-left: 1px solid var(--color-border); } + +.StrictModeNonCompliant { + margin-right: 0.25rem; + color: var(--color-console-error-icon); +} \ No newline at end of file diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js index 9ebd30a6ec0b3..62e14a7ca0a1c 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.js @@ -13,6 +13,7 @@ import {TreeDispatcherContext, TreeStateContext} from './TreeContext'; import {BridgeContext, StoreContext, OptionsContext} from '../context'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; +import Icon from '../Icon'; import {ModalDialogContext} from '../ModalDialog'; import ViewElementSourceContext from './ViewElementSourceContext'; import Toggle from '../Toggle'; @@ -235,9 +236,25 @@ export default function InspectedElementWrapper(_: Props) { ); } + let strictModeBadge = null; + if (element.isStrictModeNonCompliant) { + strictModeBadge = ( + + + + ); + } + return (
+ {strictModeBadge} + {element.key && ( <>
@@ -248,7 +265,13 @@ export default function InspectedElementWrapper(_: Props) { )}
-
+
{element.displayName}
diff --git a/packages/react-devtools-shared/src/devtools/views/Components/types.js b/packages/react-devtools-shared/src/devtools/views/Components/types.js index 372cdd5b9087d..c6789fa1e9ec1 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/types.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/types.js @@ -42,6 +42,10 @@ export type Element = {| // This property is used to quickly determine the total number of Elements, // and the Element at any given index (for windowing purposes). weight: number, + + // This element is not in a StrictMode compliant subtree. + // Only true for React versions supporting StrictMode. + isStrictModeNonCompliant: boolean, |}; export type SerializedElement = {| diff --git a/packages/react-devtools-shared/src/devtools/views/Icon.js b/packages/react-devtools-shared/src/devtools/views/Icon.js index cec6ae34ef910..bf1e867ca7de3 100644 --- a/packages/react-devtools-shared/src/devtools/views/Icon.js +++ b/packages/react-devtools-shared/src/devtools/views/Icon.js @@ -25,6 +25,7 @@ export type IconType = | 'search' | 'settings' | 'store-as-global-variable' + | 'strict-mode-non-compliant' | 'warning'; type Props = {| @@ -77,6 +78,9 @@ export default function Icon({className = '', type}: Props) { case 'store-as-global-variable': pathData = PATH_STORE_AS_GLOBAL_VARIABLE; break; + case 'strict-mode-non-compliant': + pathData = PATH_STRICT_MODE_NON_COMPLIANT; + break; case 'warning': pathData = PATH_WARNING; break; @@ -170,4 +174,9 @@ const PATH_STORE_AS_GLOBAL_VARIABLE = ` 8h-4v-2h4v2zm0-4h-4v-2h4v2z `; +const PATH_STRICT_MODE_NON_COMPLIANT = ` + M4.47 21h15.06c1.54 0 2.5-1.67 1.73-3L13.73 4.99c-.77-1.33-2.69-1.33-3.46 0L2.74 18c-.77 1.33.19 3 1.73 3zM12 + 14c-.55 0-1-.45-1-1v-2c0-.55.45-1 1-1s1 .45 1 1v2c0 .55-.45 1-1 1zm1 4h-2v-2h2v2z +`; + const PATH_WARNING = `M12 1l-12 22h24l-12-22zm-1 8h2v7h-2v-7zm1 11.25c-.69 0-1.25-.56-1.25-1.25s.56-1.25 1.25-1.25 1.25.56 1.25 1.25-.56 1.25-1.25 1.25z`; diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js index 1d16aba7fc292..286048a172bf5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js @@ -13,6 +13,7 @@ import { TREE_OPERATION_REMOVE, TREE_OPERATION_REMOVE_ROOT, TREE_OPERATION_REORDER_CHILDREN, + TREE_OPERATION_SET_SUBTREE_MODE, TREE_OPERATION_UPDATE_TREE_BASE_DURATION, TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS, } from 'react-devtools-shared/src/constants'; @@ -179,7 +180,7 @@ function updateTree( const operation = operations[i]; switch (operation) { - case TREE_OPERATION_ADD: + case TREE_OPERATION_ADD: { id = ((operations[i + 1]: any): number); const type = ((operations[i + 2]: any): ElementType); @@ -192,7 +193,9 @@ function updateTree( } if (type === ElementTypeRoot) { + i++; // isStrictModeCompliant i++; // supportsProfiling flag + i++; // supportsStrictMode flag i++; // hasOwnerMetadata flag if (__DEBUG__) { @@ -250,6 +253,7 @@ function updateTree( } break; + } case TREE_OPERATION_REMOVE: { const removeLength = ((operations[i + 1]: any): number); i += 2; @@ -307,6 +311,17 @@ function updateTree( break; } + case TREE_OPERATION_SET_SUBTREE_MODE: { + id = operations[i + 1]; + const mode = operations[i + 1]; + + i += 3; + + if (__DEBUG__) { + debug('Subtree mode', `Subtree with root ${id} set to mode ${mode}`); + } + break; + } case TREE_OPERATION_UPDATE_TREE_BASE_DURATION: { id = operations[i + 1]; @@ -323,7 +338,7 @@ function updateTree( i += 3; break; } - case TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS: + case TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS: { id = operations[i + 1]; const numErrors = operations[i + 2]; const numWarnings = operations[i + 3]; @@ -337,6 +352,7 @@ function updateTree( ); } break; + } default: throw Error(`Unsupported Bridge operation "${operation}"`); diff --git a/packages/react-devtools-shared/src/types.js b/packages/react-devtools-shared/src/types.js index 5a54979b63b19..859fb9bd8ff72 100644 --- a/packages/react-devtools-shared/src/types.js +++ b/packages/react-devtools-shared/src/types.js @@ -100,3 +100,5 @@ export type StyleXPlugin = {| export type Plugins = {| stylex: StyleXPlugin | null, |}; + +export const StrictMode = 1; diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index 0c50374ae30b6..c022cd4650ed7 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -28,6 +28,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'; @@ -211,7 +212,9 @@ export function printOperationsArray(operations: Array) { if (type === ElementTypeRoot) { logs.push(`Add new root node ${id}`); + i++; // isStrictModeCompliant i++; // supportsProfiling + i++; // supportsStrictMode i++; // hasOwnerMetadata } else { const parentID = ((operations[i]: any): number); @@ -249,6 +252,15 @@ export function printOperationsArray(operations: Array) { logs.push(`Remove root ${rootID}`); break; } + case TREE_OPERATION_SET_SUBTREE_MODE: { + const id = operations[i + 1]; + const mode = operations[i + 1]; + + i += 3; + + logs.push(`Mode ${mode} set for subtree with root ${id}`); + break; + } case TREE_OPERATION_REORDER_CHILDREN: { const id = ((operations[i + 1]: any): number); const numChildren = ((operations[i + 2]: any): number); diff --git a/packages/react-devtools-shell/src/app/PartiallyStrictApp/index.js b/packages/react-devtools-shell/src/app/PartiallyStrictApp/index.js new file mode 100644 index 0000000000000..068e11935bce6 --- /dev/null +++ b/packages/react-devtools-shell/src/app/PartiallyStrictApp/index.js @@ -0,0 +1,34 @@ +/** + * 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 + */ + +import * as React from 'react'; +import {StrictMode} from 'react'; + +export default function PartiallyStrictApp() { + return ( + <> + + + + + + ); +} + +function Child() { + return ; +} + +function StrictChild() { + return ; +} + +function Grandchild() { + return null; +} diff --git a/packages/react-devtools-shell/src/app/index.js b/packages/react-devtools-shell/src/app/index.js index 805cd0a2a707e..f949b8598f48a 100644 --- a/packages/react-devtools-shell/src/app/index.js +++ b/packages/react-devtools-shell/src/app/index.js @@ -6,6 +6,8 @@ import {createElement} from 'react'; import { // $FlowFixMe Flow does not yet know about createRoot() createRoot, + render, + unmountComponentAtNode, } from 'react-dom'; import DeeplyNestedComponents from './DeeplyNestedComponents'; import Iframe from './Iframe'; @@ -18,6 +20,7 @@ import ReactNativeWeb from './ReactNativeWeb'; import ToDoList from './ToDoList'; import Toggle from './Toggle'; import ErrorBoundaries from './ErrorBoundaries'; +import PartiallyStrictApp from './PartiallyStrictApp'; import SuspenseTree from './SuspenseTree'; import {ignoreErrors, ignoreLogs, ignoreWarnings} from './console'; @@ -34,36 +37,68 @@ ignoreErrors([ ignoreWarnings(['Warning: componentWillReceiveProps has been renamed']); ignoreLogs([]); -const roots = []; +const unmountFunctions = []; -function mountHelper(App) { +function createContainer() { const container = document.createElement('div'); ((document.body: any): HTMLBodyElement).appendChild(container); + return container; +} + +function mountApp(App) { + const container = createContainer(); + const root = createRoot(container); root.render(createElement(App)); - roots.push(root); + unmountFunctions.push(() => root.unmount()); +} + +function mountStrictApp(App) { + function StrictRoot() { + return createElement(App); + } + + const container = createContainer(); + + const root = createRoot(container, {unstable_strictMode: true}); + root.render(createElement(StrictRoot)); + + unmountFunctions.push(() => root.unmount()); +} + +function mountLegacyApp(App) { + function LegacyRender() { + return createElement(App); + } + + const container = createContainer(); + + render(createElement(LegacyRender), container); + + unmountFunctions.push(() => unmountComponentAtNode(container)); } function mountTestApp() { - mountHelper(ToDoList); - mountHelper(InspectableElements); - mountHelper(Hydration); - mountHelper(ElementTypes); - mountHelper(EditableProps); - mountHelper(InlineWarnings); - mountHelper(ReactNativeWeb); - mountHelper(Toggle); - mountHelper(ErrorBoundaries); - mountHelper(SuspenseTree); - mountHelper(DeeplyNestedComponents); - mountHelper(Iframe); + mountStrictApp(ToDoList); + mountApp(InspectableElements); + mountApp(Hydration); + mountApp(ElementTypes); + mountApp(EditableProps); + mountApp(InlineWarnings); + mountApp(ReactNativeWeb); + mountApp(Toggle); + mountApp(ErrorBoundaries); + mountApp(SuspenseTree); + mountApp(DeeplyNestedComponents); + mountApp(Iframe); + mountLegacyApp(PartiallyStrictApp); } function unmountTestApp() { - roots.forEach(root => root.unmount()); + unmountFunctions.forEach(fn => fn()); } mountTestApp(); diff --git a/packages/react-devtools/OVERVIEW.md b/packages/react-devtools/OVERVIEW.md index 70b2c9ba095b0..46612fcbf2e03 100644 --- a/packages/react-devtools/OVERVIEW.md +++ b/packages/react-devtools/OVERVIEW.md @@ -63,7 +63,9 @@ For example, adding a root fiber with an id of 1: 1, // add operation 1, // fiber id 11, // ElementTypeRoot + 1, // this root is StrictMode enabled 1, // this root's renderer supports profiling + 1, // this root's renderer supports StrictMode 1, // this root has owner metadata ] ``` From c06012b8a86210fa913b20df139f17c9ecc87536 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 9 Dec 2021 12:58:11 -0500 Subject: [PATCH 2/3] Remove red background highlight and add hover tooltip to icon --- .../src/devtools/views/Components/Element.css | 13 ------------- .../src/devtools/views/Components/Element.js | 5 +---- .../src/devtools/views/Icon.js | 4 +++- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.css b/packages/react-devtools-shared/src/devtools/views/Components/Element.css index 06c1206f3b3e2..5939ff1cc9598 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Element.css +++ b/packages/react-devtools-shared/src/devtools/views/Components/Element.css @@ -43,19 +43,6 @@ --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); -} - -.SelectedElement .StrictModeNonCompliantMarker { - color: var(--color-text); -} - .KeyName { color: var(--color-attribute-name); } diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.js b/packages/react-devtools-shared/src/devtools/views/Components/Element.js index 5e4a15b4955b2..d7260cf8eeefe 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Element.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Element.js @@ -127,10 +127,6 @@ export default function Element({data, index, style}: Props) { className = styles.HoveredElement; } - if (isStrictModeNonCompliant) { - className += ' ' + styles.StrictModeNonCompliantElement; - } - return (
)} diff --git a/packages/react-devtools-shared/src/devtools/views/Icon.js b/packages/react-devtools-shared/src/devtools/views/Icon.js index bf1e867ca7de3..bcf59773d06a5 100644 --- a/packages/react-devtools-shared/src/devtools/views/Icon.js +++ b/packages/react-devtools-shared/src/devtools/views/Icon.js @@ -30,10 +30,11 @@ export type IconType = type Props = {| className?: string, + title?: string, type: IconType, |}; -export default function Icon({className = '', type}: Props) { +export default function Icon({className = '', title = '', type}: Props) { let pathData = null; switch (type) { case 'arrow': @@ -96,6 +97,7 @@ export default function Icon({className = '', type}: Props) { width="24" height="24" viewBox="0 0 24 24"> + {title && {title}} From 555e8718715618df6b322b4dbc81a57bf9984233 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 9 Dec 2021 13:56:59 -0500 Subject: [PATCH 3/3] Updated OVERVIEW.dm to account for Bridge protocol changes --- packages/react-devtools/OVERVIEW.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/react-devtools/OVERVIEW.md b/packages/react-devtools/OVERVIEW.md index 46612fcbf2e03..01ed4bd02a884 100644 --- a/packages/react-devtools/OVERVIEW.md +++ b/packages/react-devtools/OVERVIEW.md @@ -54,7 +54,9 @@ Adding a root to the tree requires sending 5 numbers: 1. add operation constant (`1`) 1. fiber id 1. element type constant (`11 === ElementTypeRoot`) -1. profiling supported flag +1. root has `StrictMode` enabled +1. supports profiling flag +1. supports `StrictMode` flag 1. owner metadata flag For example, adding a root fiber with an id of 1: @@ -178,6 +180,20 @@ Special case of unmounting an entire root (include its descendants). This specia This operation has no additional payload because renderer and root ids are already sent at the beginning of every operations payload. +#### Setting the mode for a subtree + +This message specifies that a subtree operates under a specific mode (e.g. `StrictMode`). + +```js +[ + 7, // set subtree mode + 1, // subtree root fiber id + 0b01 // mode bitmask +] +``` + +Modes are constant meaning that the modes a subtree mounts with will never change. + ## Reconstructing the tree The frontend stores its information about the tree in a map of id to objects with the following keys: