From 7ffca015803560308aa0b8d440e7fad2871c5e49 Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Fri, 22 Feb 2019 18:45:23 +0000 Subject: [PATCH] Continue rendering within suspense boundary once fallback triggered [major] --- README.md | 2 - lib/renderer.js | 152 +++++++++++++++++++++++++++++----------------- test/lazy.test.js | 6 +- 3 files changed, 100 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index affa1ee..afa55e9 100644 --- a/README.md +++ b/README.md @@ -188,8 +188,6 @@ It's possible for a lazy component to begin loading, but then its result not to In these cases, if the promise has an `.abort()` method, it will be called. -When a Suspense boundary's fallback is triggered, rendering of all further elements within the boundary is abandoned. - ### Additional notes #### Stream rendering diff --git a/lib/renderer.js b/lib/renderer.js index 939b4a9..722806c 100644 --- a/lib/renderer.js +++ b/lib/renderer.js @@ -42,7 +42,10 @@ class PartialRenderer extends ReactDOMServerRenderer { }; this.tree = tree; this.node = tree; + + // Init suspense state this.suspenseNode = null; + this.suspended = false; // Init lazy node awaiting counter this.numAwaiting = 0; @@ -121,7 +124,7 @@ class PartialRenderer extends ReactDOMServerRenderer { this.errored(err); } - if (this.numAwaiting !== 0) return; + if (this.numAwaiting !== 0 || this.suspended) return; // Finished processing this.destroy(); @@ -302,7 +305,9 @@ class PartialRenderer extends ReactDOMServerRenderer { if (fallback !== undefined) { node = this.createChildWithStackState(TYPE_SUSPENSE, frame); node.fallback = fallback; + node.suspended = false; this.suspenseNode = node; + this.suspended = false; } else { node = this.createChild(null); } @@ -317,12 +322,23 @@ class PartialRenderer extends ReactDOMServerRenderer { } handlePromise(element, promise) { - if (promise[NO_SSR]) return this.handleNoSsrPromise(promise); - // Pop frame from stack const {stack} = this; const frame = this.stackPopOriginal.call(stack); + // If not rendering because inside suspense boundary which is suspended, + // abort promise and ignore + // TODO Check this works if promise resolves sync. + // I think it won't because on client side it will render immediately + // and that could include more lazy elements. + if (this.suspended) { + followAndAbort(promise); + return; + } + + // Handle no SSR promise + if (promise[NO_SSR]) return this.handleNoSsrPromise(promise); + // Follow promise let node; promise.then( @@ -381,16 +397,39 @@ class PartialRenderer extends ReactDOMServerRenderer { node.resolved = true; this.numAwaiting--; + // Re-render element + this.rerender(node, node.element); + } + + rerender(node, element) { // Step into node and reinstate stack state with element on stack ready to render this.node = node; - this.suspenseNode = node.parentSuspense; - this.restoreStack(node.stackState, node.element); + const suspenseNode = node.parentSuspense; + this.suspenseNode = suspenseNode; + this.suspended = suspenseNode ? suspenseNode.suspended : false; + this.restoreStack(node.stackState, element); // Render element this.cycle(); + // If errored, exit + if (this.exhausted) return; + // Clear contexts added in `.restoreStack()` this.resetProviders(); + + // If suspended, render fallback of suspense boundary + if (!this.suspended) return; + + // Convert node to fallback and discard children + const {fallback} = suspenseNode; + this.convertNodeToFallback(suspenseNode); + + // Render fallback + if (isRenderableElement(fallback)) this.rerender(suspenseNode, fallback); + + // Clear frame to free memory + suspenseNode.frame = null; } rejected(node, err) { @@ -436,62 +475,18 @@ class PartialRenderer extends ReactDOMServerRenderer { handleNoSsrPromise(promise) { // Abort this promise - let resolved = false; - const resolve = () => resolved = true; - promise.then(resolve, resolve); - if (!resolved) abort(promise); + followAndAbort(promise); // If no enclosing suspense boundary, exit with error - const {node, suspenseNode} = this; + const {suspenseNode} = this; if (!suspenseNode) return this.halt(promise); // Abort all promises within boundary this.abortDescendents(suspenseNode); - // Convert boundary node to fallback node (+ remove all children) - const {frame: suspenseFrame, fallback, stackState} = suspenseNode; - suspenseNode.type = null; - suspenseNode.children.length = 0; - suspenseNode.fallback = null; - suspenseNode.stackState = null; - - // Move back down tree into this node - this.node = suspenseNode; - this.suspenseNode = suspenseNode.parentSuspense; - this.previousWasTextNode = false; - - // If boundary not in current render cycle, abandon this render cycle, - // and render fallback. - if (!suspenseFrame) { - // Clear current render cycle state - this.stack.length = 1; - this.resetProviders(); - - // Restore stack and render fallback. - // NB Render cycle already in progress, so no need to call `.read()` again. - if (isRenderableElement(fallback)) this.restoreStack(stackState, fallback); - return; - } - - // Boundary is in current render cycle. - // Prevent further rendering of all children in frames down to suspense boundary. - const {stack} = this; - let thisNode = node; - for (let i = stack.length - 1; i >= 0; i--) { - const frame = stack[i]; - frame.childIndex = frame.children.length; - frame._footer = ''; - - if (frame === suspenseFrame) break; - - if (frame === thisNode.frame) { - this.nonDomFrames--; - thisNode = thisNode.parent; - } - } - - // Add fallback to stack ready to render - if (isRenderableElement(fallback)) suspenseFrame.children.push(fallback); + // Suspend + this.suspended = true; + suspenseNode.suspended = true; } createChildWithStackState(type, frame) { @@ -550,9 +545,9 @@ class PartialRenderer extends ReactDOMServerRenderer { if (frame === node.frame) { node.frame = null; this.node = node.parent; - if (node === this.suspenseNode) this.suspenseNode = node.parentSuspense; this.nonDomFrames--; this.previousWasTextNode = false; + if (node === this.suspenseNode) this.popSuspense(node, frame); } else if (frame._footer !== undefined) { this.output(frame._footer); } @@ -560,6 +555,41 @@ class PartialRenderer extends ReactDOMServerRenderer { return frame; } + popSuspense(node, frame) { + const {suspended} = this, + {fallback} = node; + + // Step down suspense stack + const {parentSuspense} = node; + this.suspenseNode = parentSuspense; + this.suspended = parentSuspense ? parentSuspense.suspended : false; + + if (!suspended) return; + + // Was suspended + // Convert node to fallback + this.convertNodeToFallback(node); + node.stackState = null; + + if (!isRenderableElement(fallback)) return; + + // Render fallback + frame.children = toArray(fallback); + frame.childIndex = 0; + + this.stack.push(frame); + this.nonDomFrames++; + + this.node = node; + node.frame = frame; + } + + convertNodeToFallback(node) { + node.type = null; + node.fallback = null; + node.children.length = 0; + } + renderDOM(element, context, parentNamespace) { // Render let out = super.renderDOM(element, context, parentNamespace); @@ -633,6 +663,18 @@ class PartialRenderer extends ReactDOMServerRenderer { } } +/** + * Follow promise (to catch rejections) and abort if does not resolve synchronously + * @param {Promise} promise - Promise to abort + * @returns {undefined} + */ +function followAndAbort(promise) { + let resolved = false; + const resolve = () => resolved = true; + promise.then(resolve, resolve); + if (!resolved) abort(promise); +} + /** * Abort promise. * @param {Promise} promise - Promise to abort diff --git a/test/lazy.test.js b/test/lazy.test.js index 46e9843..d44060b 100644 --- a/test/lazy.test.js +++ b/test/lazy.test.js @@ -423,7 +423,7 @@ describe('multiple lazy components', () => { `)); }); - itRenders('prevents later elements being rendered', async ({render, openTag}) => { + itRenders('still renders later elements', async ({render, openTag}) => { const Lazy1 = spy(lazy(() =>
Lazy inner 1
)); const Lazy2 = spy(lazy(() =>
Lazy inner 2
, {noSsr: true})); const Lazy3 = spy(lazy(() =>
Lazy inner 3
)); @@ -444,7 +444,7 @@ describe('multiple lazy components', () => { expect(Lazy1).toHaveBeenCalled(); expect(Lazy2).toHaveBeenCalled(); - expect(Lazy3).not.toHaveBeenCalled(); + expect(Lazy3).toHaveBeenCalled(); expect(Lazy4).toHaveBeenCalled(); expect(h).toBe(removeSpacing(` @@ -505,7 +505,7 @@ describe('multiple lazy components', () => { expect(Lazy1.promise.abort).toHaveBeenCalledTimes(1); expect(Lazy2.promise.abort).toHaveBeenCalledTimes(1); - expect(Lazy3.promise).toBeUndefined(); + expect(Lazy3.promise.abort).toHaveBeenCalledTimes(1); expect(Lazy4.promise.abort).not.toHaveBeenCalled(); const h = await p;