From 4f5437edf767f153152d14e2565c777921dd1a47 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 4 Apr 2019 15:26:11 +0100 Subject: [PATCH 1/3] Allow to toggle Suspense in Components pane --- shells/dev/app/index.js | 1 + src/backend/agent.js | 20 ++++++ src/backend/renderer.js | 65 ++++++++++++++++++- src/backend/types.js | 5 ++ .../views/Components/SelectedElement.js | 33 ++++++++-- src/devtools/views/Components/types.js | 3 + 6 files changed, 119 insertions(+), 8 deletions(-) diff --git a/shells/dev/app/index.js b/shells/dev/app/index.js index 1daae45595..e731e64470 100644 --- a/shells/dev/app/index.js +++ b/shells/dev/app/index.js @@ -32,6 +32,7 @@ function mountTestApp() { mountHelper(ElementTypes); mountHelper(EditableProps); mountHelper(DeeplyNestedComponents); + mountHelper(SuspenseTree); } function unmountTestApp() { diff --git a/src/backend/agent.js b/src/backend/agent.js index ddbcee3e2c..1f49a0fa2d 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -38,6 +38,12 @@ type SetInParams = {| value: any, |}; +type OverrideSuspenseParams = {| + id: number, + rendererID: number, + forceFallback: boolean, +|}; + export default class Agent extends EventEmitter { _bridge: Bridge = ((null: any): Bridge); _isProfiling: boolean = false; @@ -68,6 +74,7 @@ export default class Agent extends EventEmitter { bridge.addListener('overrideHookState', this.overrideHookState); bridge.addListener('overrideProps', this.overrideProps); bridge.addListener('overrideState', this.overrideState); + bridge.addListener('overrideSuspense', this.overrideSuspense); bridge.addListener('reloadAndProfile', this.reloadAndProfile); bridge.addListener('screenshotCaptured', this.screenshotCaptured); bridge.addListener('selectElement', this.selectElement); @@ -302,6 +309,19 @@ export default class Agent extends EventEmitter { } }; + overrideSuspense = ({ + id, + rendererID, + forceFallback, + }: OverrideSuspenseParams) => { + const renderer = this._rendererInterfaces[rendererID]; + if (renderer == null) { + console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`); + } else { + renderer.overrideSuspense(id, forceFallback); + } + }; + setRendererInterface( rendererID: RendererID, rendererInterface: RendererInterface diff --git a/src/backend/renderer.js b/src/backend/renderer.js index c6381cdb27..ea57106438 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -199,6 +199,7 @@ export function attach( IndeterminateComponent, MemoComponent, SimpleMemoComponent, + SuspenseComponent, } = ReactTypeOfWork; const { CONCURRENT_MODE_NUMBER, @@ -219,7 +220,15 @@ export function attach( DEPRECATED_PLACEHOLDER_SYMBOL_STRING, } = ReactSymbols; - const { overrideHookState, overrideProps } = renderer; + const { + overrideHookState, + overrideProps, + setSuspenseHandler, + scheduleUpdate, + } = renderer; + const supportsEditingSuspense = + typeof setSuspenseHandler === 'function' && + typeof scheduleUpdate === 'function'; const debug = (name: string, fiber: Fiber, parentFiber: ?Fiber): void => { if (__DEBUG__) { @@ -845,8 +854,7 @@ export function attach( // Suspense components only have a non-null memoizedState if they're timed-out. const isTimedOutSuspense = - nextFiber.tag === ReactTypeOfWork.SuspenseComponent && - nextFiber.memoizedState !== null; + nextFiber.tag === SuspenseComponent && nextFiber.memoizedState !== null; if (isTimedOutSuspense) { // The behavior of timed-out Suspense trees is unique. @@ -1411,6 +1419,9 @@ export function attach( } } + const isTimedOutSuspense = + tag === SuspenseComponent && memoizedState !== null; + return { id, @@ -1420,6 +1431,14 @@ export function attach( // Does the current renderer support editable function props? canEditFunctionProps: typeof overrideProps === 'function', + canEditSuspense: + supportsEditingSuspense && + // If it's showing the real content, we can always flip fallback. + (!isTimedOutSuspense || + // If it's showing fallback because we previously forced it to, + // allow toggling it back to remove the fallback override. + forceFallbackForSuspenseIDs.has(id)), + // Can view component source location. canViewSource, @@ -1667,6 +1686,45 @@ export function attach( startProfiling(); } + // React will switch between these implementations depending on whether + // we have any manually suspended Fibers or not. + + function shouldSuspendFiberAlwaysFalse() { + return false; + } + + let forceFallbackForSuspenseIDs = new Set(); + function shouldSuspendFiberAccordingToSet(fiber) { + const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber))); + return forceFallbackForSuspenseIDs.has(id); + } + + function overrideSuspense(id, forceFallback) { + if ( + typeof setSuspenseHandler !== 'function' || + typeof scheduleUpdate !== 'function' + ) { + throw new Error( + 'Expected overrideSuspense() to not get called for earlier React versions.' + ); + } + if (forceFallback) { + forceFallbackForSuspenseIDs.add(id); + if (forceFallbackForSuspenseIDs.size === 1) { + // First override is added. Switch React to slower path. + setSuspenseHandler(shouldSuspendFiberAccordingToSet); + } + } else { + forceFallbackForSuspenseIDs.delete(id); + if (forceFallbackForSuspenseIDs.size === 0) { + // Last override is gone. Switch React back to fast path. + setSuspenseHandler(shouldSuspendFiberAlwaysFalse); + } + } + const fiber = idToFiberMap.get(id); + scheduleUpdate(fiber); + } + return { cleanup, flushInitialOperations, @@ -1680,6 +1738,7 @@ export function attach( handleCommitFiberUnmount, inspectElement, prepareViewElementSource, + overrideSuspense, renderer, selectElement, setInContext, diff --git a/src/backend/types.js b/src/backend/types.js index 83ad660ee7..b18c900d62 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -45,6 +45,10 @@ export type ReactRenderer = { value: any ) => void, + // 16.9+ + scheduleUpdate?: ?(fiber: Object) => void, + setSuspenseHandler?: ?(shouldSuspend: (fiber: Object) => boolean) => void, + // Only injected by React v16.8+ in order to support hooks inspection. currentDispatcherRef?: {| current: null | Dispatcher |}, }; @@ -95,6 +99,7 @@ export type RendererInterface = { handleCommitFiberRoot: (fiber: Object) => void, handleCommitFiberUnmount: (fiber: Object) => void, inspectElement: (id: number) => InspectedElement | null, + overrideSuspense: (id: number, forceFallback: boolean) => void, prepareViewElementSource: (id: number) => void, renderer: ReactRenderer | null, selectElement: (id: number) => void, diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js index 1872ca991f..6f4958040b 100644 --- a/src/devtools/views/Components/SelectedElement.js +++ b/src/devtools/views/Components/SelectedElement.js @@ -20,6 +20,7 @@ import { ElementTypeForwardRef, ElementTypeFunction, ElementTypeMemo, + ElementTypeSuspense, } from '../../types'; import type { InspectedElement } from './types'; @@ -115,6 +116,8 @@ type InspectedElementViewProps = {| inspectedElement: InspectedElement, |}; +const IS_SUSPENDED = 'Suspended'; + function InspectedElementView({ element, inspectedElement, @@ -123,6 +126,7 @@ function InspectedElementView({ const { canEditFunctionProps, canEditHooks, + canEditSuspense, context, hooks, owners, @@ -137,6 +141,7 @@ function InspectedElementView({ let overrideContextFn = null; let overridePropsFn = null; let overrideStateFn = null; + let overrideSuspenseFn = null; if (type === ElementTypeClass) { overrideContextFn = (path: Array, value: any) => { const rendererID = store.getRendererIDForElement(id); @@ -160,6 +165,14 @@ function InspectedElementView({ const rendererID = store.getRendererIDForElement(id); bridge.send('overrideProps', { id, path, rendererID, value }); }; + } else if (type === ElementTypeSuspense && canEditSuspense) { + overrideSuspenseFn = (path: Array, value: boolean) => { + if (path.length !== 1 && path !== IS_SUSPENDED) { + throw new Error('Unexpected path.'); + } + const rendererID = store.getRendererIDForElement(id); + bridge.send('overrideSuspense', { id, rendererID, forceFallback: value }); + }; } return ( @@ -170,11 +183,21 @@ function InspectedElementView({ overrideValueFn={overridePropsFn} showWhenEmpty /> - + {type === ElementTypeSuspense ? ( + + ) : ( + + )} Date: Thu, 4 Apr 2019 20:15:25 +0100 Subject: [PATCH 2/3] Fix highlighting timed out Suspense DOM node --- src/backend/agent.js | 7 ++----- src/backend/renderer.js | 26 +++++++++++++++++++------- src/backend/types.js | 2 +- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/backend/agent.js b/src/backend/agent.js index 1f49a0fa2d..4e3be94eed 100644 --- a/src/backend/agent.js +++ b/src/backend/agent.js @@ -210,11 +210,8 @@ export default class Agent extends EventEmitter { } let node: HTMLElement | null = null; - if ( - renderer !== null && - typeof renderer.getNativeFromReactElement === 'function' - ) { - node = ((renderer.getNativeFromReactElement(id): any): HTMLElement); + if (renderer !== null) { + node = ((renderer.findNativeByFiberID(id): any): HTMLElement); } if (node != null) { diff --git a/src/backend/renderer.js b/src/backend/renderer.js index ea57106438..c149a2541e 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -1053,18 +1053,30 @@ export function attach( currentRootID = -1; } - // The naming is confusing. - // They deal with opaque nodes (fibers), not elements. - function getNativeFromReactElement(id: number) { + function findNativeByFiberID(id: number) { try { - const primaryFiber = getPrimaryFiber(idToFiberMap.get(id)); - const hostInstance = renderer.findHostInstanceByFiber(primaryFiber); - return hostInstance; + const fiber = findCurrentFiberUsingSlowPath(idToFiberMap.get(id)); + if (fiber === null) { + return null; + } + const isTimedOutSuspense = + fiber.tag === SuspenseComponent && fiber.memoizedState !== null; + if (!isTimedOutSuspense) { + // Normal case. + return renderer.findHostInstanceByFiber(fiber); + } else { + // A timed-out Suspense's findDOMNode is useless. + // Try our best to find the fallback directly. + const maybeFallbackFiber = + (fiber.child && fiber.child.sibling) || fiber; + return renderer.findHostInstanceByFiber(maybeFallbackFiber); + } } catch (err) { // The fiber might have unmounted by now. return null; } } + function getFiberIDFromNative( hostInstance, findNearestUnfilteredAncestor = false @@ -1731,7 +1743,7 @@ export function attach( getCommitDetails, getFiberIDFromNative, getInteractions, - getNativeFromReactElement, + findNativeByFiberID, getProfilingDataForDownload, getProfilingSummary, handleCommitFiberRoot, diff --git a/src/backend/types.js b/src/backend/types.js index b18c900d62..0ee472a015 100644 --- a/src/backend/types.js +++ b/src/backend/types.js @@ -86,9 +86,9 @@ export type ProfilingSummary = {| export type RendererInterface = { cleanup: () => void, + findNativeByFiberID: (id: number) => ?NativeType, flushInitialOperations: () => void, getCommitDetails: (rootID: number, commitIndex: number) => CommitDetails, - getNativeFromReactElement?: ?(component: Fiber) => ?NativeType, getFiberIDFromNative: ( component: NativeType, findNearestUnfilteredAncestor?: boolean From b8245de5a116f7c704af70dcf475bca37170bebc Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 4 Apr 2019 21:53:57 +0100 Subject: [PATCH 3/3] Nits --- shells/dev/app/index.js | 1 - src/backend/renderer.js | 6 +++--- src/devtools/views/Components/SelectedElement.js | 4 ++-- src/devtools/views/Components/types.js | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/shells/dev/app/index.js b/shells/dev/app/index.js index e731e64470..1daae45595 100644 --- a/shells/dev/app/index.js +++ b/shells/dev/app/index.js @@ -32,7 +32,6 @@ function mountTestApp() { mountHelper(ElementTypes); mountHelper(EditableProps); mountHelper(DeeplyNestedComponents); - mountHelper(SuspenseTree); } function unmountTestApp() { diff --git a/src/backend/renderer.js b/src/backend/renderer.js index c149a2541e..29caf523d7 100644 --- a/src/backend/renderer.js +++ b/src/backend/renderer.js @@ -226,7 +226,7 @@ export function attach( setSuspenseHandler, scheduleUpdate, } = renderer; - const supportsEditingSuspense = + const supportsTogglingSuspense = typeof setSuspenseHandler === 'function' && typeof scheduleUpdate === 'function'; @@ -1443,8 +1443,8 @@ export function attach( // Does the current renderer support editable function props? canEditFunctionProps: typeof overrideProps === 'function', - canEditSuspense: - supportsEditingSuspense && + canToggleSuspense: + supportsTogglingSuspense && // If it's showing the real content, we can always flip fallback. (!isTimedOutSuspense || // If it's showing fallback because we previously forced it to, diff --git a/src/devtools/views/Components/SelectedElement.js b/src/devtools/views/Components/SelectedElement.js index 6f4958040b..4f6295bce9 100644 --- a/src/devtools/views/Components/SelectedElement.js +++ b/src/devtools/views/Components/SelectedElement.js @@ -126,7 +126,7 @@ function InspectedElementView({ const { canEditFunctionProps, canEditHooks, - canEditSuspense, + canToggleSuspense, context, hooks, owners, @@ -165,7 +165,7 @@ function InspectedElementView({ const rendererID = store.getRendererIDForElement(id); bridge.send('overrideProps', { id, path, rendererID, value }); }; - } else if (type === ElementTypeSuspense && canEditSuspense) { + } else if (type === ElementTypeSuspense && canToggleSuspense) { overrideSuspenseFn = (path: Array, value: boolean) => { if (path.length !== 1 && path !== IS_SUSPENDED) { throw new Error('Unexpected path.'); diff --git a/src/devtools/views/Components/types.js b/src/devtools/views/Components/types.js index b26fb4edc1..00d5a45844 100644 --- a/src/devtools/views/Components/types.js +++ b/src/devtools/views/Components/types.js @@ -42,7 +42,7 @@ export type InspectedElement = {| canEditFunctionProps: boolean, // Is this Suspense, and can its value be overriden now? - canEditSuspense: boolean, + canToggleSuspense: boolean, // Can view component source location. canViewSource: boolean,