From b53f368b498e99b102c3fd832251ac56fff054ac Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sat, 13 Jul 2024 10:13:41 +0200 Subject: [PATCH 1/3] Denote suspenseful components with comment markers --- src/index.js | 41 ++++++++++-- test/compat/async.test.jsx | 126 +++++++++++++++++++++++++++++++++++-- 2 files changed, 156 insertions(+), 11 deletions(-) diff --git a/src/index.js b/src/index.js index ff4c774e..35e642da 100644 --- a/src/index.js +++ b/src/index.js @@ -29,6 +29,8 @@ const EMPTY_ARR = []; const isArray = Array.isArray; const assign = Object.assign; const EMPTY_STR = ''; +const BEGIN_SUSPENSE_DENOMINATOR = ''; +const END_SUSPENSE_DENOMINATOR = ''; // Global state for the current render pass let beforeDiff, afterDiff, renderHook, ummountHook; @@ -372,7 +374,12 @@ function _renderToString( if (renderHook) renderHook(vnode); - rendered = type.call(component, props, cctx); + try { + rendered = type.call(component, props, cctx); + } catch (e) { + if (asyncMode) vnode._suspended = true; + throw e; + } } component[DIRTY] = true; } @@ -403,6 +410,7 @@ function _renderToString( selectValue, vnode, asyncMode, + false, renderer ); } catch (err) { @@ -475,6 +483,21 @@ function _renderToString( if (options.unmount) options.unmount(vnode); + if (vnode._suspended) { + if (typeof str === 'string') { + return BEGIN_SUSPENSE_DENOMINATOR + str + END_SUSPENSE_DENOMINATOR; + } else if (isArray(str)) { + str.unshift(BEGIN_SUSPENSE_DENOMINATOR); + str.push(END_SUSPENSE_DENOMINATOR); + return str; + } + + return str.then( + (resolved) => + BEGIN_SUSPENSE_DENOMINATOR + resolved + END_SUSPENSE_DENOMINATOR + ); + } + return str; } catch (error) { if (!asyncMode && renderer && renderer.onError) { @@ -503,7 +526,7 @@ function _renderToString( const renderNestedChildren = () => { try { - return _renderToString( + const result = _renderToString( rendered, context, isSvgMode, @@ -512,12 +535,15 @@ function _renderToString( asyncMode, renderer ); + return vnode._suspended + ? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR + : result; } catch (e) { if (!e || typeof e.then != 'function') throw e; return e.then( - () => - _renderToString( + () => { + const result = _renderToString( rendered, context, isSvgMode, @@ -525,8 +551,11 @@ function _renderToString( vnode, asyncMode, renderer - ), - renderNestedChildren + ) + return vnode._suspended + ? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR + : result; + }, renderNestedChildren ); } }; diff --git a/test/compat/async.test.jsx b/test/compat/async.test.jsx index 12e9ac5a..bf1792bf 100644 --- a/test/compat/async.test.jsx +++ b/test/compat/async.test.jsx @@ -3,6 +3,7 @@ import { h, Fragment } from 'preact'; import { Suspense, useId, lazy, createContext } from 'preact/compat'; import { expect } from 'chai'; import { createSuspender } from '../utils.jsx'; +const wait = (ms) => new Promise((r) => setTimeout(r, ms)); describe('Async renderToString', () => { it('should render JSX after a suspense boundary', async () => { @@ -16,7 +17,30 @@ describe('Async renderToString', () => { ); - const expected = `
bar
`; + const expected = `
bar
`; + + suspended.resolve(); + + const rendered = await promise; + + expect(rendered).to.equal(expected); + }); + + it('should correctly denote null returns of suspending components', async () => { + const { Suspender, suspended } = createSuspender(); + + const Analytics = () => null; + + const promise = renderToStringAsync( + loading...}> + + + +
bar
+ + ); + + const expected = `
bar
`; suspended.resolve(); @@ -49,7 +73,7 @@ describe('Async renderToString', () => { ); - const expected = ``; + const expected = ``; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -85,7 +109,7 @@ describe('Async renderToString', () => { ); - const expected = ``; + const expected = ``; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -95,6 +119,98 @@ describe('Async renderToString', () => { expect(rendered).to.equal(expected); }); + it('should render JSX with nested suspense boundaries containing multiple suspending components', async () => { + const { + Suspender: SuspenderOne, + suspended: suspendedOne + } = createSuspender(); + const { + Suspender: SuspenderTwo, + suspended: suspendedTwo + } = createSuspender(); + const { + Suspender: SuspenderThree, + suspended: suspendedThree + } = createSuspender('three'); + + const promise = renderToStringAsync( + + ); + + const expected = ``; + + suspendedOne.resolve(); + suspendedTwo.resolve(); + await wait(0); + suspendedThree.resolve(); + + const rendered = await promise; + + expect(rendered).to.equal(expected); + }); + + it.only('should render JSX with deeply nested suspense boundaries', async () => { + const { + Suspender: SuspenderOne, + suspended: suspendedOne + } = createSuspender(); + const { + Suspender: SuspenderTwo, + suspended: suspendedTwo + } = createSuspender(); + const { + Suspender: SuspenderThree, + suspended: suspendedThree + } = createSuspender(); + + const promise = renderToStringAsync( + + ); + + const expected = ``; + + suspendedOne.resolve(); + suspendedTwo.resolve(); + await wait(0); + suspendedThree.resolve(); + + const rendered = await promise; + + expect(rendered).to.equal(expected); + }); + it('should render JSX with multiple suspended direct children within a single suspense boundary', async () => { const { Suspender: SuspenderOne, @@ -127,7 +243,7 @@ describe('Async renderToString', () => { ); - const expected = ``; + const expected = ``; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -187,7 +303,7 @@ describe('Async renderToString', () => { suspended.resolve(); const rendered = await promise; - expect(rendered).to.equal('

ok

'); + expect(rendered).to.equal('

ok

'); }); it('should work with an in-render suspension', async () => { From 6017e2d0233e5b903958680ea13781349b63b01f Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sun, 14 Jul 2024 09:00:59 +0200 Subject: [PATCH 2/3] Add changeset --- .changeset/happy-peas-type.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/happy-peas-type.md diff --git a/.changeset/happy-peas-type.md b/.changeset/happy-peas-type.md new file mode 100644 index 00000000..fa929705 --- /dev/null +++ b/.changeset/happy-peas-type.md @@ -0,0 +1,5 @@ +--- +'preact-render-to-string': minor +--- + +Insert comment markers for suspended trees, only in renderToStringAsync From 0da840e2b95445a863609aad8a0948ec9d0d5d33 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Mon, 15 Jul 2024 11:09:34 +0200 Subject: [PATCH 3/3] use shorter notation --- src/index.js | 38 +++++++++++++++++++------------------- test/compat/async.test.jsx | 23 +++++++++++++---------- 2 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/index.js b/src/index.js index 35e642da..f6530f4f 100644 --- a/src/index.js +++ b/src/index.js @@ -29,8 +29,8 @@ const EMPTY_ARR = []; const isArray = Array.isArray; const assign = Object.assign; const EMPTY_STR = ''; -const BEGIN_SUSPENSE_DENOMINATOR = ''; -const END_SUSPENSE_DENOMINATOR = ''; +const BEGIN_SUSPENSE_DENOMINATOR = ''; +const END_SUSPENSE_DENOMINATOR = ''; // Global state for the current render pass let beforeDiff, afterDiff, renderHook, ummountHook; @@ -377,7 +377,9 @@ function _renderToString( try { rendered = type.call(component, props, cctx); } catch (e) { - if (asyncMode) vnode._suspended = true; + if (asyncMode) { + vnode._suspended = true; + } throw e; } } @@ -541,22 +543,20 @@ function _renderToString( } catch (e) { if (!e || typeof e.then != 'function') throw e; - return e.then( - () => { - const result = _renderToString( - rendered, - context, - isSvgMode, - selectValue, - vnode, - asyncMode, - renderer - ) - return vnode._suspended - ? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR - : result; - }, renderNestedChildren - ); + return e.then(() => { + const result = _renderToString( + rendered, + context, + isSvgMode, + selectValue, + vnode, + asyncMode, + renderer + ); + return vnode._suspended + ? BEGIN_SUSPENSE_DENOMINATOR + result + END_SUSPENSE_DENOMINATOR + : result; + }, renderNestedChildren); } }; diff --git a/test/compat/async.test.jsx b/test/compat/async.test.jsx index bf1792bf..0ca78352 100644 --- a/test/compat/async.test.jsx +++ b/test/compat/async.test.jsx @@ -17,7 +17,7 @@ describe('Async renderToString', () => { ); - const expected = `
bar
`; + const expected = `
bar
`; suspended.resolve(); @@ -40,7 +40,7 @@ describe('Async renderToString', () => { ); - const expected = `
bar
`; + const expected = `
bar
`; suspended.resolve(); @@ -73,7 +73,7 @@ describe('Async renderToString', () => { ); - const expected = `
  • one
  • two
  • three
`; + const expected = `
  • one
  • two
  • three
`; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -109,7 +109,7 @@ describe('Async renderToString', () => { ); - const expected = `
  • one
  • two
  • three
`; + const expected = `
  • one
  • two
  • three
`; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -152,7 +152,7 @@ describe('Async renderToString', () => { ); - const expected = `
  • one
  • two
  • three
  • four
`; + const expected = `
  • one
  • two
  • three
  • four
`; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -164,7 +164,7 @@ describe('Async renderToString', () => { expect(rendered).to.equal(expected); }); - it.only('should render JSX with deeply nested suspense boundaries', async () => { + it('should render JSX with deeply nested suspense boundaries', async () => { const { Suspender: SuspenderOne, suspended: suspendedOne @@ -199,7 +199,7 @@ describe('Async renderToString', () => { ); - const expected = `
  • one
  • two
  • three
  • four
`; + const expected = `
  • one
  • two
  • three
  • four
`; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -243,7 +243,7 @@ describe('Async renderToString', () => { ); - const expected = `
  • one
  • two
  • three
`; + const expected = `
  • one
  • two
  • three
`; suspendedOne.resolve(); suspendedTwo.resolve(); @@ -303,7 +303,7 @@ describe('Async renderToString', () => { suspended.resolve(); const rendered = await promise; - expect(rendered).to.equal('

ok

'); + expect(rendered).to.equal('

ok

'); }); it('should work with an in-render suspension', async () => { @@ -340,7 +340,10 @@ describe('Async renderToString', () => { ); - expect(rendered).to.equal(`
2
`); + // Before we get to the actual DOM this suspends twice + expect(rendered).to.equal( + `
2
` + ); }); describe('dangerouslySetInnerHTML', () => {