-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MVP] Toggle suspense #36
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
// @flow | ||
|
||
import React, { Suspense } from 'react'; | ||
|
||
function SuspenseTree() { | ||
return ( | ||
<> | ||
<h1>Suspense</h1> | ||
<Suspense fallback={<h2>Loading outer</h2>}> | ||
<Parent /> | ||
</Suspense> | ||
</> | ||
); | ||
} | ||
|
||
function Parent() { | ||
return ( | ||
<div> | ||
<Suspense fallback={<h3>Loading inner 1</h3>}> | ||
<Child>Hello</Child> | ||
</Suspense> | ||
<Suspense fallback={<h3>Loading inner 2</h3>}> | ||
<Child>World</Child> | ||
</Suspense> | ||
</div> | ||
); | ||
} | ||
|
||
function Child({ children }) { | ||
return <p>{children}</p>; | ||
} | ||
|
||
export default SuspenseTree; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1334,6 +1334,12 @@ export function attach( | |
// Does the current renderer support editable function props? | ||
canEditFunctionProps: typeof overrideProps === 'function', | ||
|
||
// Is this Suspense, and can we toggle it? | ||
// TODO: this will incorrectly enable it on old versions. | ||
canToggleSuspense: | ||
typeof overrideProps === 'function' && | ||
tag === ReactTypeOfWork.SuspenseComponent, | ||
|
||
// Can view component source location. | ||
canViewSource, | ||
|
||
|
@@ -1554,6 +1560,31 @@ export function attach( | |
isProfiling = false; | ||
} | ||
|
||
let suspendedIds = new Set(); | ||
|
||
function shouldSuspendFiber(fiber) { | ||
if (suspendedIds.size === 0) { | ||
return false; | ||
} | ||
// TODO: maybe optimize this. | ||
const id = getFiberID(getPrimaryFiber(((fiber: any): Fiber))); | ||
return suspendedIds.has(id); | ||
} | ||
|
||
function toggleSuspense(id) { | ||
if (typeof overrideProps !== 'function') { | ||
return; | ||
} | ||
if (suspendedIds.has(id)) { | ||
suspendedIds.delete(id); | ||
} else { | ||
suspendedIds.add(id); | ||
} | ||
// Force a re-render. | ||
const fiber = idToFiberMap.get(id); | ||
overrideProps(fiber, [], null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm curious: why not inject a method like e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to preserve our ability to do filtering based on something other than just putting it in a Set. |
||
} | ||
|
||
return { | ||
cleanup, | ||
getCommitDetails, | ||
|
@@ -1571,6 +1602,8 @@ export function attach( | |
setInHook, | ||
setInProps, | ||
setInState, | ||
shouldSuspendFiber, | ||
toggleSuspense, | ||
startProfiling, | ||
stopProfiling, | ||
walkTree, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ export type IconType = | |
| 'record' | ||
| 'reload' | ||
| 'search' | ||
| 'toggle-suspense' | ||
| 'undo' | ||
| 'up' | ||
| 'view-dom' | ||
|
@@ -60,6 +61,8 @@ export default function ButtonIcon({ type }: Props) { | |
case 'search': | ||
pathData = PATH_SEARCH; | ||
break; | ||
case 'toggle-suspense': | ||
return '🌀'; // TODO | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😆 You love this emoji |
||
case 'undo': | ||
pathData = PATH_UNDO; | ||
break; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,11 @@ import HooksTree from './HooksTree'; | |
import InspectedElementTree from './InspectedElementTree'; | ||
import { hydrate } from 'src/hydration'; | ||
import styles from './SelectedElement.css'; | ||
import { ElementTypeClass, ElementTypeFunction } from '../../types'; | ||
import { | ||
ElementTypeClass, | ||
ElementTypeFunction, | ||
ElementTypeSuspense, | ||
} from '../../types'; | ||
|
||
import type { InspectedElement } from './types'; | ||
import type { DehydratedData, Element } from './types'; | ||
|
@@ -52,6 +56,17 @@ export default function SelectedElement(_: Props) { | |
} | ||
}, [selectedElementID, viewElementSource]); | ||
|
||
const toggleSuspense = useCallback(() => { | ||
if (element === null) { | ||
return; | ||
} | ||
const rendererID = store.getRendererIDForElement(element.id); | ||
bridge.send('toggleSuspense', { | ||
id: element.id, | ||
rendererID, | ||
}); | ||
}, [element]); | ||
|
||
if (element === null) { | ||
return ( | ||
<div className={styles.SelectedElement}> | ||
|
@@ -65,6 +80,9 @@ export default function SelectedElement(_: Props) { | |
inspectedElement.canViewSource && | ||
viewElementSource !== null; | ||
|
||
const canToggleSuspense = | ||
inspectedElement && inspectedElement.canToggleSuspense; | ||
|
||
return ( | ||
<div className={styles.SelectedElement}> | ||
<div className={styles.TitleRow}> | ||
|
@@ -74,6 +92,16 @@ export default function SelectedElement(_: Props) { | |
</div> | ||
</div> | ||
|
||
{canToggleSuspense && ( | ||
<Button | ||
className={styles.IconButton} | ||
onClick={toggleSuspense} | ||
title="Toggle Suspense loading state" | ||
> | ||
<ButtonIcon type="toggle-suspense" /> | ||
</Button> | ||
)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wherever we end up placing this toggle, we should also:
|
||
|
||
<Button | ||
className={styles.IconButton} | ||
onClick={highlightElement} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, nice TODO. This should probably look for a new injected flag that signals whether the current version and build supports the override behavior.