Skip to content

Commit

Permalink
[Fizz] Reset error component stack and fix error messages (facebook#2…
Browse files Browse the repository at this point in the history
…7456)

The way we collect component stacks right now are pretty fragile.

We expect that we'll call captureBoundaryErrorDetailsDev whenever an
error happens. That resets lastBoundaryErrorComponentStackDev to null
but if we don't, it just lingers and we don't set it to anything new
then which leaks the previous component stack into the next time we have
an error. So we need to reset it in a bunch of places.

This is still broken with erroredReplay because it has the inverse
problem that abortRemainingReplayNodes can call
captureBoundaryErrorDetailsDev more than one time. So the second
boundary won't get a stack.

We probably should try to figure out an alternative way to carry along
the stack. Perhaps WeakMap keyed by the error object.

This also fixes an issue where we weren't invoking the onShellReady
event if we error a replay. That event is a bit weird for resuming
because we're probably really supposed to just invoke it immediately if
we have already flushed the shell in the prerender which is always atm.
Right now, it gets invoked later than necessary because you could have a
resumed hole ready before a sibling in the shell is ready and that's
blocked.
  • Loading branch information
sebmarkbage authored and Umeshmalik committed Oct 8, 2023
1 parent 6f13243 commit 4324d17
Show file tree
Hide file tree
Showing 17 changed files with 637 additions and 401 deletions.
1 change: 1 addition & 0 deletions fixtures/concurrent/time-slicing/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@
npm-debug.log*
yarn-debug.log*
yarn-error.log*
yarn.lock
20 changes: 15 additions & 5 deletions fixtures/concurrent/time-slicing/package.json
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
{
"name": "cpu-demo",
"version": "0.1.0",
"version": "0.2.0",
"private": true,
"dependencies": {
"glamor": "^2.20.40",
"react": "0.0.0-experimental-269dd6ec5",
"react-dom": "0.0.0-experimental-269dd6ec5",
"react-markdown": "^3.2.0",
"react-scripts": "^1.1.4",
"victory": "^0.25.6"
"react-scripts": "^5.0.1",
"victory": "^36.0.0"
},
"scripts": {
"copy-source": "cp -r ../../../build/oss-experimental/* ./node_modules/",
"dev": "react-scripts start",
"build": "react-scripts build",
"test": "react-scripts test --env=jsdom",
"eject": "react-scripts eject"
},
"browserslist": {
"production": [
">0.2%",
"not dead",
"not op_mini all"
],
"development": [
"last 1 chrome version",
"last 1 firefox version",
"last 1 safari version"
]
}
}
1 change: 1 addition & 0 deletions fixtures/concurrent/time-slicing/public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
<meta name="theme-color" content="#000000">
<meta name="description" content="Shows use of sync, debounce, async computation in react">
<!--
manifest.json provides metadata used when your web app is added to the
homescreen on Android. See https://developers.google.com/web/fundamentals/engage-and-retain/web-app-manifest/
Expand Down
112 changes: 112 additions & 0 deletions fixtures/concurrent/time-slicing/src/App.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import React, { useEffect, useState, useTransition } from "react";

import _map from "lodash/map";
import _debounce from "lodash/debounce";

import { Charts, Clock, Options } from "./components";

import { getStreamData } from "./utils";
import { OPTIONS, INITIAL_STATE, STRATEGY, QUESTION_MARK } from "./constants";

import "./index.css";

let _ignoreClick = null;

const App = () => {
const [showDemo, setShowDemo] = useState(INITIAL_STATE.showDemo);
const [strategy, setStrategy] = useState(INITIAL_STATE.strategy);
const [value, setValue] = useState(INITIAL_STATE.value);
const [showClock, setShowClock] = useState(INITIAL_STATE.showClock);
const [isPending, startTransition] = useTransition();

useEffect(() => {
window.addEventListener("keydown", showClockEventHandler);
return () => {
window.removeEventListener("keydown", showClockEventHandler);
};
}, []);

const showClockEventHandler = (e) => {
if (e.key.toLowerCase() === QUESTION_MARK) {
e.preventDefault();
setShowClock((prev) => !prev);
}
};

const handleChartClick = (e) => {
if (showDemo) {
if (e.shiftKey) {
setShowDemo(false);
}
return;
}
if (strategy !== STRATEGY.ASYNC) {
setShowDemo((prev) => !prev);
return;
}
if (_ignoreClick) {
return;
}
_ignoreClick = true;

startTransition(() => {
setShowDemo(true);
_ignoreClick = false;
});
};

const handleChange = (e) => {
const val = e.target.value;
switch (strategy) {
case STRATEGY.SYNC:
setValue(val);
break;
case STRATEGY.DEBOUNCED:
debouncedHandleChange(val);
break;
case STRATEGY.ASYNC:
startTransition(() => {
setValue(val);
});
break;
default:
break;
}
};

const debouncedHandleChange = _debounce((val) => {
if (strategy === STRATEGY.DEBOUNCED) {
setValue(val);
}
}, 1000);

return (
<div className="container">
<div className="rendering">
{_map(OPTIONS, (option) => (
<Options
{...option}
key={option.strategy}
setStrategy={setStrategy}
currentStrategy={strategy}
/>
))}
</div>
<input
className={"input " + strategy}
id="input"
placeholder="longer input → more components and DOM nodes"
defaultValue={value}
onChange={handleChange}
/>
<div className="demo" onClick={handleChartClick}>
{showDemo && (
<Charts data={getStreamData(value)} isPending={isPending} />
)}
{showClock && <Clock />}
</div>
</div>
);
};

export default App;
126 changes: 0 additions & 126 deletions fixtures/concurrent/time-slicing/src/Charts.js

This file was deleted.

105 changes: 0 additions & 105 deletions fixtures/concurrent/time-slicing/src/Clock.js

This file was deleted.

Loading

0 comments on commit 4324d17

Please sign in to comment.