Skip to content

Commit e5dbe50

Browse files
JoviDeCroockJoviDeCroock
authored
Merge pull request #446 from preactjs/chunked-rendering-fixes
Ensure we return a full document
2 parents b283245 + b7b288c commit e5dbe50

4 files changed

Lines changed: 120 additions & 5 deletions

File tree

‎.changeset/lovely-ways-smoke.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'preact-render-to-string': patch
3+
---
4+
5+
Fix issues regarding streaming full HTML documents

‎src/lib/chunked.js‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,41 @@ export async function renderToChunks(vnode, { context, onWrite, abortSignal }) {
2323
// Synchronously render the shell
2424
// @ts-ignore - using third internal RendererState argument
2525
const shell = renderToString(vnode, context, renderer);
26-
onWrite(shell);
2726

2827
// Wait for any suspended sub-trees if there are any
2928
const len = renderer.suspended.length;
3029
if (len > 0) {
30+
// When rendering a full HTML document, the shell ends with </body></html>.
31+
// Inserting the deferred <div hidden> wrapper after </html> is invalid HTML
32+
// and causes browsers to reject the content. Instead, we inject the deferred
33+
// content before the closing tags, then emit them last.
34+
const docSuffixIndex = getDocumentClosingTagsIndex(shell);
35+
const hasHtmlTag = shell.trimStart().startsWith('<html');
36+
const initialWrite =
37+
docSuffixIndex !== -1 ? shell.slice(0, docSuffixIndex) : shell;
38+
const prefix = hasHtmlTag ? '<!DOCTYPE html>' : '';
39+
onWrite(prefix + initialWrite);
3140
onWrite('<div hidden>');
3241
onWrite(createInitScript(len));
3342
// We should keep checking all promises
3443
await forkPromises(renderer);
3544
onWrite('</div>');
45+
if (docSuffixIndex !== -1) onWrite(shell.slice(docSuffixIndex));
46+
} else {
47+
onWrite(shell);
3648
}
3749
}
3850

51+
/**
52+
* If the shell ends with </body></html> (full document rendering), return that
53+
* suffix so it can be emitted *after* the deferred content, keeping the HTML valid.
54+
* @param {string} html
55+
* @returns {number}
56+
*/
57+
function getDocumentClosingTagsIndex(html) {
58+
return html.lastIndexOf('</body>');
59+
}
60+
3961
async function forkPromises(renderer) {
4062
if (renderer.suspended.length > 0) {
4163
const suspensions = [...renderer.suspended];

‎src/lib/client.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
// else if (n.data == '/preact-island:' + i) e = n;
2020
// if (s && e) break;
2121
// }
22-
// if (s && e) {
22+
// if (s && e && s.parentNode !== document) {
2323
// requestAnimationFrame(() => {
2424
// var p = e.previousSibling;
2525
// while (p != s) {
@@ -46,7 +46,7 @@
4646
// }
4747

4848
// To modify the INIT_SCRIPT, uncomment the above code, modify it, and paste it into https://try.terser.org/.
49-
const INIT_SCRIPT = `class e extends HTMLElement{connectedCallback(){var e=this;if(!e.isConnected)return;let t=this.getAttribute("data-target");if(t){for(var r,a,i=document.createNodeIterator(document,128);i.nextNode();){let e=i.referenceNode;if(e.data=="preact-island:"+t?r=e:e.data=="/preact-island:"+t&&(a=e),r&&a)break}r&&a&&requestAnimationFrame((()=>{for(var t=a.previousSibling;t!=r&&t&&t!=r;)a.parentNode.removeChild(t),t=a.previousSibling;for(i=r;e.firstChild;)r=e.firstChild,e.removeChild(r),i.after(r),i=r;e.parentNode.removeChild(e)}))}}}customElements.define("preact-island",e);`;
49+
const INIT_SCRIPT = `class e extends HTMLElement{connectedCallback(){var e=this;if(!e.isConnected)return;let t=this.getAttribute("data-target");if(t){for(var r,a,i=document.createNodeIterator(document,128);i.nextNode();){let e=i.referenceNode;if(e.data=="preact-island:"+t?r=e:e.data=="/preact-island:"+t&&(a=e),r&&a)break}r&&a&&r.parentNode!==document&&requestAnimationFrame((()=>{for(var t=a.previousSibling;t!=r&&t&&t!=r;)a.parentNode.removeChild(t),t=a.previousSibling;for(i=r;e.firstChild;)r=e.firstChild,e.removeChild(r),i.after(r),i=r;e.parentNode.removeChild(e)}))}}}customElements.define("preact-island",e);`;
5050

5151
export function createInitScript() {
5252
return `<script>(function(){${INIT_SCRIPT}}())</script>`;

‎test/compat/render-chunked.test.jsx‎

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,94 @@ describe('renderToChunks', () => {
191191
]);
192192
});
193193

194+
it('should inject deferred content before </body></html> for full document rendering', async () => {
195+
const { Suspender, suspended } = createSuspender();
196+
197+
const result = [];
198+
const promise = renderToChunks(
199+
<html>
200+
<head>
201+
<title>Test</title>
202+
</head>
203+
<body>
204+
<Suspense fallback="loading...">
205+
<Suspender />
206+
</Suspense>
207+
</body>
208+
</html>,
209+
{ onWrite: (s) => result.push(s) }
210+
);
211+
suspended.resolve();
212+
await promise;
213+
214+
const fullHtml = result.join('');
215+
216+
// Deferred wrapper must appear before </body></html>, not after
217+
const deferredPos = fullHtml.indexOf('<div hidden>');
218+
const bodyClosePos = fullHtml.indexOf('</body>');
219+
const htmlClosePos = fullHtml.indexOf('</html>');
220+
221+
expect(deferredPos).toBeGreaterThan(-1);
222+
expect(deferredPos).toBeLessThan(bodyClosePos);
223+
expect(bodyClosePos).toBeLessThan(htmlClosePos);
224+
225+
// The document must end with </html>
226+
expect(fullHtml.endsWith('</html>')).toBe(true);
227+
// No content after </html>
228+
expect(result[result.length - 1]).toBe('</body></html>');
229+
});
230+
231+
it('should prepend <!DOCTYPE html> when rendering a full document with suspended content', async () => {
232+
const { Suspender, suspended } = createSuspender();
233+
234+
const result = [];
235+
const promise = renderToChunks(
236+
<html>
237+
<head>
238+
<title>Test</title>
239+
</head>
240+
<body>
241+
<Suspense fallback="loading...">
242+
<Suspender />
243+
</Suspense>
244+
</body>
245+
</html>,
246+
{ onWrite: (s) => result.push(s) }
247+
);
248+
suspended.resolve();
249+
await promise;
250+
251+
// The first chunk must be prefixed with <!DOCTYPE html>
252+
expect(result[0].startsWith('<!DOCTYPE html>')).toBe(true);
253+
254+
// The full output must start with the doctype
255+
const fullHtml = result.join('');
256+
expect(fullHtml.startsWith('<!DOCTYPE html>')).toBe(true);
257+
258+
// The doctype should appear exactly once
259+
const doctypeCount = (fullHtml.match(/<!DOCTYPE html>/gi) || []).length;
260+
expect(doctypeCount).toBe(1);
261+
});
262+
263+
it('should not prepend <!DOCTYPE html> when rendering a non-document fragment with suspended content', async () => {
264+
const { Suspender, suspended } = createSuspender();
265+
266+
const result = [];
267+
const promise = renderToChunks(
268+
<div>
269+
<Suspense fallback="loading...">
270+
<Suspender />
271+
</Suspense>
272+
</div>,
273+
{ onWrite: (s) => result.push(s) }
274+
);
275+
suspended.resolve();
276+
await promise;
277+
278+
const fullHtml = result.join('');
279+
expect(fullHtml.includes('<!DOCTYPE html>')).toBe(false);
280+
});
281+
194282
it('should support a component that suspends multiple times', async () => {
195283
const { Suspender, suspended } = createSuspender();
196284
const { Suspender: Suspender2, suspended: suspended2 } = createSuspender();
@@ -217,10 +305,10 @@ describe('renderToChunks', () => {
217305
await promise;
218306

219307
expect(result).to.deep.equal([
220-
'<div><!--preact-island:49-->loading part 1...<!--/preact-island:49--></div>',
308+
'<div><!--preact-island:70-->loading part 1...<!--/preact-island:70--></div>',
221309
'<div hidden>',
222310
createInitScript(1),
223-
createSubtree('49', '<p>it works</p><p>it works</p>'),
311+
createSubtree('70', '<p>it works</p><p>it works</p>'),
224312
'</div>'
225313
]);
226314
});

0 commit comments

Comments
 (0)