diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index a2024be3d2e5e..1f7916e0cefd8 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -974,12 +974,8 @@ describe('Store', () => { `); - const rendererID = getRendererID(); - const rootID = store.getRootIDForElement(store.getElementIDAtIndex(0)); await actAsync(() => { agent.overrideSuspenseMilestone({ - rendererID, - rootID, suspendedSet: [ store.getElementIDAtIndex(4), store.getElementIDAtIndex(8), @@ -1009,8 +1005,6 @@ describe('Store', () => { await actAsync(() => { agent.overrideSuspenseMilestone({ - rendererID, - rootID, suspendedSet: [], }); }); diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 98091a06d6a8f..2999fc6ac87c3 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -131,8 +131,6 @@ type OverrideSuspenseParams = { }; type OverrideSuspenseMilestoneParams = { - rendererID: number, - rootID: number, suspendedSet: Array, }; @@ -567,17 +565,13 @@ export default class Agent extends EventEmitter<{ }; overrideSuspenseMilestone: OverrideSuspenseMilestoneParams => void = ({ - rendererID, - rootID, suspendedSet, }) => { - const renderer = this._rendererInterfaces[rendererID]; - if (renderer == null) { - console.warn( - `Invalid renderer id "${rendererID}" to override suspense milestone`, - ); - } else { - renderer.overrideSuspenseMilestone(rootID, suspendedSet); + for (const rendererID in this._rendererInterfaces) { + const renderer = ((this._rendererInterfaces[ + (rendererID: any) + ]: any): RendererInterface); + renderer.overrideSuspenseMilestone(suspendedSet); } }; diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 33786a41877b8..d8703dd9a60bc 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -7756,10 +7756,7 @@ export function attach( * @param rootID The root that contains this milestone * @param suspendedSet List of IDs of SuspenseComponent Fibers */ - function overrideSuspenseMilestone( - rootID: FiberInstance['id'], - suspendedSet: Array, - ) { + function overrideSuspenseMilestone(suspendedSet: Array) { if ( typeof setSuspenseHandler !== 'function' || typeof scheduleUpdate !== 'function' @@ -7769,7 +7766,6 @@ export function attach( ); } - // TODO: Allow overriding the timeline for the specified root. forceFallbackForFibers.forEach(fiber => { scheduleUpdate(fiber); }); @@ -7778,9 +7774,7 @@ export function attach( for (let i = 0; i < suspendedSet.length; ++i) { const instance = idToDevToolsInstanceMap.get(suspendedSet[i]); if (instance === undefined) { - console.warn( - `Could not suspend ID '${suspendedSet[i]}' since the instance can't be found.`, - ); + // this is an ID from a different root or even renderer. continue; } diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 9d3e5a0d04e25..3177b9717bb8a 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -437,10 +437,7 @@ export type RendererInterface = { onErrorOrWarning?: OnErrorOrWarning, overrideError: (id: number, forceError: boolean) => void, overrideSuspense: (id: number, forceFallback: boolean) => void, - overrideSuspenseMilestone: ( - rootID: number, - suspendedSet: Array, - ) => void, + overrideSuspenseMilestone: (suspendedSet: Array) => void, overrideValueAtPath: ( type: Type, id: number, diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 616f2d3d3ec23..58f6b98beebc6 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -141,8 +141,6 @@ type OverrideSuspense = { }; type OverrideSuspenseMilestone = { - rendererID: number, - rootID: number, suspendedSet: Array, }; diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 79e6957aecfe4..e6d6653b65458 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -885,38 +885,39 @@ export default class Store extends EventEmitter<{ * @param uniqueSuspendersOnly Filters out boundaries without unique suspenders */ getSuspendableDocumentOrderSuspense( - rootID: Element['id'] | void, uniqueSuspendersOnly: boolean, ): $ReadOnlyArray { - if (rootID === undefined) { - return []; - } - const root = this.getElementByID(rootID); - if (root === null) { - return []; - } - if (!this.supportsTogglingSuspense(rootID)) { - return []; - } const list: SuspenseNode['id'][] = []; - const suspense = this.getSuspenseByID(rootID); - if (suspense !== null) { - const stack = [suspense]; - while (stack.length > 0) { - const current = stack.pop(); - if (current === undefined) { - continue; - } - // Include the root even if we won't show it suspended (because that's just blank). - // You should be able to see what suspended the shell. - if (!uniqueSuspendersOnly || current.hasUniqueSuspenders) { - list.push(current.id); - } - // Add children in reverse order to maintain document order - for (let j = current.children.length - 1; j >= 0; j--) { - const childSuspense = this.getSuspenseByID(current.children[j]); - if (childSuspense !== null) { - stack.push(childSuspense); + // Arbitrarily pick the order in which roots were committed as document-order. + for (let i = 0; i < this._roots.length; i++) { + const rootID = this._roots[i]; + const root = this.getElementByID(rootID); + + if (root === null) { + return []; + } + if (!this.supportsTogglingSuspense(rootID)) { + return []; + } + const suspense = this.getSuspenseByID(rootID); + if (suspense !== null) { + const stack = [suspense]; + while (stack.length > 0) { + const current = stack.pop(); + if (current === undefined) { + continue; + } + // Include the root even if we won't show it suspended (because that's just blank). + // You should be able to see what suspended the shell. + if (!uniqueSuspendersOnly || current.hasUniqueSuspenders) { + list.push(current.id); + } + // Add children in reverse order to maintain document order + for (let j = current.children.length - 1; j >= 0; j--) { + const childSuspense = this.getSuspenseByID(current.children[j]); + if (childSuspense !== null) { + stack.push(childSuspense); + } } } } diff --git a/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js b/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js index a94766d4f1235..c138b6833581f 100644 --- a/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js +++ b/packages/react-devtools-shared/src/devtools/views/ButtonIcon.js @@ -52,7 +52,7 @@ type Props = { type: IconType, }; -const panelIcons = '0 -960 960 820'; +const panelIconsViewBox = '144 -816 672 672'; export default function ButtonIcon({className = '', type}: Props): React.Node { let pathData = null; let viewBox = '0 0 24 24'; @@ -131,27 +131,27 @@ export default function ButtonIcon({className = '', type}: Props): React.Node { break; case 'panel-left-close': pathData = PATH_MATERIAL_PANEL_LEFT_CLOSE; - viewBox = panelIcons; + viewBox = panelIconsViewBox; break; case 'panel-left-open': pathData = PATH_MATERIAL_PANEL_LEFT_OPEN; - viewBox = panelIcons; + viewBox = panelIconsViewBox; break; case 'panel-right-close': pathData = PATH_MATERIAL_PANEL_RIGHT_CLOSE; - viewBox = panelIcons; + viewBox = panelIconsViewBox; break; case 'panel-right-open': pathData = PATH_MATERIAL_PANEL_RIGHT_OPEN; - viewBox = panelIcons; + viewBox = panelIconsViewBox; break; case 'panel-bottom-open': pathData = PATH_MATERIAL_PANEL_BOTTOM_OPEN; - viewBox = panelIcons; + viewBox = panelIconsViewBox; break; case 'panel-bottom-close': pathData = PATH_MATERIAL_PANEL_BOTTOM_CLOSE; - viewBox = panelIcons; + viewBox = panelIconsViewBox; break; case 'suspend': pathData = PATH_SUSPEND; diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css index 60a7328589274..4f321ed3656ab 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.css @@ -53,6 +53,12 @@ display: none; } + +.TogglePanelIcon { + width: 16px; + height: 16px; +} + @container devtools (width < 600px) { .SuspenseTab { flex-direction: column; @@ -111,14 +117,24 @@ .SuspenseTreeViewHeader { padding: 0.25rem; - display: grid; + display: grid; grid-template-columns: auto 1fr auto; - align-items: flex-start; + grid-template-rows: auto auto; + grid-template-areas: + "toggle-left timeline toggle-right" + ". breadcrumbs breadcrumbs"; } -.SuspenseTreeViewHeaderMain { - display: grid; - grid-template-rows: auto auto; +.ToggleTreeList { + grid-area: toggle-left; +} + +.SuspenseTimeline { + grid-area: timeline; +} + +.ToggleInspectedElement { + grid-area: toggle-right;; } .SuspenseBreadcrumbs { @@ -127,4 +143,5 @@ * OwnerStack has more constraints that make it easier so it won't be a 1:1 port. */ overflow-x: auto; + grid-area: breadcrumbs; } diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js index 64ad391dd94d1..c17edf28989bc 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTab.js @@ -57,6 +57,7 @@ function ToggleTreeList({ }) { return ( @@ -105,7 +107,7 @@ function ToggleInspectedElement({ ? 'Show Inspected Element' : 'Hide Inspected Element' }> - + ); } @@ -307,13 +309,11 @@ function SuspenseTab(_: {}) {
-
-
- -
-
- -
+
+ +
+
+
0 ? timeline.length - 1 : 0; - if (rootID === null) { - return ( -
No root selected.
- ); - } - - if (!store.supportsTogglingSuspense(rootID)) { - return ( -
- Can't step through Suspense in production apps. -
- ); - } - if (timeline.length === 0) { return (
- Root contains no Suspense nodes. + Timeline contains no suspendable boundaries.
); } @@ -117,23 +95,10 @@ function SuspenseTimelineInput() { } function handleChange(event: SyntheticEvent) { - if (rootID === null) { - return; - } - const rendererID = store.getRendererIDForElement(rootID); - if (rendererID === null) { - console.error( - `No renderer ID found for root element ${rootID} in suspense timeline.`, - ); - return; - } - const pendingTimelineIndex = +event.currentTarget.value; const suspendedSet = timeline.slice(pendingTimelineIndex); bridge.send('overrideSuspenseMilestone', { - rendererID, - rootID, suspendedSet, }); @@ -202,54 +167,9 @@ function SuspenseTimelineInput() { } export default function SuspenseTimeline(): React$Node { - const store = useContext(StoreContext); - const {roots, selectedRootID, uniqueSuspendersOnly} = useContext( - SuspenseTreeStateContext, - ); - const treeDispatch = useContext(TreeDispatcherContext); - const suspenseTreeDispatch = useContext(SuspenseTreeDispatcherContext); - - function handleChange(event: SyntheticEvent) { - const newRootID = +event.currentTarget.value; - // TODO: scrollIntoView both suspense rects and host instance. - const nextTimeline = store.getSuspendableDocumentOrderSuspense( - newRootID, - uniqueSuspendersOnly, - ); - suspenseTreeDispatch({ - type: 'SET_SUSPENSE_TIMELINE', - payload: [nextTimeline, newRootID, uniqueSuspendersOnly], - }); - if (nextTimeline.length > 0) { - const milestone = nextTimeline[nextTimeline.length - 1]; - treeDispatch({type: 'SELECT_ELEMENT_BY_ID', payload: milestone}); - } - } - return (
- - {roots.length > 0 && ( - - )} +
); } diff --git a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js index 3f0c5fd41ae2b..4099161e3338e 100644 --- a/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js +++ b/packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseTreeContext.js @@ -7,10 +7,7 @@ * @flow */ import type {ReactContext} from 'shared/ReactTypes'; -import type { - Element, - SuspenseNode, -} from 'react-devtools-shared/src/frontend/types'; +import type {SuspenseNode} from 'react-devtools-shared/src/frontend/types'; import type Store from '../../store'; import * as React from 'react'; @@ -27,7 +24,6 @@ import {StoreContext} from '../context'; export type SuspenseTreeState = { lineage: $ReadOnlyArray | null, roots: $ReadOnlyArray, - selectedRootID: SuspenseNode['id'] | null, selectedSuspenseID: SuspenseNode['id'] | null, timeline: $ReadOnlyArray, timelineIndex: number | -1, @@ -80,56 +76,25 @@ type Props = { children: React$Node, }; -function getDefaultRootID(store: Store): Element['id'] | null { - const designatedRootID = store.roots.find(rootID => { - const suspense = store.getSuspenseByID(rootID); - return ( - store.supportsTogglingSuspense(rootID) && - suspense !== null && - suspense.children.length > 1 - ); - }); - - return designatedRootID === undefined ? null : designatedRootID; -} - function getInitialState(store: Store): SuspenseTreeState { - let initialState: SuspenseTreeState; const uniqueSuspendersOnly = true; - const selectedRootID = getDefaultRootID(store); - // TODO: Default to nearest from inspected - if (selectedRootID === null) { - initialState = { - selectedSuspenseID: null, - lineage: null, - roots: store.roots, - selectedRootID, - timeline: [], - timelineIndex: -1, - uniqueSuspendersOnly, - }; - } else { - const timeline = store.getSuspendableDocumentOrderSuspense( - selectedRootID, - uniqueSuspendersOnly, - ); - const timelineIndex = timeline.length - 1; - const selectedSuspenseID = - timelineIndex === -1 ? null : timeline[timelineIndex]; - const lineage = - selectedSuspenseID !== null - ? store.getSuspenseLineage(selectedSuspenseID) - : []; - initialState = { - selectedSuspenseID, - lineage, - roots: store.roots, - selectedRootID, - timeline, - timelineIndex, - uniqueSuspendersOnly, - }; - } + const timeline = + store.getSuspendableDocumentOrderSuspense(uniqueSuspendersOnly); + const timelineIndex = timeline.length - 1; + const selectedSuspenseID = + timelineIndex === -1 ? null : timeline[timelineIndex]; + const lineage = + selectedSuspenseID !== null + ? store.getSuspenseLineage(selectedSuspenseID) + : []; + const initialState: SuspenseTreeState = { + selectedSuspenseID, + lineage, + roots: store.roots, + timeline, + timelineIndex, + uniqueSuspendersOnly, + }; return initialState; } @@ -178,23 +143,10 @@ function SuspenseTreeContextController({children}: Props): React.Node { selectedTimelineID = removedIDs.get(selectedTimelineID); } - let nextRootID = state.selectedRootID; - if (selectedTimelineID !== null && selectedTimelineID !== 0) { - nextRootID = - store.getSuspenseRootIDForSuspense(selectedTimelineID); - } - if (nextRootID === null) { - nextRootID = getDefaultRootID(store); - } - - const nextTimeline = - nextRootID === null - ? [] - : // TODO: Handle different timeline modes (e.g. random order) - store.getSuspendableDocumentOrderSuspense( - nextRootID, - state.uniqueSuspendersOnly, - ); + const nextTimeline = // TODO: Handle different timeline modes (e.g. random order) + store.getSuspendableDocumentOrderSuspense( + state.uniqueSuspendersOnly, + ); let nextTimelineIndex = selectedTimelineID === null || nextTimeline.length === 0 @@ -219,7 +171,6 @@ function SuspenseTreeContextController({children}: Props): React.Node { ...state, lineage: nextLineage, roots: store.roots, - selectedRootID: nextRootID, selectedSuspenseID, timeline: nextTimeline, timelineIndex: nextTimelineIndex, @@ -227,26 +178,20 @@ function SuspenseTreeContextController({children}: Props): React.Node { } case 'SELECT_SUSPENSE_BY_ID': { const selectedSuspenseID = action.payload; - const selectedRootID = - store.getSuspenseRootIDForSuspense(selectedSuspenseID); return { ...state, selectedSuspenseID, - selectedRootID, }; } case 'SET_SUSPENSE_LINEAGE': { const suspenseID = action.payload; const lineage = store.getSuspenseLineage(suspenseID); - const selectedRootID = - store.getSuspenseRootIDForSuspense(suspenseID); return { ...state, lineage, selectedSuspenseID: suspenseID, - selectedRootID, }; } case 'SET_SUSPENSE_TIMELINE': { @@ -283,8 +228,6 @@ function SuspenseTreeContextController({children}: Props): React.Node { ...state, selectedSuspenseID: nextSelectedSuspenseID, lineage: nextLineage, - selectedRootID: - nextRootID === null ? state.selectedRootID : nextRootID, timeline: nextTimeline, timelineIndex: nextMilestoneIndex, uniqueSuspendersOnly: nextUniqueSuspendersOnly,