Skip to content

Commit

Permalink
Throw on unhandled SSR suspending (facebook#16460)
Browse files Browse the repository at this point in the history
* Throw on unhandled SSR suspending

* Add a nicer message when the flag is off

* Tweak internal refinement error message
  • Loading branch information
gaearon authored Aug 19, 2019
1 parent dce430a commit 56f93a7
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ function initModules() {
};
}

const {resetModules, serverRender} = ReactDOMServerIntegrationUtils(
initModules,
);
const {
itThrowsWhenRendering,
resetModules,
serverRender,
} = ReactDOMServerIntegrationUtils(initModules);

describe('ReactDOMServerSuspense', () => {
beforeEach(() => {
Expand Down Expand Up @@ -133,4 +135,86 @@ describe('ReactDOMServerSuspense', () => {
expect(divA).toBe(divA2);
expect(divB).toBe(divB2);
});

itThrowsWhenRendering(
'a suspending component outside a Suspense node',
async render => {
await render(
<div>
<React.Suspense />
<AsyncText text="Children" />
<React.Suspense />
</div>,
1,
);
},
'Add a <Suspense fallback=...> component higher in the tree',
);

itThrowsWhenRendering(
'a suspending component without a Suspense above',
async render => {
await render(
<div>
<AsyncText text="Children" />
</div>,
1,
);
},
'Add a <Suspense fallback=...> component higher in the tree',
);

it('does not get confused by throwing null', () => {
function Bad() {
// eslint-disable-next-line no-throw-literal
throw null;
}

let didError;
let error;
try {
ReactDOMServer.renderToString(<Bad />);
} catch (err) {
didError = true;
error = err;
}
expect(didError).toBe(true);
expect(error).toBe(null);
});

it('does not get confused by throwing undefined', () => {
function Bad() {
// eslint-disable-next-line no-throw-literal
throw undefined;
}

let didError;
let error;
try {
ReactDOMServer.renderToString(<Bad />);
} catch (err) {
didError = true;
error = err;
}
expect(didError).toBe(true);
expect(error).toBe(undefined);
});

it('does not get confused by throwing a primitive', () => {
function Bad() {
// eslint-disable-next-line no-throw-literal
throw 'foo';
}

let didError;
let error;
try {
ReactDOMServer.renderToString(<Bad />);
} catch (err) {
didError = true;
error = err;
}
expect(didError).toBe(true);
expect(error).toBe('foo');
});
});
64 changes: 64 additions & 0 deletions packages/react-dom/src/__tests__/ReactServerRendering-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,70 @@ describe('ReactDOMServer', () => {
}).toThrow('ReactDOMServer does not yet support lazy-loaded components.');
});

it('throws when suspending on the server', () => {
function AsyncFoo() {
throw new Promise(() => {});
}

expect(() => {
ReactDOMServer.renderToString(<AsyncFoo />);
}).toThrow('ReactDOMServer does not yet support Suspense.');
});

it('does not get confused by throwing null', () => {
function Bad() {
// eslint-disable-next-line no-throw-literal
throw null;
}

let didError;
let error;
try {
ReactDOMServer.renderToString(<Bad />);
} catch (err) {
didError = true;
error = err;
}
expect(didError).toBe(true);
expect(error).toBe(null);
});

it('does not get confused by throwing undefined', () => {
function Bad() {
// eslint-disable-next-line no-throw-literal
throw undefined;
}

let didError;
let error;
try {
ReactDOMServer.renderToString(<Bad />);
} catch (err) {
didError = true;
error = err;
}
expect(didError).toBe(true);
expect(error).toBe(undefined);
});

it('does not get confused by throwing a primitive', () => {
function Bad() {
// eslint-disable-next-line no-throw-literal
throw 'foo';
}

let didError;
let error;
try {
ReactDOMServer.renderToString(<Bad />);
} catch (err) {
didError = true;
error = err;
}
expect(didError).toBe(true);
expect(error).toBe('foo');
});

it('should throw (in dev) when children are mutated during render', () => {
function Wrapper(props) {
props.children[1] = <p key={1} />; // Mutation is illegal
Expand Down
19 changes: 16 additions & 3 deletions packages/react-dom/src/server/ReactPartialRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -883,7 +883,8 @@ class ReactDOMServerRenderer {
const fallbackFrame = frame.fallbackFrame;
invariant(
fallbackFrame,
'suspense fallback not found, something is broken',
'ReactDOMServer did not find an internal fallback frame for Suspense. ' +
'This is a bug in React. Please file an issue.',
);
this.stack.push(fallbackFrame);
out[this.suspenseDepth] += '<!--$!-->';
Expand All @@ -909,8 +910,20 @@ class ReactDOMServerRenderer {
try {
outBuffer += this.render(child, frame.context, frame.domNamespace);
} catch (err) {
if (enableSuspenseServerRenderer && typeof err.then === 'function') {
suspended = true;
if (err != null && typeof err.then === 'function') {
if (enableSuspenseServerRenderer) {
invariant(
this.suspenseDepth > 0,
// TODO: include component name. This is a bit tricky with current factoring.
'A React component suspended while rendering, but no fallback UI was specified.\n' +
'\n' +
'Add a <Suspense fallback=...> component higher in the tree to ' +
'provide a loading indicator or placeholder to display.',
);
suspended = true;
} else {
invariant(false, 'ReactDOMServer does not yet support Suspense.');
}
} else {
throw err;
}
Expand Down
5 changes: 3 additions & 2 deletions scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@
"300": "Rendered fewer hooks than expected. This may be caused by an accidental early return statement.",
"301": "Too many re-renders. React limits the number of renders to prevent an infinite loop.",
"302": "It is not supported to run the profiling version of a renderer (for example, `react-dom/profiling`) without also replacing the `scheduler/tracing` module with `scheduler/tracing-profiling`. Your bundler might have a setting for aliasing both modules. Learn more at http://fb.me/react-profiling",
"303": "suspense fallback not found, something is broken",
"303": "ReactDOMServer did not find an internal fallback frame for Suspense. This is a bug in React. Please file an issue.",
"304": "Maximum number of concurrent React renderers exceeded. This can happen if you are not properly destroying the Readable provided by React. Ensure that you call .destroy() on it if you no longer want to read from it, and did not read to the end. If you use .pipe() this should be automatic.",
"305": "The current renderer does not support hydration. This error is likely caused by a bug in React. Please file an issue.",
"306": "Element type is invalid. Received a promise that resolves to: %s. Lazy element type must resolve to a class or function.%s",
Expand Down Expand Up @@ -339,5 +339,6 @@
"338": "ReactDOMServer does not yet support the fundamental API.",
"339": "An invalid value was used as an event listener. Expect one or many event listeners created via React.unstable_useResponder().",
"340": "Threw in newly mounted dehydrated component. This is likely a bug in React. Please file an issue.",
"341": "We just came from a parent so we must have had a parent. This is a bug in React."
"341": "We just came from a parent so we must have had a parent. This is a bug in React.",
"342": "A React component suspended while rendering, but no fallback UI was specified.\n\nAdd a <Suspense fallback=...> component higher in the tree to provide a loading indicator or placeholder to display."
}

0 comments on commit 56f93a7

Please sign in to comment.