Skip to content
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

Fix Suspense-wrapping heuristic (and bump version numbers) #19373

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/create-subscription/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "create-subscription",
"description": "utility for subscribing to external data sources inside React components",
"version": "16.13.1",
"version": "17.0.0-alpha.0",
"repository": {
"type": "git",
"url": "https://github.com/facebook/react.git",
Expand All @@ -15,7 +15,7 @@
"cjs/"
],
"peerDependencies": {
"react": "^16.3.0"
"react": "^17.0.0-alpha"
},
"devDependencies": {
"rxjs": "^5.5.6"
Expand Down
6 changes: 3 additions & 3 deletions packages/jest-react/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "jest-react",
"version": "0.11.1",
"version": "0.12.0-alpha.0",
"description": "Jest matchers and utilities for testing React components.",
"main": "index.js",
"repository": {
Expand All @@ -20,8 +20,8 @@
"homepage": "https://reactjs.org/",
"peerDependencies": {
"jest": "^23.0.1 || ^24.0.0 || ^25.1.0",
"react": "^16.0.0",
"react-test-renderer": "^16.0.0"
"react": "^17.0.0-alpha",
"react-test-renderer": "^17.0.0-alpha"
},
"dependencies": {
"object-assign": "^4.1.1"
Expand Down
4 changes: 2 additions & 2 deletions packages/react-art/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "react-art",
"description": "React ART is a JavaScript library for drawing vector graphics using React. It provides declarative and reactive bindings to the ART library. Using the same declarative API you can render the output to either Canvas, SVG or VML (IE8).",
"version": "16.13.1",
"version": "17.0.0-alpha.0",
"main": "index.js",
"repository": {
"type": "git",
Expand Down Expand Up @@ -29,7 +29,7 @@
"scheduler": "^0.19.0"
},
"peerDependencies": {
"react": "^16.13.0"
"react": "^17.0.0-alpha"
},
"files": [
"LICENSE",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-cache/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@
"umd/"
],
"peerDependencies": {
"react": "^16.3.0-alpha.1"
"react": "^17.0.0-alpha"
}
}
2 changes: 1 addition & 1 deletion packages/react-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"node": ">=0.10.0"
},
"peerDependencies": {
"react": "^16.0.0"
"react": "^17.0.0-alpha"
},
"dependencies": {
"loose-envify": "^1.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-debug-tools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"node": ">=0.10.0"
},
"peerDependencies": {
"react": "^16.0.0"
"react": "^17.0.0-alpha"
},
"dependencies": {
"error-stack-parser": "^2.0.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,4 +716,93 @@ describe('ProfilingCache', () => {
TestRenderer.create(<Validator commitIndex={0} rootID={rootID} />);
});
});

// See https://github.com/facebook/react/issues/18831
it('should not crash during route transitions with Suspense', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This newly added test caught the regression between 16.13.1 (NPM release) and previous DevTools master.

const RouterContext = React.createContext();

function App() {
return (
<Router>
<Switch>
<Route path="/">
<Home />
</Route>
<Route path="/about">
<About />
</Route>
</Switch>
</Router>
);
}

const Home = () => {
return (
<React.Suspense>
<Link path="/about">Home</Link>
</React.Suspense>
);
};

const About = () => <div>About</div>;

// Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Router.js
function Router({children}) {
const [path, setPath] = React.useState('/');
return (
<RouterContext.Provider value={{path, setPath}}>
{children}
</RouterContext.Provider>
);
}

// Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Switch.js
function Switch({children}) {
return (
<RouterContext.Consumer>
{context => {
let element = null;
React.Children.forEach(children, child => {
if (context.path === child.props.path) {
element = child.props.children;
}
});
return element ? React.cloneElement(element) : null;
}}
</RouterContext.Consumer>
);
}

// Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router/modules/Route.js
function Route({children, path}) {
return null;
}

const linkRef = React.createRef();

// Mimics https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/Link.js
function Link({children, path}) {
return (
<RouterContext.Consumer>
{context => {
return (
<button ref={linkRef} onClick={() => context.setPath(path)}>
{children}
</button>
);
}}
</RouterContext.Consumer>
);
}

const {Simulate} = require('react-dom/test-utils');

const container = document.createElement('div');
utils.act(() => ReactDOM.render(<App />, container));
expect(container.textContent).toBe('Home');
utils.act(() => store.profilerStore.startProfiling());
utils.act(() => Simulate.click(linkRef.current));
utils.act(() => store.profilerStore.stopProfiling());
expect(container.textContent).toBe('About');
});
});
48 changes: 44 additions & 4 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ export function getInternalReactConstants(
// **********************************************************
// The section below is copied from files in React repo.
// Keep it in sync, and add version guards if it changes.
if (gte(version, '16.6.0-beta.0')) {
if (gte(version, '17.0.0-alpha')) {
// TODO (Offscreen) Update the version number above to reflect the first Offscreen alpha/beta release.
ReactTypeOfWork = {
Block: 22,
ClassComponent: 1,
Expand Down Expand Up @@ -181,6 +182,34 @@ export function getInternalReactConstants(
SuspenseListComponent: 19, // Experimental
YieldComponent: -1, // Removed
};
} else if (gte(version, '16.6.0-beta.0')) {
ReactTypeOfWork = {
Block: 22,
ClassComponent: 1,
ContextConsumer: 9,
ContextProvider: 10,
CoroutineComponent: -1, // Removed
CoroutineHandlerPhase: -1, // Removed
DehydratedSuspenseComponent: 18, // Behind a flag
ForwardRef: 11,
Fragment: 7,
FunctionComponent: 0,
HostComponent: 5,
HostPortal: 4,
HostRoot: 3,
HostText: 6,
IncompleteClassComponent: 17,
IndeterminateComponent: 2,
LazyComponent: 16,
MemoComponent: 14,
Mode: 8,
OffscreenComponent: -1, // Experimental
Profiler: 12,
SimpleMemoComponent: 15,
SuspenseComponent: 13,
SuspenseListComponent: 19, // Experimental
YieldComponent: -1, // Removed
};
} else if (gte(version, '16.4.3-alpha')) {
ReactTypeOfWork = {
Block: -1, // Doesn't exist yet
Expand Down Expand Up @@ -452,14 +481,16 @@ export function attach(
const debug = (name: string, fiber: Fiber, parentFiber: ?Fiber): void => {
if (__DEBUG__) {
const displayName = getDisplayNameForFiber(fiber) || 'null';
const id = getFiberID(fiber);
const parentDisplayName =
(parentFiber != null && getDisplayNameForFiber(parentFiber)) || 'null';
const parentID = parentFiber ? getFiberID(parentFiber) : '';
// NOTE: calling getFiberID or getPrimaryFiber is unsafe here
// because it will put them in the map. For now, we'll omit them.
// TODO: better debugging story for this.
console.log(
`[renderer] %c${name} %c${displayName} %c${
parentFiber ? parentDisplayName : ''
`[renderer] %c${name} %c${displayName} (${id}) %c${
parentFiber ? `${parentDisplayName} (${parentID})` : ''
}`,
'color: red; font-weight: bold;',
'color: blue;',
Expand Down Expand Up @@ -1076,6 +1107,10 @@ export function attach(
}

function recordMount(fiber: Fiber, parentFiber: Fiber | null) {
if (__DEBUG__) {
debug('recordMount()', fiber, parentFiber);
}

const isRoot = fiber.tag === HostRoot;
const id = getFiberID(getPrimaryFiber(fiber));

Expand Down Expand Up @@ -1130,6 +1165,10 @@ export function attach(
}

function recordUnmount(fiber: Fiber, isSimulated: boolean) {
if (__DEBUG__) {
debug('recordUnmount()', fiber);
}

if (trackedPathMatchFiber !== null) {
// We're in the process of trying to restore previous selection.
// If this fiber matched but is being unmounted, there's no use trying.
Expand Down Expand Up @@ -1215,7 +1254,8 @@ export function attach(
// because we don't want to highlight every host node inside of a newly mounted subtree.
}

if (fiber.tag === ReactTypeOfWork.SuspenseComponent) {
const isSuspense = fiber.tag === ReactTypeOfWork.SuspenseComponent;
if (isSuspense) {
const isTimedOut = fiber.memoizedState !== null;
if (isTimedOut) {
// Special case: if Suspense mounts in a timed-out state,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-dom",
"version": "16.13.1",
"version": "17.0.0-alpha.0",
"description": "React package for working with the DOM.",
"main": "index.js",
"repository": {
Expand All @@ -22,7 +22,7 @@
"scheduler": "^0.19.0"
},
"peerDependencies": {
"react": "^16.13.0"
"react": "^17.0.0-alpha"
},
"files": [
"LICENSE",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-fetch/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"cjs/"
],
"peerDependencies": {
"react": "^16.13.1"
"react": "^17.0.0-alpha"
},
"browser": {
"./index.js": "./index.browser.js"
Expand Down
2 changes: 1 addition & 1 deletion packages/react-interactions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"loose-envify": "^1.1.0"
},
"peerDependencies": {
"react": "^16.0.0"
"react": "^17.0.0-alpha"
},
"browserify": {
"transform": [
Expand Down
2 changes: 1 addition & 1 deletion packages/react-is/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-is",
"version": "16.13.1",
"version": "17.0.0-alpha.0",
"description": "Brand checking of React Elements.",
"main": "index.js",
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native-renderer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
"scheduler": "^0.11.0"
},
"peerDependencies": {
"react": "^16.0.0"
"react": "^17.0.0-alpha"
}
}
2 changes: 1 addition & 1 deletion packages/react-noop-renderer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"react-server": "*"
},
"peerDependencies": {
"react": "^16.13.0"
"react": "^17.0.0-alpha"
},
"files": [
"LICENSE",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"node": ">=0.10.0"
},
"peerDependencies": {
"react": "^16.13.0"
"react": "^17.0.0-alpha"
},
"dependencies": {
"loose-envify": "^1.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"node": ">=0.10.0"
},
"peerDependencies": {
"react": "^16.0.0"
"react": "^17.0.0-alpha"
},
"dependencies": {
"loose-envify": "^1.1.0",
Expand Down
9 changes: 6 additions & 3 deletions packages/react-test-renderer/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-test-renderer",
"version": "16.13.1",
"version": "17.0.0-alpha.0",
"description": "React package for snapshot testing.",
"main": "index.js",
"repository": {
Expand All @@ -20,12 +20,15 @@
"homepage": "https://reactjs.org/",
"dependencies": {
"object-assign": "^4.1.1",
"react-is": "^16.8.6",
"react-is": "^17.0.0-alpha",
"react-shallow-renderer": "^16.13.1",
"scheduler": "^0.19.0"
},
"peerDependencies": {
"react": "^16.13.0"
"react": "^17.0.0-alpha"
},
"resolutions": {
"react-shallow-renderer/react-is": "^17.0.0-alpha"
},
"files": [
"LICENSE",
Expand Down
4 changes: 2 additions & 2 deletions packages/react-transport-dom-relay/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"scheduler": "^0.11.0"
},
"peerDependencies": {
"react": "^16.0.0",
"react-dom": "^16.0.0"
"react": "^17.0.0-alpha",
"react-dom": "^17.0.0-alpha"
}
}
4 changes: 2 additions & 2 deletions packages/react-transport-dom-webpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
"node": ">=0.10.0"
},
"peerDependencies": {
"react": "^16.0.0",
"react-dom": "^16.0.0",
"react": "^17.0.0-alpha",
"react-dom": "^17.0.0-alpha",
"webpack": "^4.43.0"
},
"dependencies": {
Expand Down
Loading