Skip to content

Commit

Permalink
Refactor to use abort reason
Browse files Browse the repository at this point in the history
Both renderToReadableStream and renderToPipeableStream expose an abort functionality. This commit adds a reason argument to both abort pathways. This can be used by implementors to provide custome reasons for why an abort happened. Additionally the legacy rending pathways such as renderToString are now using this abort reason to convey the instructional message about why using Suspense with these render methods is not correct.
  • Loading branch information
gnoff committed Jun 7, 2022
1 parent f22a8d8 commit bd66c7b
Show file tree
Hide file tree
Showing 7 changed files with 348 additions and 51 deletions.
169 changes: 169 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Expand Up @@ -3057,6 +3057,175 @@ describe('ReactDOMFizzServer', () => {
);
});

// @gate experimental
it('Supports custom abort reasons with a string', async () => {
function App() {
return (
<div>
<p>
<Suspense fallback={'p'}>
<AsyncText text={'hello'} />
</Suspense>
</p>
<span>
<Suspense fallback={'span'}>
<AsyncText text={'world'} />
</Suspense>
</span>
</div>
);
}

let abort;
const loggedErrors = [];
await act(async () => {
const {
pipe,
abort: abortImpl,
} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
onError(error, errorInfo) {
loggedErrors.push(error.message);
return 'a digest';
},
});
abort = abortImpl;
pipe(writable);
});

expect(loggedErrors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>p</p>
<span>span</span>
</div>,
);

await act(() => {
abort('foobar');
});

expect(loggedErrors).toEqual([
'The server did not finish this Suspense boundary. foobar',
'The server did not finish this Suspense boundary. foobar',
]);

let errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error, errorInfo) {
errors.push({error, errorInfo});
},
});

expect(Scheduler).toFlushAndYield([]);

expectErrors(
errors,
[
[
'The server did not finish this Suspense boundary. foobar',
'a digest',
componentStack(['Suspense', 'p', 'div', 'App']),
],
[
'The server did not finish this Suspense boundary. foobar',
'a digest',
componentStack(['Suspense', 'span', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
'a digest',
],
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
'a digest',
],
],
);
});

// @gate experimental
it('Supports custom abort reasons with an Error', async () => {
function App() {
return (
<div>
<p>
<Suspense fallback={'p'}>
<AsyncText text={'hello'} />
</Suspense>
</p>
<span>
<Suspense fallback={'span'}>
<AsyncText text={'world'} />
</Suspense>
</span>
</div>
);
}

let abort;
const loggedErrors = [];
await act(async () => {
const {
pipe,
abort: abortImpl,
} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
onError(error, errorInfo) {
loggedErrors.push(error.message);
return 'a digest';
},
});
abort = abortImpl;
pipe(writable);
});

expect(loggedErrors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>p</p>
<span>span</span>
</div>,
);

await act(() => {
abort(new Error('uh oh'));
});

expect(loggedErrors).toEqual(['uh oh', 'uh oh']);

let errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error, errorInfo) {
errors.push({error, errorInfo});
},
});

expect(Scheduler).toFlushAndYield([]);

expectErrors(
errors,
[
['uh oh', 'a digest', componentStack(['Suspense', 'p', 'div', 'App'])],
[
'uh oh',
'a digest',
componentStack(['Suspense', 'span', 'div', 'App']),
],
],
[
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
'a digest',
],
[
'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.',
'a digest',
],
],
);
});

describe('error escaping', () => {
//@gate experimental
it('escapes error hash, message, and component stack values in directly flushed errors (html escaping)', async () => {
Expand Down
103 changes: 103 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
Expand Up @@ -316,6 +316,109 @@ describe('ReactDOMFizzServer', () => {
result = await readResult(stream);
expect(result).toMatchInlineSnapshot(`"<div>${str2049}</div>"`);
});

// @gate experimental
it('Supports custom abort reasons with a string', async () => {
let hasLoaded = false;
let resolve;
let isComplete = false;
let rendered = false;
const promise = new Promise(r => (resolve = r));
function Wait() {
if (!hasLoaded) {
throw promise;
}
rendered = true;
return 'Done';
}
function App() {
return (
<div>
<p>
<Suspense fallback={'p'}>
<Wait />
</Suspense>
</p>
<span>
<Suspense fallback={'span'}>
<Wait />
</Suspense>
</span>
</div>
);
}

const errors = [];
const controller = new AbortController();
const stream = await ReactDOMFizzServer.renderToReadableStream(<App />, {
signal: controller.signal,
onError(x) {
errors.push(x.message);
return 'a digest';
},
});

// @TODO this is a hack to work around lack of support for abortSignal.reason in node
// The abort call itself should set this property but since we are testing in node we
// set it here manually
controller.signal.reason = 'foobar';
controller.abort('foobar');

expect(errors).toEqual([
'The server did not finish this Suspense boundary. foobar',
'The server did not finish this Suspense boundary. foobar',
]);
});

// @gate experimental
it('Supports custom abort reasons with an Error', async () => {
let hasLoaded = false;
let resolve;
let isComplete = false;
let rendered = false;
const promise = new Promise(r => (resolve = r));
function Wait() {
if (!hasLoaded) {
throw promise;
}
rendered = true;
return 'Done';
}
function App() {
return (
<div>
<p>
<Suspense fallback={'p'}>
<Wait />
</Suspense>
</p>
<span>
<Suspense fallback={'span'}>
<Wait />
</Suspense>
</span>
</div>
);
}

const errors = [];
const controller = new AbortController();
const stream = await ReactDOMFizzServer.renderToReadableStream(<App />, {
signal: controller.signal,
onError(x) {
errors.push(x.message);
return 'a digest';
},
});

// @TODO this is a hack to work around lack of support for abortSignal.reason in node
// The abort call itself should set this property but since we are testing in node we
// set it here manually
controller.signal.reason = new Error('uh oh');
controller.abort(new Error('uh oh'));

expect(errors).toEqual(['uh oh', 'uh oh']);
});
});

// @gate experimental
Expand Down
11 changes: 10 additions & 1 deletion packages/react-dom/src/server/ReactDOMFizzServerBrowser.js
Expand Up @@ -97,7 +97,16 @@ function renderToReadableStream(
if (options && options.signal) {
const signal = options.signal;
const listener = () => {
abort(request);
const reason = signal.reason;
if (
reason &&
(typeof reason === 'string' ||
(typeof reason === 'object' && typeof reason.message === 'string'))
) {
abort(request, reason);
} else {
abort(request, null);
}
signal.removeEventListener('abort', listener);
};
signal.addEventListener('abort', listener);
Expand Down
12 changes: 10 additions & 2 deletions packages/react-dom/src/server/ReactDOMFizzServerNode.js
Expand Up @@ -93,8 +93,16 @@ function renderToPipeableStream(
destination.on('close', createAbortHandler(request));
return destination;
},
abort() {
abort(request);
abort(reason) {
if (
reason &&
(typeof reason === 'string' ||
(typeof reason === 'object' && typeof reason.message === 'string'))
) {
abort(request, reason);
} else {
abort(request, null);
}
},
};
}
Expand Down
18 changes: 13 additions & 5 deletions packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js
Expand Up @@ -16,7 +16,6 @@ import {
startWork,
startFlowing,
abort,
runWithLegacyRuntimeContext,
} from 'react-server/src/ReactFizzServer';

import {
Expand All @@ -36,6 +35,7 @@ export function renderToStringImpl(
children: ReactNodeList,
options: void | ServerOptions,
generateStaticMarkup: boolean,
suspenseAbortMessage: string,
): string {
let didFatal = false;
let fatalError = null;
Expand Down Expand Up @@ -74,7 +74,7 @@ export function renderToStringImpl(
startWork(request);
// If anything suspended and is still pending, we'll abort it before writing.
// That way we write only client-rendered boundaries from the start.
abort(request);
abort(request, suspenseAbortMessage);
startFlowing(request, destination);
if (didFatal) {
throw fatalError;
Expand All @@ -98,16 +98,24 @@ function renderToString(
children: ReactNodeList,
options?: ServerOptions,
): string {
return runWithLegacyRuntimeContext('renderToString', 'browser', () =>
renderToStringImpl(children, options, false),
return renderToStringImpl(
children,
options,
false,
'The server used "renderToString" which does not support Suspense. If you intended for this Suspense boundary to render the fallback content on the server consider throwing an Error somewhere within the Suspense boundary. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server',
);
}

function renderToStaticMarkup(
children: ReactNodeList,
options?: ServerOptions,
): string {
return renderToStringImpl(children, options, true);
return renderToStringImpl(
children,
options,
true,
'The server used "renderToStaticMarkup" which does not support Suspense. If you intended to have the server wait for the suspended component please switch to "renderToReadableStream" which supports Suspense on the server',
);
}

function renderToNodeStream() {
Expand Down

0 comments on commit bd66c7b

Please sign in to comment.