Rename Node SSR Callbacks to onShellReady/onAllReady and Other Fixes (#24030)

* I forgot to call onFatalError
I can't figure out how to write a test for this because it only happens
when there is a bug in React itself which would then be fixed if we found
it.
We're also covered by the protection of ReadableStream which doesn't leak
other errors to us.
* Abort requests if the reader cancels
No need to continue computing at this point.
* Abort requests if node streams get destroyed
This is if the downstream cancels is for example.
* Rename Node APIs for Parity with allReady
The "Complete" terminology is a little misleading because not everything
has been written yet. It's just "Ready" to be written now.
onShellReady
onShellError
onAllReady
* 'close' should be enough

Author: sebmarkbage

fixtures/ssr/server/render.js

@@ -22,13 +22,13 @@ export default function render(url, res) {
 letdidError=false;
 const{pipe, abort}=renderToPipeableStream(<Appassets={assets}/>,{
 bootstrapScripts: [assets['main.js']],
-onCompleteShell(){
+onShellReady(){
 // If something errored before we started streaming, we set the error code appropriately.
 res.statusCode=didError ? 500 : 200;
 res.setHeader('Content-type','text/html');
 pipe(res);
 },
-onErrorShell(x){
+onShellError(x){
 // Something errored before we could complete the shell so we emit an alternative shell.
 res.statusCode=500;
 res.send('<!doctype><p>Error</p>');

fixtures/ssr2/server/render.js

@@ -49,7 +49,7 @@ module.exports = function render(url, res) {
 res.setHeader('Content-type','text/html');
 pipe(res);
 },
-onErrorShell(x){
+onShellError(x){
 // Something errored before we could complete the shell so we emit an alternative shell.
 res.statusCode=500;
 res.send('<!doctype><p>Error</p>');

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

@@ -914,7 +914,7 @@ describe('ReactDOMFizzServer', () => {
 </Suspense>,
 {
 identifierPrefix: 'A_',
-onCompleteShell(){
+onShellReady(){
 writableA.write('<div id="container-A">');
 pipe(writableA);
 writableA.write('</div>');
@@ -933,7 +933,7 @@ describe('ReactDOMFizzServer', () => {
 </Suspense>,
 {
 identifierPrefix: 'B_',
-onCompleteShell(){
+onShellReady(){
 writableB.write('<div id="container-B">');
 pipe(writableB);
 writableB.write('</div>');
@@ -1168,7 +1168,7 @@ describe('ReactDOMFizzServer', () => {
 
 {
 namespaceURI: 'http://www.w3.org/2000/svg',
-onCompleteShell(){
+onShellReady(){
 writable.write('<svg>');
 pipe(writable);
 writable.write('</svg>');

packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js

@@ -209,4 +209,43 @@ describe('ReactDOMFizzServer', () => {
 constresult=awaitreadResult(stream);
 expect(result).toContain('Loading');
 });
+
+// @gate experimental
+it('should not continue rendering after the reader cancels',async()=>{
+lethasLoaded=false;
+letresolve;
+letisComplete=false;
+letrendered=false;
+constpromise=newPromise(r=>(resolve=r));
+functionWait(){
+if(!hasLoaded){
+throwpromise;
+}
+rendered=true;
+return'Done';
+}
+conststream=awaitReactDOMFizzServer.renderToReadableStream(
+<div>
+<Suspensefallback={<div>Loading</div>}>
+<Wait/> />
+</Suspense>
+</div>,
+);
+
+stream.allReady.then(()=>(isComplete=true));
+
+expect(rendered).toBe(false);
+expect(isComplete).toBe(false);
+
+constreader=stream.getReader();
+reader.cancel();
+
+hasLoaded=true;
+resolve();
+
+awaitjest.runAllTimers();
+
+expect(rendered).toBe(false);
+expect(isComplete).toBe(true);
+});
 });

packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js

@@ -138,7 +138,7 @@ describe('ReactDOMFizzServer', () => {
 </div>,
 
 {
-onCompleteAll(){
+onAllReady(){
 isCompleteCalls++;
 },
 },
@@ -179,7 +179,7 @@ describe('ReactDOMFizzServer', () => {
 onError(x){
 reportedErrors.push(x);
 },
-onErrorShell(x){
+onShellError(x){
 reportedShellErrors.push(x);
 },
 },
@@ -213,7 +213,7 @@ describe('ReactDOMFizzServer', () => {
 onError(x){
 reportedErrors.push(x);
 },
-onErrorShell(x){
+onShellError(x){
 reportedShellErrors.push(x);
 },
 },
@@ -244,7 +244,7 @@ describe('ReactDOMFizzServer', () => {
 onError(x){
 reportedErrors.push(x);
 },
-onErrorShell(x){
+onShellError(x){
 reportedShellErrors.push(x);
 },
 },
@@ -298,7 +298,7 @@ describe('ReactDOMFizzServer', () => {
 </div>,
 
 {
-onCompleteAll(){
+onAllReady(){
 isCompleteCalls++;
 },
 },
@@ -333,7 +333,7 @@ describe('ReactDOMFizzServer', () => {
 </div>,
 
 {
-onCompleteAll(){
+onAllReady(){
 isCompleteCalls++;
 },
 },
@@ -537,4 +537,49 @@ describe('ReactDOMFizzServer', () => {
 expect(output.result).not.toContain('context never found');
 expect(output.result).toContain('OK');
 });
+
+// @gate experimental
+it('should not continue rendering after the writable ends unexpectedly',async()=>{
+lethasLoaded=false;
+letresolve;
+letisComplete=false;
+letrendered=false;
+constpromise=newPromise(r=>(resolve=r));
+functionWait(){
+if(!hasLoaded){
+throwpromise;
+}
+rendered=true;
+return'Done';
+}
+const{writable, completed}=getTestWritable();
+const{pipe}=ReactDOMFizzServer.renderToPipeableStream(
+<div>
+<Suspensefallback={<div>Loading</div>}>
+<Wait/>
+</Suspense>
+</div>,
+{
+onAllReady(){
+isComplete=true;
+},
+},
+);
+pipe(writable);
+
+expect(rendered).toBe(false);
+expect(isComplete).toBe(false);
+
+writable.end();
+
+awaitjest.runAllTimers();
+
+hasLoaded=true;
+resolve();
+
+awaitcompleted;
+
+expect(rendered).toBe(false);
+expect(isComplete).toBe(true);
+});
 });

packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js

@@ -155,7 +155,7 @@ module.exports = function(initModules) {
 newPromise((resolve,reject)=>{
 constwritable=newDrainWritable();
 consts=ReactDOMServer.renderToPipeableStream(reactElement,{
-onErrorShell(e){
+onShellError(e){
 reject(e);
 },
 });

packages/react-dom/src/server/ReactDOMFizzServerBrowser.js

@@ -46,25 +46,27 @@ function renderToReadableStream(
 ): Promise<ReactDOMServerReadableStream>{
 returnnewPromise((resolve,reject)=>{
 letonFatalError;
-letonCompleteAll;
+letonAllReady;
 constallReady=newPromise((res,rej)=>{
-onCompleteAll=res;
+onAllReady=res;
 onFatalError=rej;
 });
 
-functiononCompleteShell(){
+functiononShellReady(){
 conststream: ReactDOMServerReadableStream=(newReadableStream({
 type: 'bytes',
 pull(controller){
 startFlowing(request,controller);
 },
-cancel(reason){},
+cancel(reason){
+abort(request);
+},
 }): any);
 // TODO: Move to sub-classing ReadableStream.
 stream.allReady=allReady;
 resolve(stream);
 }
-functiononErrorShell(error: mixed){
+functiononShellError(error: mixed){
 reject(error);
 }
 constrequest=createRequest(
@@ -79,9 +81,9 @@ function renderToReadableStream(
 createRootFormatContext(options ? options.namespaceURI : undefined),
 options ? options.progressiveChunkSize : undefined,
 options ? options.onError : undefined,
-onCompleteAll,
-onCompleteShell,
-onErrorShell,
+onAllReady,
+onShellReady,
+onShellError,
 onFatalError,
 );
 if(options&&options.signal){

packages/react-dom/src/server/ReactDOMFizzServerNode.js

@@ -28,6 +28,10 @@ function createDrainHandler(destination, request) {
 return()=>startFlowing(request,destination);
 }
 
+functioncreateAbortHandler(request){
+return()=>abort(request);
+}
+
 typeOptions={|
 identifierPrefix?: string,
 namespaceURI?: string,
@@ -36,9 +40,9 @@ type Options = {|
 bootstrapScripts?: Array<string>,
 bootstrapModules?: Array<string>,
 progressiveChunkSize?: number,
-onCompleteShell?: ()=>void,
-onErrorShell?: ()=>void,
-onCompleteAll?: ()=>void,
+onShellReady?: ()=>void,
+onShellError?: ()=>void,
+onAllReady?: ()=>void,
 onError?: (error: mixed)=>void,
 |};
 
@@ -62,9 +66,9 @@ function createRequestImpl(children: ReactNodeList, options: void | Options) {
 createRootFormatContext(options ? options.namespaceURI : undefined),
 options ? options.progressiveChunkSize : undefined,
 options ? options.onError : undefined,
-options ? options.onCompleteAll : undefined,
-options ? options.onCompleteShell : undefined,
-options ? options.onErrorShell : undefined,
+options ? options.onAllReady : undefined,
+options ? options.onShellReady : undefined,
+options ? options.onShellError : undefined,
 undefined,
 );
 }
@@ -86,6 +90,7 @@ function renderToPipeableStream(
  hasStartedFlowing = true;
  startFlowing(request, destination);
  destination.on('drain', createDrainHandler(destination, request));
+ destination.on('close', createAbortHandler(request));
  return destination;
 },
 abort(){

packages/react-dom/src/server/ReactDOMLegacyServerBrowser.js

@@ -53,7 +53,7 @@ function renderToStringImpl(
 };
 
 letreadyToStream=false;
-functiononCompleteShell(){
+functiononShellReady(){
 readyToStream=true;
 }
 constrequest=createRequest(
@@ -66,7 +66,7 @@ function renderToStringImpl(
 Infinity,
 onError,
 undefined,
-onCompleteShell,
+onShellReady,
 undefined,
 undefined,
 );

packages/react-dom/src/server/ReactDOMLegacyServerNode.js

@@ -68,7 +68,7 @@ function renderToNodeStreamImpl(
 options: void|ServerOptions,
 generateStaticMarkup: boolean,
 ): Readable{
-functiononCompleteAll(){
+functiononAllReady(){
 // 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;
@@ -81,7 +81,7 @@ function renderToNodeStreamImpl(
 createRootFormatContext(),
 Infinity,
 onError,
-onCompleteAll,
+onAllReady,
 undefined,
 undefined,
 );