From 20331f17420f688ecc6357e7b9b79731309cf1d4 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 6 Jun 2022 11:25:52 -0700 Subject: [PATCH 1/6] [Fizz] Support abort reasons Fizz supports aborting the render but does not currently accept a reason. The various render functions that use Fizz have some automatic and some user-controlled abort semantics that can be useful to communicate with the running program and users about why an Abort happened. This change implements abort reasons for renderToReadableStream and renderToPipeable stream as well as legacy renderers such as renderToString and related implementations. For AbortController implementations the reason passed to the abort method is forwarded to Fizz and sent to the onError handler. If no reason is provided the AbortController should construct an AbortError DOMException and as a fallback Fizz will generate a similar error in the absense of a reason For pipeable streams, an abort function is returned alongside pipe which already accepted a reason. That reason is now forwarded to Fizz and the implementation described above. For legacy renderers there is no exposed abort functionality but it is used internally and the reasons provided give useful context to, for instance to the fact that Suspsense is not supported in renderToString-like renderers Some notable special case reasons are included below If no reason is provided then a manufactured reason "signal is aborted without reason" If a writable stream errors then a reason "The destination stream errored while writing data." If a writable stream closes early then a reason "The destination stream closed early." --- .../src/__tests__/ReactDOMFizzServer-test.js | 180 +++++- .../ReactDOMFizzServerBrowser-test.js | 571 +++++++++++------- .../__tests__/ReactDOMFizzServerNode-test.js | 14 +- .../__tests__/ReactDOMHydrationDiff-test.js | 4 +- ...DOMServerPartialHydration-test.internal.js | 50 +- .../__tests__/ReactServerRendering-test.js | 48 ++ .../src/server/ReactDOMFizzServerBrowser.js | 2 +- .../src/server/ReactDOMFizzServerNode.js | 25 +- .../src/server/ReactDOMLegacyServerBrowser.js | 96 +-- .../src/server/ReactDOMLegacyServerImpl.js | 97 +++ .../src/server/ReactDOMLegacyServerNode.js | 30 +- .../ReactDOMServerFB-test.internal.js | 4 +- packages/react-server/src/ReactFizzServer.js | 35 +- scripts/error-codes/codes.json | 3 +- scripts/shared/inlinedHostConfigs.js | 1 + 15 files changed, 818 insertions(+), 342 deletions(-) create mode 100644 packages/react-dom/src/server/ReactDOMLegacyServerImpl.js diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index cca05ad88724..c06c506f9680 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1106,7 +1106,13 @@ describe('ReactDOMFizzServer', () => { expect(Scheduler).toFlushAndYield([]); expectErrors( errors, - [['This Suspense boundary was aborted by the server.', expectedDigest]], + [ + [ + 'The server did not finish this Suspense boundary: signal is aborted without reason', + expectedDigest, + componentStack(['h1', 'Suspense', 'div', 'App']), + ], + ], [ [ 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', @@ -3057,6 +3063,178 @@ describe('ReactDOMFizzServer', () => { ); }); + // @gate experimental + it('Supports custom abort reasons with a string', async () => { + function App() { + return ( +
+

+ + + +

+ + + + + +
+ ); + } + + let abort; + const loggedErrors = []; + await act(async () => { + const { + pipe, + abort: abortImpl, + } = ReactDOMFizzServer.renderToPipeableStream(, { + onError(error) { + // In this test we contrive erroring with strings so we push the error whereas in most + // other tests we contrive erroring with Errors and push the message. + loggedErrors.push(error); + return 'a digest'; + }, + }); + abort = abortImpl; + pipe(writable); + }); + + expect(loggedErrors).toEqual([]); + expect(getVisibleChildren(container)).toEqual( +
+

p

+ span +
, + ); + + await act(() => { + abort('foobar'); + }); + + expect(loggedErrors).toEqual(['foobar', 'foobar']); + + const errors = []; + ReactDOMClient.hydrateRoot(container, , { + 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 ( +
+

+ + + +

+ + + + + +
+ ); + } + + let abort; + const loggedErrors = []; + await act(async () => { + const { + pipe, + abort: abortImpl, + } = ReactDOMFizzServer.renderToPipeableStream(, { + onError(error) { + loggedErrors.push(error.message); + return 'a digest'; + }, + }); + abort = abortImpl; + pipe(writable); + }); + + expect(loggedErrors).toEqual([]); + expect(getVisibleChildren(container)).toEqual( +
+

p

+ span +
, + ); + + await act(() => { + abort(new Error('uh oh')); + }); + + expect(loggedErrors).toEqual(['uh oh', 'uh oh']); + + const errors = []; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error, errorInfo) { + errors.push({error, errorInfo}); + }, + }); + + expect(Scheduler).toFlushAndYield([]); + + expectErrors( + errors, + [ + [ + 'The server did not finish this Suspense boundary: uh oh', + 'a digest', + componentStack(['Suspense', 'p', 'div', 'App']), + ], + [ + 'The server did not finish this Suspense boundary: 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 () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index a1429b0d2a17..0d60ca59beaa 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -14,13 +14,20 @@ global.ReadableStream = require('web-streams-polyfill/ponyfill/es6').ReadableStr global.TextEncoder = require('util').TextEncoder; let React; +let ReactDOMClient; let ReactDOMFizzServer; let Suspense; +let Scheduler; +let JSDOM; +let document; +let container; describe('ReactDOMFizzServer', () => { beforeEach(() => { jest.resetModules(); React = require('react'); + ReactDOMClient = require('react-dom/client'); + Scheduler = require('scheduler'); if (__EXPERIMENTAL__) { ReactDOMFizzServer = require('react-dom/server.browser'); } @@ -48,90 +55,137 @@ describe('ReactDOMFizzServer', () => { } } - // @gate experimental - it('should call renderToReadableStream', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( -
hello world
, - ); - const result = await readResult(stream); - expect(result).toMatchInlineSnapshot(`"
hello world
"`); - }); + describe('renderToReadableStream', () => { + // @gate experimental + it('should call renderToReadableStream', async () => { + const stream = await ReactDOMFizzServer.renderToReadableStream( +
hello world
, + ); + const result = await readResult(stream); + expect(result).toMatchInlineSnapshot(`"
hello world
"`); + }); + + // @gate experimental + it('should emit DOCTYPE at the root of the document', async () => { + const stream = await ReactDOMFizzServer.renderToReadableStream( + + hello world + , + ); + const result = await readResult(stream); + expect(result).toMatchInlineSnapshot( + `"hello world"`, + ); + }); - // @gate experimental - it('should emit DOCTYPE at the root of the document', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( - - hello world - , - ); - const result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"hello world"`, - ); - }); + // @gate experimental + it('should emit bootstrap script src at the end', async () => { + const stream = await ReactDOMFizzServer.renderToReadableStream( +
hello world
, + { + bootstrapScriptContent: 'INIT();', + bootstrapScripts: ['init.js'], + bootstrapModules: ['init.mjs'], + }, + ); + const result = await readResult(stream); + expect(result).toMatchInlineSnapshot( + `"
hello world
"`, + ); + }); + + // @gate experimental + it('emits all HTML as one unit if we wait until the end to start', async () => { + let hasLoaded = false; + let resolve; + const promise = new Promise(r => (resolve = r)); + function Wait() { + if (!hasLoaded) { + throw promise; + } + return 'Done'; + } + let isComplete = false; + const stream = await ReactDOMFizzServer.renderToReadableStream( +
+ + + +
, + ); - // @gate experimental - it('should emit bootstrap script src at the end', async () => { - const stream = await ReactDOMFizzServer.renderToReadableStream( -
hello world
, - { - bootstrapScriptContent: 'INIT();', - bootstrapScripts: ['init.js'], - bootstrapModules: ['init.mjs'], - }, - ); - const result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"
hello world
"`, - ); - }); + stream.allReady.then(() => (isComplete = true)); - // @gate experimental - it('emits all HTML as one unit if we wait until the end to start', async () => { - let hasLoaded = false; - let resolve; - const promise = new Promise(r => (resolve = r)); - function Wait() { - if (!hasLoaded) { - throw promise; - } - return 'Done'; - } - let isComplete = false; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- - - -
, - ); - - stream.allReady.then(() => (isComplete = true)); - - await jest.runAllTimers(); - expect(isComplete).toBe(false); - // Resolve the loading. - hasLoaded = true; - await resolve(); - - await jest.runAllTimers(); - - expect(isComplete).toBe(true); - - const result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"
Done
"`, - ); - }); + await jest.runAllTimers(); + expect(isComplete).toBe(false); + // Resolve the loading. + hasLoaded = true; + await resolve(); - // @gate experimental - it('should reject the promise when an error is thrown at the root', async () => { - const reportedErrors = []; - let caughtError = null; - try { - await ReactDOMFizzServer.renderToReadableStream( + await jest.runAllTimers(); + + expect(isComplete).toBe(true); + + const result = await readResult(stream); + expect(result).toMatchInlineSnapshot( + `"
Done
"`, + ); + }); + + // @gate experimental + it('should reject the promise when an error is thrown at the root', async () => { + const reportedErrors = []; + let caughtError = null; + try { + await ReactDOMFizzServer.renderToReadableStream( +
+ +
, + { + onError(x) { + reportedErrors.push(x); + }, + }, + ); + } catch (error) { + caughtError = error; + } + expect(caughtError).toBe(theError); + expect(reportedErrors).toEqual([theError]); + }); + + // @gate experimental + it('should reject the promise when an error is thrown inside a fallback', async () => { + const reportedErrors = []; + let caughtError = null; + try { + await ReactDOMFizzServer.renderToReadableStream( +
+ }> + + +
, + { + onError(x) { + reportedErrors.push(x); + }, + }, + ); + } catch (error) { + caughtError = error; + } + expect(caughtError).toBe(theError); + expect(reportedErrors).toEqual([theError]); + }); + + // @gate experimental + it('should not error the stream when an error is thrown inside suspense boundary', async () => { + const reportedErrors = []; + const stream = await ReactDOMFizzServer.renderToReadableStream(
- + Loading
}> + + , { onError(x) { @@ -139,172 +193,259 @@ describe('ReactDOMFizzServer', () => { }, }, ); - } catch (error) { - caughtError = error; - } - expect(caughtError).toBe(theError); - expect(reportedErrors).toEqual([theError]); - }); - // @gate experimental - it('should reject the promise when an error is thrown inside a fallback', async () => { - const reportedErrors = []; - let caughtError = null; - try { - await ReactDOMFizzServer.renderToReadableStream( + const result = await readResult(stream); + expect(result).toContain('Loading'); + expect(reportedErrors).toEqual([theError]); + }); + + // @gate experimental + it('should be able to complete by aborting even if the promise never resolves', async () => { + const errors = []; + const controller = new AbortController(); + const stream = await ReactDOMFizzServer.renderToReadableStream(
- }> + Loading
}> , { + signal: controller.signal, onError(x) { - reportedErrors.push(x); + errors.push(x.message); }, }, ); - } catch (error) { - caughtError = error; - } - expect(caughtError).toBe(theError); - expect(reportedErrors).toEqual([theError]); - }); - // @gate experimental - it('should not error the stream when an error is thrown inside suspense boundary', async () => { - const reportedErrors = []; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - - - , - { - onError(x) { - reportedErrors.push(x); + controller.abort(); + + const result = await readResult(stream); + expect(result).toContain('Loading'); + + expect(errors).toEqual(['signal is aborted without reason']); + }); + + // @gate experimental + it('should not continue rendering after the reader cancels', 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'; + } + const errors = []; + const stream = await ReactDOMFizzServer.renderToReadableStream( +
+ Loading
}> + /> + + , + { + onError(x) { + errors.push(x.message); + }, }, - }, - ); + ); - const result = await readResult(stream); - expect(result).toContain('Loading'); - expect(reportedErrors).toEqual([theError]); - }); + stream.allReady.then(() => (isComplete = true)); - // @gate experimental - it('should be able to complete by aborting even if the promise never resolves', async () => { - const errors = []; - const controller = new AbortController(); - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - - - , - { - signal: controller.signal, - onError(x) { - errors.push(x.message); - }, - }, - ); + expect(rendered).toBe(false); + expect(isComplete).toBe(false); - controller.abort(); + const reader = stream.getReader(); + reader.cancel(); - const result = await readResult(stream); - expect(result).toContain('Loading'); + expect(errors).toEqual(['signal is aborted without reason']); - expect(errors).toEqual([ - 'This Suspense boundary was aborted by the server.', - ]); - }); + hasLoaded = true; + resolve(); - // @gate experimental - it('should not continue rendering after the reader cancels', async () => { - let hasLoaded = false; - let resolve; - let isComplete = false; - let rendered = false; - const promise = new Promise(r => (resolve = r)); - function Wait() { - if (!hasLoaded) { + await jest.runAllTimers(); + + expect(rendered).toBe(false); + expect(isComplete).toBe(true); + }); + + // @gate experimental + it('should stream large contents that might overlow individual buffers', async () => { + const str492 = `(492) This string is intentionally 492 bytes long because we want to make sure we process chunks that will overflow buffer boundaries. It will repeat to fill out the bytes required (inclusive of this prompt):: foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux q :: total count (492)`; + const str2049 = `(2049) This string is intentionally 2049 bytes long because we want to make sure we process chunks that will overflow buffer boundaries. It will repeat to fill out the bytes required (inclusive of this prompt):: foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy :: total count (2049)`; + + // this specific layout is somewhat contrived to exercise the landing on + // an exact view boundary. it's not critical to test this edge case but + // since we are setting up a test in general for larger chunks I contrived it + // as such for now. I don't think it needs to be maintained if in the future + // the view sizes change or become dynamic becasue of the use of byobRequest + let stream; + stream = await ReactDOMFizzServer.renderToReadableStream( + <> +
+ {''} +
+
{str492}
+
{str492}
+ , + ); + + let result; + result = await readResult(stream); + expect(result).toMatchInlineSnapshot( + `"
${str492}
${str492}
"`, + ); + + // this size 2049 was chosen to be a couple base 2 orders larger than the current view + // size. if the size changes in the future hopefully this will still exercise + // a chunk that is too large for the view size. + stream = await ReactDOMFizzServer.renderToReadableStream( + <> +
{str2049}
+ , + ); + + result = await readResult(stream); + expect(result).toMatchInlineSnapshot(`"
${str2049}
"`); + }); + + // @gate experimental + it('Supports custom abort reasons with a string', async () => { + const promise = new Promise(r => {}); + function Wait() { throw promise; } - rendered = true; - return 'Done'; - } - const errors = []; - const stream = await ReactDOMFizzServer.renderToReadableStream( -
- Loading
}> - /> - - , - { + function App() { + return ( +
+

+ + + +

+ + + + + +
+ ); + } + + const errors = []; + const controller = new AbortController(); + await ReactDOMFizzServer.renderToReadableStream(, { + signal: controller.signal, onError(x) { - errors.push(x.message); + errors.push(x); + return 'a digest'; }, - }, - ); + }); - stream.allReady.then(() => (isComplete = true)); + // @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(rendered).toBe(false); - expect(isComplete).toBe(false); + expect(errors).toEqual(['foobar', 'foobar']); + }); - const reader = stream.getReader(); - reader.cancel(); - - expect(errors).toEqual([ - 'This Suspense boundary was aborted by the server.', - ]); + // @gate experimental + it('Supports custom abort reasons with an Error', async () => { + const promise = new Promise(r => {}); + function Wait() { + throw promise; + } + function App() { + return ( +
+

+ + + +

+ + + + + +
+ ); + } - hasLoaded = true; - resolve(); + const errors = []; + const controller = new AbortController(); + await ReactDOMFizzServer.renderToReadableStream(, { + signal: controller.signal, + onError(x) { + errors.push(x.message); + return 'a digest'; + }, + }); - await jest.runAllTimers(); + // @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(rendered).toBe(false); - expect(isComplete).toBe(true); + expect(errors).toEqual(['uh oh', 'uh oh']); + }); }); // @gate experimental - it('should stream large contents that might overlow individual buffers', async () => { - const str492 = `(492) This string is intentionally 492 bytes long because we want to make sure we process chunks that will overflow buffer boundaries. It will repeat to fill out the bytes required (inclusive of this prompt):: foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux q :: total count (492)`; - const str2049 = `(2049) This string is intentionally 2049 bytes long because we want to make sure we process chunks that will overflow buffer boundaries. It will repeat to fill out the bytes required (inclusive of this prompt):: foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy thud foo bar qux quux corge grault garply waldo fred plugh xyzzy :: total count (2049)`; - - // this specific layout is somewhat contrived to exercise the landing on - // an exact view boundary. it's not critical to test this edge case but - // since we are setting up a test in general for larger chunks I contrived it - // as such for now. I don't think it needs to be maintained if in the future - // the view sizes change or become dynamic becasue of the use of byobRequest - let stream; - stream = await ReactDOMFizzServer.renderToReadableStream( - <> -
- {''} -
-
{str492}
-
{str492}
- , - ); - - let result; - result = await readResult(stream); - expect(result).toMatchInlineSnapshot( - `"
${str492}
${str492}
"`, - ); - - // this size 2049 was chosen to be a couple base 2 orders larger than the current view - // size. if the size changes in the future hopefully this will still exercise - // a chunk that is too large for the view size. - stream = await ReactDOMFizzServer.renderToReadableStream( - <> -
{str2049}
- , - ); - - result = await readResult(stream); - expect(result).toMatchInlineSnapshot(`"
${str2049}
"`); + describe('renderToString', () => { + beforeEach(() => { + JSDOM = require('jsdom').JSDOM; + + // Test Environment + const jsdom = new JSDOM( + '
', + { + runScripts: 'dangerously', + }, + ); + document = jsdom.window.document; + container = document.getElementById('container'); + }); + + it('refers users to apis that support Suspense when somethign suspends', () => { + function App({isClient}) { + return ( +
+ + {isClient ? 'resolved' : } + +
+ ); + } + container.innerHTML = ReactDOMFizzServer.renderToString( + , + ); + + const errors = []; + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error, errorInfo) { + errors.push(error.message); + }, + }); + + expect(Scheduler).toFlushAndYield([]); + expect(errors.length).toBe(1); + if (__DEV__) { + expect(errors[0]).toBe( + 'The server did not finish this Suspense boundary: 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', + ); + } else { + expect(errors[0]).toBe( + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', + ); + } + }); }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index a625a8df0e2f..96dd22c4bafc 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -226,7 +226,7 @@ describe('ReactDOMFizzServer', () => { expect(output.result).toBe(''); expect(reportedErrors).toEqual([ theError.message, - 'This Suspense boundary was aborted by the server.', + 'The destination stream errored while writing data.', ]); expect(reportedShellErrors).toEqual([theError]); }); @@ -317,13 +317,11 @@ describe('ReactDOMFizzServer', () => { expect(output.result).toContain('Loading'); expect(isCompleteCalls).toBe(0); - abort(); + abort(new Error('uh oh')); await completed; - expect(errors).toEqual([ - 'This Suspense boundary was aborted by the server.', - ]); + expect(errors).toEqual(['uh oh']); expect(output.error).toBe(undefined); expect(output.result).toContain('Loading'); expect(isCompleteCalls).toBe(1); @@ -365,8 +363,8 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual([ // There are two boundaries that abort - 'This Suspense boundary was aborted by the server.', - 'This Suspense boundary was aborted by the server.', + 'signal is aborted without reason', + 'signal is aborted without reason', ]); expect(output.error).toBe(undefined); expect(output.result).toContain('Loading'); @@ -603,7 +601,7 @@ describe('ReactDOMFizzServer', () => { await completed; expect(errors).toEqual([ - 'This Suspense boundary was aborted by the server.', + 'The destination stream errored while writing data.', ]); expect(rendered).toBe(false); expect(isComplete).toBe(true); diff --git a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js index f2ddab69cdd0..e8e7dffee5d6 100644 --- a/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMHydrationDiff-test.js @@ -830,7 +830,7 @@ describe('ReactDOMServerHydration', () => { } else { expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` Array [ - "Caught [This Suspense boundary was aborted by the server.]", + "Caught [The server did not finish this Suspense boundary: 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 \\"renderToPipeableStream\\" which supports Suspense on the server]", ] `); } @@ -865,7 +865,7 @@ describe('ReactDOMServerHydration', () => { } else { expect(testMismatch(Mismatch)).toMatchInlineSnapshot(` Array [ - "Caught [This Suspense boundary was aborted by the server.]", + "Caught [The server did not finish this Suspense boundary: 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 \\"renderToPipeableStream\\" which supports Suspense on the server]", ] `); } diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index c27eabe3e97d..e086448d6914 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -1674,11 +1674,17 @@ describe('ReactDOMServerPartialHydration', () => { // we exclude fb bundles with partial renderer if (__DEV__ && !usingPartialRenderer) { expect(Scheduler).toFlushAndYield([ - 'This Suspense boundary was aborted by the server.', + 'The server did not finish this Suspense boundary: 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 "renderToPipeableStream" which supports Suspense on the server', ]); } else { expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', ]); } jest.runAllTimers(); @@ -1742,11 +1748,17 @@ describe('ReactDOMServerPartialHydration', () => { // we exclude fb bundles with partial renderer if (__DEV__ && !usingPartialRenderer) { expect(Scheduler).toFlushAndYield([ - 'This Suspense boundary was aborted by the server.', + 'The server did not finish this Suspense boundary: 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 "renderToPipeableStream" which supports Suspense on the server', ]); } else { expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', ]); } // This will have exceeded the suspended time so we should timeout. @@ -1815,11 +1827,17 @@ describe('ReactDOMServerPartialHydration', () => { // we exclude fb bundles with partial renderer if (__DEV__ && !usingPartialRenderer) { expect(Scheduler).toFlushAndYield([ - 'This Suspense boundary was aborted by the server.', + 'The server did not finish this Suspense boundary: 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 "renderToPipeableStream" which supports Suspense on the server', ]); } else { expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', ]); } // This will have exceeded the suspended time so we should timeout. @@ -2139,11 +2157,17 @@ describe('ReactDOMServerPartialHydration', () => { // we exclude fb bundles with partial renderer if (__DEV__ && !usingPartialRenderer) { expect(Scheduler).toFlushAndYield([ - 'This Suspense boundary was aborted by the server.', + 'The server did not finish this Suspense boundary: 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 "renderToPipeableStream" which supports Suspense on the server', ]); } else { expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', ]); } @@ -2208,11 +2232,17 @@ describe('ReactDOMServerPartialHydration', () => { // we exclude fb bundles with partial renderer if (__DEV__ && !usingPartialRenderer) { expect(Scheduler).toFlushAndYield([ - 'This Suspense boundary was aborted by the server.', + 'The server did not finish this Suspense boundary: 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 "renderToPipeableStream" which supports Suspense on the server', ]); } else { expect(Scheduler).toFlushAndYield([ - 'The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.', + 'The server could not finish this Suspense boundary, likely due to ' + + 'an error during server rendering. Switched to client rendering.', ]); } jest.runAllTimers(); diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js index 23be43a7735a..c22846fa637f 100644 --- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js @@ -562,6 +562,19 @@ describe('ReactDOMServer', () => { 'Bad lazy', ); }); + + it('aborts synchronously any suspended tasks and renders their fallbacks', () => { + const promise = new Promise(res => {}); + function Suspender() { + throw promise; + } + const response = ReactDOMServer.renderToStaticMarkup( + + + , + ); + expect(response).toEqual('fallback'); + }); }); describe('renderToNodeStream', () => { @@ -618,6 +631,41 @@ describe('ReactDOMServer', () => { expect(response.read()).toBeNull(); }); }); + + it('should refer users to new apis when using suspense', async () => { + let resolve = null; + const promise = new Promise(res => { + resolve = () => { + resolved = true; + res(); + }; + }); + let resolved = false; + function Suspender() { + if (resolved) { + return 'resolved'; + } + throw promise; + } + + let response; + expect(() => { + response = ReactDOMServer.renderToNodeStream( +
+ + + +
, + ); + }).toErrorDev( + 'renderToNodeStream is deprecated. Use renderToPipeableStream instead.', + {withoutStack: true}, + ); + await resolve(); + expect(response.read().toString()).toEqual( + '
resolved
', + ); + }); }); it('warns with a no-op when an async setState is triggered', () => { diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 35fbf0e6023c..fc35fdb28679 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -97,7 +97,7 @@ function renderToReadableStream( if (options && options.signal) { const signal = options.signal; const listener = () => { - abort(request); + abort(request, (signal: any).reason); signal.removeEventListener('abort', listener); }; signal.addEventListener('abort', listener); diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index c5318c2024aa..ce2b2e503086 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -28,8 +28,8 @@ function createDrainHandler(destination, request) { return () => startFlowing(request, destination); } -function createAbortHandler(request) { - return () => abort(request); +function createAbortHandler(request, reason) { + return () => abort(request, reason); } type Options = {| @@ -90,11 +90,26 @@ function renderToPipeableStream( hasStartedFlowing = true; startFlowing(request, destination); destination.on('drain', createDrainHandler(destination, request)); - destination.on('close', createAbortHandler(request)); + destination.on( + 'error', + createAbortHandler( + request, + // eslint-disable-next-line react-internal/prod-error-codes + new Error('The destination stream errored while writing data.'), + ), + ); + destination.on( + 'close', + createAbortHandler( + request, + // eslint-disable-next-line react-internal/prod-error-codes + new Error('The destination stream closed early.'), + ), + ); return destination; }, - abort() { - abort(request); + abort(reason) { + abort(request, reason); }, }; } diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js b/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js index 168f38fc59db..71786e4b5078 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js @@ -7,104 +7,36 @@ * @flow */ -import ReactVersion from 'shared/ReactVersion'; - import type {ReactNodeList} from 'shared/ReactTypes'; -import { - createRequest, - startWork, - startFlowing, - abort, -} from 'react-server/src/ReactFizzServer'; - -import { - createResponseState, - createRootFormatContext, -} from './ReactDOMServerLegacyFormatConfig'; +import {version, renderToStringImpl} from './ReactDOMLegacyServerImpl'; type ServerOptions = { identifierPrefix?: string, }; -function onError() { - // Non-fatal errors are ignored. -} - -function renderToStringImpl( - children: ReactNodeList, - options: void | ServerOptions, - generateStaticMarkup: boolean, -): string { - let didFatal = false; - let fatalError = null; - let result = ''; - const destination = { - push(chunk) { - if (chunk !== null) { - result += chunk; - } - return true; - }, - destroy(error) { - didFatal = true; - fatalError = error; - }, - }; - - let readyToStream = false; - function onShellReady() { - readyToStream = true; - } - const request = createRequest( - children, - createResponseState( - generateStaticMarkup, - options ? options.identifierPrefix : undefined, - ), - createRootFormatContext(), - Infinity, - onError, - undefined, - onShellReady, - undefined, - undefined, - ); - 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); - startFlowing(request, destination); - if (didFatal) { - throw fatalError; - } - - if (!readyToStream) { - // Note: This error message is the one we use on the client. It doesn't - // really make sense here. But this is the legacy server renderer, anyway. - // We're going to delete it soon. - throw new Error( - 'A component suspended while responding to synchronous input. This ' + - 'will cause the UI to be replaced with a loading indicator. To fix, ' + - 'updates that suspend should be wrapped with startTransition.', - ); - } - - return result; -} - function renderToString( children: ReactNodeList, options?: ServerOptions, ): string { - return 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() { @@ -126,5 +58,5 @@ export { renderToStaticMarkup, renderToNodeStream, renderToStaticNodeStream, - ReactVersion as version, + version, }; diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js b/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js new file mode 100644 index 000000000000..27e41b42e43a --- /dev/null +++ b/packages/react-dom/src/server/ReactDOMLegacyServerImpl.js @@ -0,0 +1,97 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import ReactVersion from 'shared/ReactVersion'; + +import type {ReactNodeList} from 'shared/ReactTypes'; + +import { + createRequest, + startWork, + startFlowing, + abort, +} from 'react-server/src/ReactFizzServer'; + +import { + createResponseState, + createRootFormatContext, +} from './ReactDOMServerLegacyFormatConfig'; + +type ServerOptions = { + identifierPrefix?: string, +}; + +function onError() { + // Non-fatal errors are ignored. +} + +function renderToStringImpl( + children: ReactNodeList, + options: void | ServerOptions, + generateStaticMarkup: boolean, + abortReason: string, +): string { + let didFatal = false; + let fatalError = null; + let result = ''; + const destination = { + push(chunk) { + if (chunk !== null) { + result += chunk; + } + return true; + }, + destroy(error) { + didFatal = true; + fatalError = error; + }, + }; + + let readyToStream = false; + function onShellReady() { + readyToStream = true; + } + const request = createRequest( + children, + createResponseState( + generateStaticMarkup, + options ? options.identifierPrefix : undefined, + ), + createRootFormatContext(), + Infinity, + onError, + undefined, + onShellReady, + undefined, + undefined, + ); + 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, abortReason); + startFlowing(request, destination); + if (didFatal) { + throw fatalError; + } + + if (!readyToStream) { + // Note: This error message is the one we use on the client. It doesn't + // really make sense here. But this is the legacy server renderer, anyway. + // We're going to delete it soon. + throw new Error( + 'A component suspended while responding to synchronous input. This ' + + 'will cause the UI to be replaced with a loading indicator. To fix, ' + + 'updates that suspend should be wrapped with startTransition.', + ); + } + + return result; +} + +export {renderToStringImpl, ReactVersion as version}; diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerNode.js b/packages/react-dom/src/server/ReactDOMLegacyServerNode.js index f5c6aa1f4600..26777fdc452c 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerNode.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerNode.js @@ -23,11 +23,7 @@ import { createRootFormatContext, } from './ReactDOMServerLegacyFormatConfig'; -import { - version, - renderToString, - renderToStaticMarkup, -} from './ReactDOMLegacyServerBrowser'; +import {version, renderToStringImpl} from './ReactDOMLegacyServerImpl'; import {Readable} from 'stream'; @@ -109,6 +105,30 @@ function renderToStaticNodeStream( return renderToNodeStreamImpl(children, options, true); } +function renderToString( + children: ReactNodeList, + options?: ServerOptions, +): string { + 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 "renderToPipeableStream" which supports Suspense on the server', + ); +} + +function renderToStaticMarkup( + children: ReactNodeList, + options?: ServerOptions, +): string { + 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 "renderToPipeableStream" which supports Suspense on the server', + ); +} + export { renderToString, renderToStaticMarkup, diff --git a/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js b/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js index 857a374f3e81..652bf9a783be 100644 --- a/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js +++ b/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js @@ -191,8 +191,6 @@ describe('ReactDOMServerFB', () => { const remaining = readResult(stream); expect(remaining).toEqual(''); - expect(errors).toEqual([ - 'This Suspense boundary was aborted by the server.', - ]); + expect(errors).toEqual(['signal is aborted without reason']); }); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 2b0a3d82e060..05ca9cd6c928 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1530,10 +1530,9 @@ function abortTaskSoft(task: Task): void { finishedTask(request, boundary, segment); } -function abortTask(task: Task): void { +function abortTask(task: Task, request: Request, reason: mixed): void { // This aborts the task and aborts the parent that it blocks, putting it into // client rendered mode. - const request: Request = this; const boundary = task.blockedBoundary; const segment = task.blockedSegment; segment.status = ABORTED; @@ -1553,12 +1552,28 @@ function abortTask(task: Task): void { if (!boundary.forceClientRender) { boundary.forceClientRender = true; - const error = new Error( - 'This Suspense boundary was aborted by the server.', - ); + let error = + reason === undefined + ? // eslint-disable-next-line react-internal/prod-error-codes + new Error('signal is aborted without reason') + : reason; boundary.errorDigest = request.onError(error); if (__DEV__) { - captureBoundaryErrorDetailsDev(boundary, error); + const errorPrefix = + 'The server did not finish this Suspense boundary: '; + if (error && typeof error.message === 'string') { + error = errorPrefix + error.message; + } else { + // eslint-disable-next-line react-internal/safe-string-coercion + error = errorPrefix + String(error); + } + const previousTaskInDev = currentTaskInDEV; + currentTaskInDEV = task; + try { + captureBoundaryErrorDetailsDev(boundary, error); + } finally { + currentTaskInDEV = previousTaskInDev; + } } if (boundary.parentFlushed) { request.clientRenderedBoundaries.push(boundary); @@ -1567,7 +1582,9 @@ function abortTask(task: Task): void { // If this boundary was still pending then we haven't already cancelled its fallbacks. // We'll need to abort the fallbacks, which will also error that parent boundary. - boundary.fallbackAbortableTasks.forEach(abortTask, request); + boundary.fallbackAbortableTasks.forEach(fallbackTask => + abortTask(fallbackTask, request, reason), + ); boundary.fallbackAbortableTasks.clear(); request.allPendingTasks--; @@ -2159,10 +2176,10 @@ export function startFlowing(request: Request, destination: Destination): void { } // This is called to early terminate a request. It puts all pending boundaries in client rendered state. -export function abort(request: Request): void { +export function abort(request: Request, reason: mixed): void { try { const abortableTasks = request.abortableTasks; - abortableTasks.forEach(abortTask, request); + abortableTasks.forEach(task => abortTask(task, request, reason)); abortableTasks.clear(); if (request.destination !== null) { flushCompletedQueues(request, request.destination); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 00748befe6d8..6c206f0cd6d1 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -419,5 +419,6 @@ "431": "React elements are not allowed in ServerContext", "432": "This Suspense boundary was aborted by the server.", "433": "useId can only be used while React is rendering", - "434": "`dangerouslySetInnerHTML` does not make sense on ." + "434": "`dangerouslySetInnerHTML` does not make sense on <title>.", + "435": "The server did not finish this Suspense boundary. The server used \"%s\" 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 \"%s\" which supports Suspense on the server" } diff --git a/scripts/shared/inlinedHostConfigs.js b/scripts/shared/inlinedHostConfigs.js index 4e47b4fd6ffc..3092f25391d0 100644 --- a/scripts/shared/inlinedHostConfigs.js +++ b/scripts/shared/inlinedHostConfigs.js @@ -69,6 +69,7 @@ module.exports = [ paths: [ 'react-dom', 'react-server-dom-webpack', + 'react-dom/src/server/ReactDOMLegacyServerImpl.js', // not an entrypoint, but only usable in *Brower and *Node files 'react-dom/src/server/ReactDOMLegacyServerBrowser.js', // react-dom/server.browser 'react-dom/src/server/ReactDOMLegacyServerNode.js', // react-dom/server.node 'react-client/src/ReactFlightClientStream.js', // We can only type check this in streaming configurations. From 2e785472d6c75e8bb891a16bcebb02bc5620a2f6 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Tue, 7 Jun 2022 14:20:43 -0700 Subject: [PATCH 2/6] fix gates --- .../react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 0d60ca59beaa..0258ec8ed21b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -397,7 +397,6 @@ describe('ReactDOMFizzServer', () => { }); }); - // @gate experimental describe('renderToString', () => { beforeEach(() => { JSDOM = require('jsdom').JSDOM; @@ -413,6 +412,7 @@ describe('ReactDOMFizzServer', () => { container = document.getElementById('container'); }); + // @gate experimental it('refers users to apis that support Suspense when somethign suspends', () => { function App({isClient}) { return ( From 0438912d9caa463b78a55d0d9fe7b1fe164e8dcf Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Tue, 7 Jun 2022 16:00:56 -0700 Subject: [PATCH 3/6] fix partialrenderer variants --- .../ReactDOMLegacyServerNode.classic.fb.js | 19 ++++ .../src/server/ReactDOMLegacyServerNode.js | 94 +--------------- .../server/ReactDOMLegacyServerNodeStream.js | 106 ++++++++++++++++++ scripts/shared/inlinedHostConfigs.js | 1 + 4 files changed, 130 insertions(+), 90 deletions(-) create mode 100644 packages/react-dom/src/server/ReactDOMLegacyServerNode.classic.fb.js create mode 100644 packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerNode.classic.fb.js b/packages/react-dom/src/server/ReactDOMLegacyServerNode.classic.fb.js new file mode 100644 index 000000000000..f542d77e295a --- /dev/null +++ b/packages/react-dom/src/server/ReactDOMLegacyServerNode.classic.fb.js @@ -0,0 +1,19 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export { + renderToString, + renderToStaticMarkup, + version, +} from './ReactDOMServerLegacyPartialRendererBrowser'; + +export { + renderToNodeStream, + renderToStaticNodeStream, +} from './ReactDOMLegacyServerNodeStream'; diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerNode.js b/packages/react-dom/src/server/ReactDOMLegacyServerNode.js index 26777fdc452c..20e89de8b430 100644 --- a/packages/react-dom/src/server/ReactDOMLegacyServerNode.js +++ b/packages/react-dom/src/server/ReactDOMLegacyServerNode.js @@ -9,102 +9,16 @@ import type {ReactNodeList} from 'shared/ReactTypes'; -import type {Request} from 'react-server/src/ReactFizzServer'; - -import { - createRequest, - startWork, - startFlowing, - abort, -} from 'react-server/src/ReactFizzServer'; - -import { - createResponseState, - createRootFormatContext, -} from './ReactDOMServerLegacyFormatConfig'; - import {version, renderToStringImpl} from './ReactDOMLegacyServerImpl'; - -import {Readable} from 'stream'; +import { + renderToNodeStream, + renderToStaticNodeStream, +} from './ReactDOMLegacyServerNodeStream'; type ServerOptions = { identifierPrefix?: string, }; -class ReactMarkupReadableStream extends Readable { - request: Request; - startedFlowing: boolean; - constructor() { - // Calls the stream.Readable(options) constructor. Consider exposing built-in - // features like highWaterMark in the future. - super({}); - this.request = (null: any); - this.startedFlowing = false; - } - - _destroy(err, callback) { - abort(this.request); - // $FlowFixMe: The type definition for the callback should allow undefined and null. - callback(err); - } - - _read(size) { - if (this.startedFlowing) { - startFlowing(this.request, this); - } - } -} - -function onError() { - // Non-fatal errors are ignored. -} - -function renderToNodeStreamImpl( - children: ReactNodeList, - options: void | ServerOptions, - generateStaticMarkup: boolean, -): Readable { - function onAllReady() { - // We wait until everything has loaded before starting to write. - // That way we only end up with fully resolved HTML even if we suspend. - destination.startedFlowing = true; - startFlowing(request, destination); - } - const destination = new ReactMarkupReadableStream(); - const request = createRequest( - children, - createResponseState(false, options ? options.identifierPrefix : undefined), - createRootFormatContext(), - Infinity, - onError, - onAllReady, - undefined, - undefined, - ); - destination.request = request; - startWork(request); - return destination; -} - -function renderToNodeStream( - children: ReactNodeList, - options?: ServerOptions, -): Readable { - if (__DEV__) { - console.error( - 'renderToNodeStream is deprecated. Use renderToPipeableStream instead.', - ); - } - return renderToNodeStreamImpl(children, options, false); -} - -function renderToStaticNodeStream( - children: ReactNodeList, - options?: ServerOptions, -): Readable { - return renderToNodeStreamImpl(children, options, true); -} - function renderToString( children: ReactNodeList, options?: ServerOptions, diff --git a/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js b/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js new file mode 100644 index 000000000000..25b88156e075 --- /dev/null +++ b/packages/react-dom/src/server/ReactDOMLegacyServerNodeStream.js @@ -0,0 +1,106 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {ReactNodeList} from 'shared/ReactTypes'; + +import type {Request} from 'react-server/src/ReactFizzServer'; + +import { + createRequest, + startWork, + startFlowing, + abort, +} from 'react-server/src/ReactFizzServer'; + +import { + createResponseState, + createRootFormatContext, +} from './ReactDOMServerLegacyFormatConfig'; + +import {Readable} from 'stream'; + +type ServerOptions = { + identifierPrefix?: string, +}; + +class ReactMarkupReadableStream extends Readable { + request: Request; + startedFlowing: boolean; + constructor() { + // Calls the stream.Readable(options) constructor. Consider exposing built-in + // features like highWaterMark in the future. + super({}); + this.request = (null: any); + this.startedFlowing = false; + } + + _destroy(err, callback) { + abort(this.request); + // $FlowFixMe: The type definition for the callback should allow undefined and null. + callback(err); + } + + _read(size) { + if (this.startedFlowing) { + startFlowing(this.request, this); + } + } +} + +function onError() { + // Non-fatal errors are ignored. +} + +function renderToNodeStreamImpl( + children: ReactNodeList, + options: void | ServerOptions, + generateStaticMarkup: boolean, +): Readable { + function onAllReady() { + // We wait until everything has loaded before starting to write. + // That way we only end up with fully resolved HTML even if we suspend. + destination.startedFlowing = true; + startFlowing(request, destination); + } + const destination = new ReactMarkupReadableStream(); + const request = createRequest( + children, + createResponseState(false, options ? options.identifierPrefix : undefined), + createRootFormatContext(), + Infinity, + onError, + onAllReady, + undefined, + undefined, + ); + destination.request = request; + startWork(request); + return destination; +} + +function renderToNodeStream( + children: ReactNodeList, + options?: ServerOptions, +): Readable { + if (__DEV__) { + console.error( + 'renderToNodeStream is deprecated. Use renderToPipeableStream instead.', + ); + } + return renderToNodeStreamImpl(children, options, false); +} + +function renderToStaticNodeStream( + children: ReactNodeList, + options?: ServerOptions, +): Readable { + return renderToNodeStreamImpl(children, options, true); +} + +export {renderToNodeStream, renderToStaticNodeStream}; diff --git a/scripts/shared/inlinedHostConfigs.js b/scripts/shared/inlinedHostConfigs.js index 3092f25391d0..34d1672193d4 100644 --- a/scripts/shared/inlinedHostConfigs.js +++ b/scripts/shared/inlinedHostConfigs.js @@ -72,6 +72,7 @@ module.exports = [ 'react-dom/src/server/ReactDOMLegacyServerImpl.js', // not an entrypoint, but only usable in *Brower and *Node files 'react-dom/src/server/ReactDOMLegacyServerBrowser.js', // react-dom/server.browser 'react-dom/src/server/ReactDOMLegacyServerNode.js', // react-dom/server.node + 'react-dom/src/server/ReactDOMLegacyServerNodeStream.js', // file indirection to support partial forking of some methods in *Node 'react-client/src/ReactFlightClientStream.js', // We can only type check this in streaming configurations. ], isFlowTyped: true, From 75e33fe35b8b3d750a1e98dc53b43ad8e54a3fd4 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Tue, 7 Jun 2022 16:35:05 -0700 Subject: [PATCH 4/6] properly gate test for partial renderer --- .../react-dom/src/__tests__/ReactServerRendering-test.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactServerRendering-test.js b/packages/react-dom/src/__tests__/ReactServerRendering-test.js index c22846fa637f..fed698807718 100644 --- a/packages/react-dom/src/__tests__/ReactServerRendering-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRendering-test.js @@ -14,6 +14,7 @@ let React; let ReactDOMServer; let PropTypes; let ReactCurrentDispatcher; +let useingPartialRenderer; describe('ReactDOMServer', () => { beforeEach(() => { @@ -24,6 +25,8 @@ describe('ReactDOMServer', () => { ReactCurrentDispatcher = React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED .ReactCurrentDispatcher; + + useingPartialRenderer = global.__WWW__ && !__EXPERIMENTAL__; }); describe('renderToString', () => { @@ -573,7 +576,11 @@ describe('ReactDOMServer', () => { <Suspender /> </React.Suspense>, ); - expect(response).toEqual('fallback'); + if (useingPartialRenderer) { + expect(response).toEqual('<!--$!-->fallback<!--/$-->'); + } else { + expect(response).toEqual('fallback'); + } }); }); From 06354fc8c49f86c648d8013bfc50720fee5daf42 Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Tue, 7 Jun 2022 16:39:57 -0700 Subject: [PATCH 5/6] exclude fb classic variant from other flows --- scripts/shared/inlinedHostConfigs.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/shared/inlinedHostConfigs.js b/scripts/shared/inlinedHostConfigs.js index 34d1672193d4..eb0eef910983 100644 --- a/scripts/shared/inlinedHostConfigs.js +++ b/scripts/shared/inlinedHostConfigs.js @@ -72,6 +72,7 @@ module.exports = [ 'react-dom/src/server/ReactDOMLegacyServerImpl.js', // not an entrypoint, but only usable in *Brower and *Node files 'react-dom/src/server/ReactDOMLegacyServerBrowser.js', // react-dom/server.browser 'react-dom/src/server/ReactDOMLegacyServerNode.js', // react-dom/server.node + 'react-dom/src/server/ReactDOMLegacyServerNode.classic.fb.js', 'react-dom/src/server/ReactDOMLegacyServerNodeStream.js', // file indirection to support partial forking of some methods in *Node 'react-client/src/ReactFlightClientStream.js', // We can only type check this in streaming configurations. ], From cdeeb04c0f61731596ab45ff7de7fcb6811004ab Mon Sep 17 00:00:00 2001 From: Josh Story <story@hey.com> Date: Tue, 7 Jun 2022 21:43:20 -0700 Subject: [PATCH 6/6] update error message to make more sense in node contexts --- .../react-dom/src/__tests__/ReactDOMFizzServer-test.js | 2 +- .../src/__tests__/ReactDOMFizzServerBrowser-test.js | 8 ++++++-- .../src/__tests__/ReactDOMFizzServerNode-test.js | 4 ++-- .../src/__tests__/ReactDOMServerFB-test.internal.js | 4 +++- packages/react-server/src/ReactFizzServer.js | 3 +-- scripts/error-codes/codes.json | 7 +++---- 6 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index c06c506f9680..703e3280da58 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1108,7 +1108,7 @@ describe('ReactDOMFizzServer', () => { errors, [ [ - 'The server did not finish this Suspense boundary: signal is aborted without reason', + 'The server did not finish this Suspense boundary: The render was aborted by the server without a reason.', expectedDigest, componentStack(['h1', 'Suspense', 'div', 'App']), ], diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js index 0258ec8ed21b..7ece6de67c28 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js @@ -222,7 +222,9 @@ describe('ReactDOMFizzServer', () => { const result = await readResult(stream); expect(result).toContain('Loading'); - expect(errors).toEqual(['signal is aborted without reason']); + expect(errors).toEqual([ + 'The render was aborted by the server without a reason.', + ]); }); // @gate experimental @@ -261,7 +263,9 @@ describe('ReactDOMFizzServer', () => { const reader = stream.getReader(); reader.cancel(); - expect(errors).toEqual(['signal is aborted without reason']); + expect(errors).toEqual([ + 'The render was aborted by the server without a reason.', + ]); hasLoaded = true; resolve(); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js index 96dd22c4bafc..303275bab5ec 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js @@ -363,8 +363,8 @@ describe('ReactDOMFizzServer', () => { expect(errors).toEqual([ // There are two boundaries that abort - 'signal is aborted without reason', - 'signal is aborted without reason', + 'The render was aborted by the server without a reason.', + 'The render was aborted by the server without a reason.', ]); expect(output.error).toBe(undefined); expect(output.result).toContain('Loading'); diff --git a/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js b/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js index 652bf9a783be..53de82a1ec28 100644 --- a/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js +++ b/packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js @@ -191,6 +191,8 @@ describe('ReactDOMServerFB', () => { const remaining = readResult(stream); expect(remaining).toEqual(''); - expect(errors).toEqual(['signal is aborted without reason']); + expect(errors).toEqual([ + 'The render was aborted by the server without a reason.', + ]); }); }); diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 05ca9cd6c928..9e6551d938c0 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1554,8 +1554,7 @@ function abortTask(task: Task, request: Request, reason: mixed): void { boundary.forceClientRender = true; let error = reason === undefined - ? // eslint-disable-next-line react-internal/prod-error-codes - new Error('signal is aborted without reason') + ? new Error('The render was aborted by the server without a reason.') : reason; boundary.errorDigest = request.onError(error); if (__DEV__) { diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 6c206f0cd6d1..48afd882a0c7 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -417,8 +417,7 @@ "429": "ServerContext: %s already defined", "430": "ServerContext can only have a value prop and children. Found: %s", "431": "React elements are not allowed in ServerContext", - "432": "This Suspense boundary was aborted by the server.", + "432": "The render was aborted by the server without a reason.", "433": "useId can only be used while React is rendering", - "434": "`dangerouslySetInnerHTML` does not make sense on <title>.", - "435": "The server did not finish this Suspense boundary. The server used \"%s\" 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 \"%s\" which supports Suspense on the server" -} + "434": "`dangerouslySetInnerHTML` does not make sense on <title>." +} \ No newline at end of file