From 9b324afaf28e5accc27469e02527cd8c1c7d2608 Mon Sep 17 00:00:00 2001 From: Arthur Fiorette Date: Sun, 10 Mar 2024 19:32:21 -0300 Subject: [PATCH] feat: simplified html stream generation --- .changeset/calm-games-lay.md | 6 + packages/fastify-html-plugin/index.js | 115 +++---------- packages/fastify-html-plugin/lib/constants.js | 14 -- packages/fastify-html-plugin/lib/is-html.js | 26 --- .../fastify-html-plugin/lib/is-tag-html.js | 6 +- .../lib/prepend-doctype.js | 13 -- packages/fastify-html-plugin/package.json | 2 +- .../test/auto-detect.test.tsx | 46 ++++++ .../fastify-html-plugin/test/auto-detect.tsx | 74 --------- .../fastify-html-plugin/test/index.test.tsx | 19 +-- .../fastify-html-plugin/test/is-html.test.tsx | 69 -------- .../test/prepend-doctype.test.tsx | 95 ----------- .../test/stream-html.test.tsx | 77 +++------ packages/fastify-html-plugin/tsconfig.json | 2 +- packages/fastify-html-plugin/types/index.d.ts | 107 ------------ .../types/index.test-d.tsx | 13 +- packages/html/index.js | 2 + packages/html/package.json | 2 +- packages/html/suspense.d.ts | 64 +++---- packages/html/suspense.js | 156 +++++++++--------- packages/html/test/suspense.test.tsx | 54 +++--- 21 files changed, 238 insertions(+), 724 deletions(-) create mode 100644 .changeset/calm-games-lay.md delete mode 100644 packages/fastify-html-plugin/lib/constants.js delete mode 100644 packages/fastify-html-plugin/lib/is-html.js delete mode 100644 packages/fastify-html-plugin/lib/prepend-doctype.js create mode 100644 packages/fastify-html-plugin/test/auto-detect.test.tsx delete mode 100644 packages/fastify-html-plugin/test/auto-detect.tsx delete mode 100644 packages/fastify-html-plugin/test/is-html.test.tsx delete mode 100644 packages/fastify-html-plugin/test/prepend-doctype.test.tsx diff --git a/.changeset/calm-games-lay.md b/.changeset/calm-games-lay.md new file mode 100644 index 00000000..6f10432c --- /dev/null +++ b/.changeset/calm-games-lay.md @@ -0,0 +1,6 @@ +--- +'@kitajs/fastify-html-plugin': patch +'@kitajs/html': patch +--- + +Simplified html stream generation diff --git a/packages/fastify-html-plugin/index.js b/packages/fastify-html-plugin/index.js index 2cf97f04..6e2b0d09 100644 --- a/packages/fastify-html-plugin/index.js +++ b/packages/fastify-html-plugin/index.js @@ -1,13 +1,10 @@ /// const fp = require('fastify-plugin'); -const { pipeHtml } = require('@kitajs/html/suspense'); -const { prependDoctype } = require('./lib/prepend-doctype'); -const { isHtml } = require('./lib/is-html'); -const { CONTENT_TYPE_VALUE } = require('./lib/constants'); -const { Readable } = require('stream'); +const { isTagHtml } = require('./lib/is-tag-html'); -function noop() {} +// loads SUSPENSE_ROOT +require('@kitajs/html/suspense'); /** * @type {import('fastify').FastifyPluginCallback< @@ -15,118 +12,46 @@ function noop() {} * >} */ function fastifyKitaHtml(fastify, opts, next) { - // Enables suspense if it's not enabled yet - SUSPENSE_ROOT.enabled ||= true; - // Good defaults - opts.autoDetect ??= true; opts.autoDoctype ??= true; - opts.contentType ??= CONTENT_TYPE_VALUE; - opts.isHtml ??= isHtml; - // The normal .html handler is much simpler than the streamHtml one fastify.decorateReply('html', html); - fastify.decorateReply('setupHtmlStream', setupHtmlStream); - fastify.decorateRequest('replyHtmlStream', null); - - // As JSX is evaluated from the inside out, renderToStream() method requires - // a function to be able to execute some code before the JSX calls gets to - // render, it can be avoided by simply executing the code in the - // streamHtml getter method. - fastify.decorateReply('streamHtml', { - getter() { - this.setupHtmlStream(); - return streamHtml; - } - }); - - if (opts.autoDetect) { - // The onSend hook is only used by autoDetect, so we can - // skip adding it if it's not enabled. - fastify.addHook('onSend', onSend); - } return next(); - /** @type {import('fastify').FastifyReply['setupHtmlStream']} */ - function setupHtmlStream() { - SUSPENSE_ROOT.requests.set(this.request.id, { - // Creating a Readable is better than hijacking the reply stream - // https://lirantal.com/blog/avoid-fastify-reply-raw-and-reply-hijack-despite-being-a-powerful-http-streams-tool - stream: new Readable({ read: noop }), - running: 0, - sent: false - }); - - return this; - } - /** @type {import('fastify').FastifyReply['html']} */ function html(htmlStr) { - if (htmlStr instanceof Promise) { - // Handles possibility of html being a promise + if (typeof htmlStr !== 'string') { + // Recursive promise handling, rejections should just rethrow + // just like as if it was a sync component error. return htmlStr.then(html.bind(this)); } - this.header('content-length', Buffer.byteLength(htmlStr)); - this.header('content-type', opts.contentType); - - if (opts.autoDoctype) { - htmlStr = prependDoctype(htmlStr); + // prepends doctype if the html is a full html document + if (opts.autoDoctype && isTagHtml(htmlStr)) { + htmlStr = '' + htmlStr; } - return this.send(htmlStr); - } + this.header('content-type', 'text/html; charset=utf-8'); - /** @type {import('fastify').FastifyReply['streamHtml']} */ - function streamHtml(htmlStr) { - // Content-length is optional as long as the connection is closed after the response is done - // https://www.rfc-editor.org/rfc/rfc7230#section-3.3.3 - this.header('content-type', opts.contentType); - - if (opts.autoDoctype) { - if (htmlStr instanceof Promise) { - // Handles possibility of html being a promise - htmlStr = htmlStr.then(prependDoctype); - } else { - htmlStr = prependDoctype(htmlStr); - } - } - - // When the .streamHtml is called, the fastify decorator's getter method - // already created the request data at the SUSPENSE_ROOT for us, so we - // can simply pipe the first html wave to the reply stream. + // If no suspense component was used, this will not be defined. const requestData = SUSPENSE_ROOT.requests.get(this.request.id); - /* c8 ignore next 5 */ if (!requestData) { - throw new Error( - 'The request data was not found, this is a bug in the fastify-html-plugin' + return ( + this + // Nothing needs to be sent later, content length is known + .header('content-length', Buffer.byteLength(htmlStr)) + .send(htmlStr) ); } - // Pipes the HTML to the stream, this promise never - // throws, so we don't need to handle it. - pipeHtml(htmlStr, requestData.stream, this.request.id); - - return this.send(requestData.stream); - } + requestData.stream.push(htmlStr); - /** @type {import('fastify').onSendHookHandler} */ - function onSend(_request, reply, payload, done) { - if (opts.isHtml(payload)) { - // Streamed html should not enter here, because it's not a string, - // and already was handled by the streamHtml method. - reply.header('content-type', opts.contentType); - - if (opts.autoDoctype) { - // Payload will never be a promise here, because the content was already - // serialized. - payload = prependDoctype(payload); - } - } + // Content-length is optional as long as the connection is closed after the response is done + // https://www.rfc-editor.org/rfc/rfc7230#section-3.3.3 - return done(null, payload); + return this.send(requestData.stream); } } diff --git a/packages/fastify-html-plugin/lib/constants.js b/packages/fastify-html-plugin/lib/constants.js deleted file mode 100644 index 9673ecdc..00000000 --- a/packages/fastify-html-plugin/lib/constants.js +++ /dev/null @@ -1,14 +0,0 @@ -const HTML_TAG = ' - length >= 7 && - // open tag - value[0] === '<' && - // close tag - value[length - 1] === '>' - ); -}; diff --git a/packages/fastify-html-plugin/lib/is-tag-html.js b/packages/fastify-html-plugin/lib/is-tag-html.js index b66dda0e..5d7df7c5 100644 --- a/packages/fastify-html-plugin/lib/is-tag-html.js +++ b/packages/fastify-html-plugin/lib/is-tag-html.js @@ -1,5 +1,3 @@ -const { HTML_TAG_LENGTH, HTML_TAG } = require('./constants'); - /** * Returns true if the string starts with ` { + await using app = fastify(); + app.register(fastifyKitaHtml); + + app.get('/default', () =>
Not a html root element
); + app.get('/default/root', () => ); + app.get('/html', (_, res) => res.html(
Not a html root element
)); + app.get('/html/root', (_, res) => res.html()); + + await t.test('Default', async () => { + const res = await app.inject({ method: 'GET', url: '/default' }); + + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/plain; charset=utf-8'); + assert.strictEqual(res.body, '
Not a html root element
'); + }); + + await t.test('Default root', async () => { + const res = await app.inject({ method: 'GET', url: '/default/root' }); + + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/plain; charset=utf-8'); + assert.strictEqual(res.body, ''); + }); + + await t.test('Html ', async () => { + const res = await app.inject({ method: 'GET', url: '/html' }); + + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); + assert.strictEqual(res.body, '
Not a html root element
'); + }); + + await t.test('Html root', async () => { + const res = await app.inject({ method: 'GET', url: '/html/root' }); + + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); + assert.strictEqual(res.body, ''); + }); +}); diff --git a/packages/fastify-html-plugin/test/auto-detect.tsx b/packages/fastify-html-plugin/test/auto-detect.tsx deleted file mode 100644 index 5683168e..00000000 --- a/packages/fastify-html-plugin/test/auto-detect.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import fastify from 'fastify'; -import assert from 'node:assert'; -import test, { describe } from 'node:test'; -import { fastifyKitaHtml } from '..'; - -describe('opts.autoDetect', () => { - test('Auto detects HTML by default', async (t) => { - await using app = fastify(); - app.register(fastifyKitaHtml); - - app.get('/html', () => ); - app.get('/div', () =>
Not a html root element
); - - await t.test('Html root', async () => { - const res = await app.inject({ method: 'GET', url: '/html' }); - - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); - assert.strictEqual(res.body, ''); - }); - - await t.test('Non html root', async () => { - const res = await app.inject({ method: 'GET', url: '/div' }); - - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); - assert.strictEqual(res.body, '
Not a html root element
'); - }); - }); - - test('Does not autodetect when disabled', async (t) => { - await using app = fastify(); - app.register(fastifyKitaHtml, { - autoDetect: false - }); - - app.get('/default', () =>
Not a html root element
); - app.get('/default/root', () => ); - app.get('/html', (_, res) => res.html(
Not a html root element
)); - app.get('/html/root', (_, res) => res.html()); - - await t.test('Default', async () => { - const res = await app.inject({ method: 'GET', url: '/default' }); - - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], 'text/plain; charset=utf-8'); - assert.strictEqual(res.body, '
Not a html root element
'); - }); - - await t.test('Default root', async () => { - const res = await app.inject({ method: 'GET', url: '/default/root' }); - - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], 'text/plain; charset=utf-8'); - assert.strictEqual(res.body, ''); - }); - - await t.test('Html ', async () => { - const res = await app.inject({ method: 'GET', url: '/html' }); - - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); - assert.strictEqual(res.body, '
Not a html root element
'); - }); - - await t.test('Html root', async () => { - const res = await app.inject({ method: 'GET', url: '/html/root' }); - - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); - assert.strictEqual(res.body, ''); - }); - }); -}); diff --git a/packages/fastify-html-plugin/test/index.test.tsx b/packages/fastify-html-plugin/test/index.test.tsx index f1130f84..19d44e7f 100644 --- a/packages/fastify-html-plugin/test/index.test.tsx +++ b/packages/fastify-html-plugin/test/index.test.tsx @@ -3,7 +3,6 @@ import assert from 'node:assert'; import test, { describe } from 'node:test'; import { setImmediate } from 'node:timers/promises'; import { fastifyKitaHtml } from '..'; -import { CONTENT_TYPE_VALUE } from '../lib/constants'; describe('reply.html()', () => { test('renders html', async () => { @@ -14,9 +13,9 @@ describe('reply.html()', () => { const res = await app.inject({ method: 'GET', url: '/' }); - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); assert.strictEqual(res.body, '
Hello from JSX!
'); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); + assert.strictEqual(res.statusCode, 200); }); test('renders async html', async () => { @@ -29,9 +28,9 @@ describe('reply.html()', () => { const res = await app.inject({ method: 'GET', url: '/' }); - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); assert.strictEqual(res.body, '
Hello from async JSX!
'); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); + assert.strictEqual(res.statusCode, 200); }); test('fails when html is not a string', async () => { @@ -45,14 +44,12 @@ describe('reply.html()', () => { const res = await app.inject({ method: 'GET', url: '/' }); - assert.strictEqual(res.statusCode, 500); assert.strictEqual(res.headers['content-type'], 'application/json; charset=utf-8'); + assert.strictEqual(res.statusCode, 500); assert.deepStrictEqual(res.json(), { statusCode: 500, - code: 'ERR_INVALID_ARG_TYPE', error: 'Internal Server Error', - message: - 'The "string" argument must be of type string or an instance of Buffer or ArrayBuffer. Received type number (12345)' + message: 'htmlStr.then is not a function' }); }); @@ -71,10 +68,8 @@ describe('reply.html()', () => { assert.strictEqual(res.headers['content-type'], 'application/json; charset=utf-8'); assert.deepStrictEqual(res.json(), { statusCode: 500, - code: 'ERR_INVALID_ARG_TYPE', error: 'Internal Server Error', - message: - 'The "string" argument must be of type string or an instance of Buffer or ArrayBuffer. Received type number (12345)' + message: 'htmlStr.then is not a function' }); }); }); diff --git a/packages/fastify-html-plugin/test/is-html.test.tsx b/packages/fastify-html-plugin/test/is-html.test.tsx deleted file mode 100644 index 06c511ee..00000000 --- a/packages/fastify-html-plugin/test/is-html.test.tsx +++ /dev/null @@ -1,69 +0,0 @@ -import assert from 'node:assert'; -import test, { describe } from 'node:test'; -import { isHtml } from '../lib/is-html'; - -describe('isHtml', () => { - test('detects html', () => { - assert.ok(isHtml('

Hi

')); - assert.ok(isHtml('')); - assert.ok(isHtml('')); - assert.ok(isHtml('')); - assert.ok(isHtml('')); - assert.ok(isHtml('

Hi

')); - assert.ok(isHtml('

Hi

')); - assert.ok(isHtml('

Hi

')); - assert.ok(isHtml('

Hi

')); - }); - - test('trims html', () => { - assert.ok(isHtml('

Hi

')); - assert.ok(isHtml(' \n \n\n

Hi

\n\n \n \n ')); - }); - - test('detects html with JSX', () => { - assert.ok(isHtml(

Hi

)); - // biome-ignore lint/a11y/useHtmlLang: this is a test - assert.ok(isHtml()); - assert.ok(isHtml()); - assert.ok( - isHtml( - <> - -

Hi

- - ) - ); - assert.ok( - isHtml( - -

Hi

- - ) - ); - }); - - test('does not detects html on non strings', () => { - // biome-ignore lint/suspicious/noExplicitAny: to ignore type errors - const anyIsHtml: any = isHtml; - - assert.ok(!anyIsHtml()); - assert.ok(!anyIsHtml(undefined)); - assert.ok(!anyIsHtml(null)); - assert.ok(!anyIsHtml(0)); - assert.ok(!anyIsHtml(1)); - assert.ok(!anyIsHtml({ a: 1 })); - assert.ok(!anyIsHtml({ html: '
' })); - assert.ok(!anyIsHtml([])); - assert.ok(!anyIsHtml(true)); - assert.ok(!anyIsHtml(false)); - }); - - test('does not detects html on non html strings', () => { - assert.ok(!isHtml('')); - assert.ok(!isHtml('Hi')); - assert.ok(!isHtml('Hi

Hi

')); - assert.ok(!isHtml('

Hi

Hi')); - assert.ok(!isHtml('

invalid invalid

')); - }); -}); diff --git a/packages/fastify-html-plugin/test/prepend-doctype.test.tsx b/packages/fastify-html-plugin/test/prepend-doctype.test.tsx deleted file mode 100644 index 85c47c32..00000000 --- a/packages/fastify-html-plugin/test/prepend-doctype.test.tsx +++ /dev/null @@ -1,95 +0,0 @@ -import assert from 'node:assert'; -import test, { describe } from 'node:test'; -import { prependDoctype } from '../lib/prepend-doctype'; - -describe('prependDoctype', () => { - test('prepends doctype to html', () => { - assert.strictEqual( - prependDoctype(''), - '' - ); - - assert.strictEqual(prependDoctype(''), ''); - - // keeps single doctype - assert.strictEqual( - prependDoctype(''), - '' - ); - }); - - test('does not prepends doctype to non html tags', () => { - assert.strictEqual(prependDoctype('
'), '
'); - - assert.strictEqual(prependDoctype('
Hi
'), '
Hi
'); - - assert.strictEqual( - prependDoctype('
'), - '
' - ); - - assert.strictEqual( - prependDoctype('
Hi
'), - '
Hi
' - ); - }); - - test('does not prepends doctype to non html strings', () => { - assert.strictEqual(prependDoctype('Hi'), 'Hi'); - - assert.strictEqual(prependDoctype(''), ''); - }); - - test('prepends doctype to html with JSX', () => { - assert.strictEqual( - //@ts-expect-error - should fail - prependDoctype(), - '' - ); - - assert.strictEqual( - //@ts-expect-error - should fail - // biome-ignore lint/a11y/useHtmlLang: this is a test - prependDoctype(), - '' - ); - - // keeps single doctype - assert.strictEqual( - prependDoctype( - //@ts-expect-error - should fail - <> - {''} - {/* biome-ignore lint/a11y/useHtmlLang: this is a test */} - - - ), - '' - ); - }); - - test('does not prepends doctype to non html JSX', () => { - assert.strictEqual( - //@ts-expect-error - should fail - prependDoctype(
), - '
' - ); - - assert.strictEqual( - //@ts-expect-error - should fail - prependDoctype(
Hi
), - '
Hi
' - ); - - assert.strictEqual( - prependDoctype( - //@ts-expect-error - should fail - <> -
-
Hi
- - ), - '
Hi
' - ); - }); -}); diff --git a/packages/fastify-html-plugin/test/stream-html.test.tsx b/packages/fastify-html-plugin/test/stream-html.test.tsx index 71c2bc68..d532eafb 100644 --- a/packages/fastify-html-plugin/test/stream-html.test.tsx +++ b/packages/fastify-html-plugin/test/stream-html.test.tsx @@ -12,7 +12,6 @@ import assert from 'node:assert'; import { afterEach, describe, test } from 'node:test'; import { setTimeout } from 'node:timers/promises'; import { fastifyKitaHtml } from '..'; -import { CONTENT_TYPE_VALUE } from '../lib/constants'; async function SleepForMs({ ms, children }: PropsWithChildren<{ ms: number }>) { await setTimeout(ms * 2); @@ -25,7 +24,6 @@ describe('Suspense', () => { assert.equal(SUSPENSE_ROOT.requests.size, 0, 'Suspense root left pending resources'); // Reset suspense root - SUSPENSE_ROOT.enabled = false; SUSPENSE_ROOT.autoScript = true; SUSPENSE_ROOT.requestCounter = 1; SUSPENSE_ROOT.requests.clear(); @@ -35,13 +33,13 @@ describe('Suspense', () => { await using app = fastify(); app.register(fastifyKitaHtml); - app.get('/', (_, res) => res.streamHtml(
)); + app.get('/', (_, res) => res.html(
)); const res = await app.inject({ method: 'GET', url: '/' }); - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); assert.strictEqual(res.body, '
'); + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); }); test('Suspense sync children', async () => { @@ -49,7 +47,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1
}>
2
@@ -58,9 +56,9 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); assert.strictEqual(res.body, '
2
'); + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); }); test('Suspense async children', async () => { @@ -68,7 +66,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1
}> @@ -78,7 +76,7 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( res.body, <> @@ -103,7 +101,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1
}> @@ -113,7 +111,7 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( res.body, <> @@ -138,7 +136,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1
)}>
2
@@ -147,9 +145,9 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); - assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); assert.strictEqual(res.body, '
2
'); + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); }); test('Multiple async renders cleanup', async () => { @@ -157,7 +155,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1)}> @@ -170,7 +168,7 @@ describe('Suspense', () => { promises.push( app.inject({ method: 'GET', url: '/' }).then((res) => { assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( res.body, <> @@ -200,7 +198,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1)}> @@ -210,7 +208,7 @@ describe('Suspense', () => { for (let i = 0; i < 10; i++) { const res = await app.inject({ method: 'GET', url: '/' }); assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( res.body, <> @@ -236,7 +234,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html(
1
}> @@ -256,7 +254,7 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( res.body, @@ -307,7 +305,7 @@ describe('Suspense', () => { const seconds = (req.query as { seconds: number }).seconds; res.header('seconds', seconds); - return res.streamHtml( + return res.html(
{Array.from({ length: seconds }, (_, i) => ( {seconds - i} loading
}> @@ -334,7 +332,7 @@ describe('Suspense', () => { const seconds = +result.headers.seconds!; assert.strictEqual(result.statusCode, 200); - assert.strictEqual(result.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(result.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( result.body, <> @@ -363,35 +361,12 @@ describe('Suspense', () => { } }); - test('throws if used outside of streamHtml', async () => { - await using app = fastify(); - app.register(fastifyKitaHtml); - - app.get('/', (_, res) => - res.html( - - {Promise.resolve(2)} - - ) - ); - - const res = await app.inject({ method: 'GET', url: '/' }); - - assert.strictEqual(res.statusCode, 500); - assert.strictEqual(res.headers['content-type'], 'application/json; charset=utf-8'); - assert.deepStrictEqual(res.json(), { - statusCode: 500, - error: 'Internal Server Error', - message: 'Request data was deleted before all suspense components were resolved.' - }); - }); - test('works with parallel deep suspense calls resolving first', async (t) => { await using app = fastify(); app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html(
{Array.from({ length: 5 }, (_, i) => ( {i} fb outer
}> @@ -413,7 +388,7 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); await t.test('Html stream', () => { assert.strictEqual( @@ -555,7 +530,7 @@ describe('Suspense', () => { app.register(fastifyKitaHtml); app.get('/', (req, res) => - res.streamHtml( + res.html( 1}> {Promise.reject(new Error('component failed'))} @@ -578,7 +553,7 @@ describe('Suspense', () => { const err = new Error('component failed'); app.get('/', (req, res) => - res.streamHtml( + res.html( 1} @@ -595,7 +570,7 @@ describe('Suspense', () => { const res = await app.inject({ method: 'GET', url: '/' }); assert.strictEqual(res.statusCode, 200); - assert.strictEqual(res.headers['content-type'], CONTENT_TYPE_VALUE); + assert.strictEqual(res.headers['content-type'], 'text/html; charset=utf-8'); assert.strictEqual( res.body, <> diff --git a/packages/fastify-html-plugin/tsconfig.json b/packages/fastify-html-plugin/tsconfig.json index 8a11d961..b09c3b0e 100644 --- a/packages/fastify-html-plugin/tsconfig.json +++ b/packages/fastify-html-plugin/tsconfig.json @@ -9,5 +9,5 @@ "outDir": "dist", "plugins": [{ "name": "@kitajs/ts-html-plugin" }] }, - "include": ["lib", "test", "index.js"] + "include": ["lib", "test", "types", "index.js"] } diff --git a/packages/fastify-html-plugin/types/index.d.ts b/packages/fastify-html-plugin/types/index.d.ts index 466d1499..5b7312c1 100644 --- a/packages/fastify-html-plugin/types/index.d.ts +++ b/packages/fastify-html-plugin/types/index.d.ts @@ -32,84 +32,6 @@ declare module 'fastify' { * @returns The response. */ html(this: this, html: JSX.Element): this | Promise; - - /** - * Sends the html to the browser as a single stream, the entire component tree will be - * waited synchronously. When using any `Suspense`, its fallback will be synchronously - * waited and sent to the browser in the original stream, as its children are - * resolved, new pieces of html will be sent to the browser. When all `Suspense`s - * pending promises are resolved, the connection is closed normally. - * - * ### `request.id` must be used as the `Suspense`'s `rid` parameter - * - * This method hijacks the response, as the html stream is just a single continuous - * stream in the http body, any changes to the status code or headers after calling - * this method **will not have effect**. - * - * If the HTML does not start with a doctype and `opts.autoDoctype` is enabled, it - * will be added automatically. The correct `Content-Type` header will also be - * defined. - * - * **Http trailers are not yet supported when using `streamHtml`** - * - * @example - * - * ```tsx - * app.get('/', (req, reply) => - * reply.streamHtml( - * Loading...}> - * - * - * ) - * ); - * ``` - * - * @param html The HTML to send. - * @returns The response. - */ - streamHtml(this: this, html: JSX.Element): this; - - /** - * This function is called internally by the `streamHtml` getter. - * - * ### Executing code before sending the response and after creating your - * - * Html is a bad pattern and should be avoided! - * - * This function must be called **manually** at the top of the route handler when you - * have to execute some code **after** your root layout and **before** the `streamHtml - * call. - * - * If `setupHtmlStream` is executed and no call to `streamHtml` happens before the - * request finishes, a memory leak will be created. Make sure that `setupHtmlStream` - * will never be executed without being followed by `streamHtml`. - * - * @example - * - * ```tsx - * app.get('/bad', (_, reply) => { - * const html = // Error: Request data was deleted before all - * // suspense components were resolved. - * - * // code that must be executed after the template - * foo(); - * - * return reply.streamHtml(html); - * }) - * - * app.get('/good', (_, reply) => { - * reply.setupHtmlStream(); - * - * const html = // works! - * - * // code that must be executed after the template - * foo(); - * - * return reply.streamHtml(html); - * }) - * ``` - */ - setupHtmlStream(this: this): this; } } @@ -120,21 +42,6 @@ type FastifyKitaHtmlPlugin = FastifyPluginCallback< declare namespace fastifyKitaHtml { /** Options for @kitajs/fastify-html-plugin plugin. */ export interface FastifyKitaHtmlOptions { - /** - * The value of the `Content-Type` header. - * - * @default 'text/html; charset=utf8' - */ - contentType: string; - - /** - * Whether to automatically detect HTML content and set the content-type when - * `.html()` is not used. - * - * @default true - */ - autoDetect: boolean; - /** * Whether to automatically add `` to a response starting with , * if not found. @@ -150,20 +57,6 @@ declare namespace fastifyKitaHtml { * @default true */ autoDoctype: boolean; - - /** - * The function used to detect if a string is a html or not when `autoDetect` is - * enabled. - * - * Default implementation if length is greater than 3, starts with `<` and ends with - * `>`. - * - * There's no real way to validate HTML, so this is a best guess. - * - * @see https://stackoverflow.com/q/1732348 - * @see https://stackoverflow.com/q/11229831 - */ - isHtml: (this: void, value: string) => boolean; } export const fastifyKitaHtml: FastifyKitaHtmlPlugin; diff --git a/packages/fastify-html-plugin/types/index.test-d.tsx b/packages/fastify-html-plugin/types/index.test-d.tsx index 9a021070..7cccd3e7 100644 --- a/packages/fastify-html-plugin/types/index.test-d.tsx +++ b/packages/fastify-html-plugin/types/index.test-d.tsx @@ -7,12 +7,7 @@ const app = fastify(); app.register(fastifyKitaHtml); app.register(fastifyKitaHtml, { - autoDetect: true, - autoDoctype: true, - contentType: 'text/html; charset=utf-8', - isHtml(value) { - return value.length > 0; - } + autoDoctype: true }); app.get('/', async (_, reply) => { @@ -24,15 +19,15 @@ app.get('/jsx', async (_, reply) => { }); app.get('/stream', async (_, reply) => { - reply.streamHtml('
hello world
'); + reply.html('
hello world
'); }); app.get('/stream/jsx', async (_, reply) => { - reply.streamHtml(
hello world
); + reply.html(
hello world
); }); app.get('/stream/suspense', async (request, reply) => { - reply.streamHtml( + reply.html( fallback}> {Promise.resolve(1)} diff --git a/packages/html/index.js b/packages/html/index.js index bdf60154..0ad69d16 100644 --- a/packages/html/index.js +++ b/packages/html/index.js @@ -1,4 +1,6 @@ /// +/// +/// const ESCAPED_REGEX = /[<"'&]/; const CAMEL_REGEX = /[a-z][A-Z]/; diff --git a/packages/html/package.json b/packages/html/package.json index b2b4f551..03e4abe6 100644 --- a/packages/html/package.json +++ b/packages/html/package.json @@ -1,6 +1,6 @@ { "name": "@kitajs/html", - "version": "3.1.2", + "version": "4.1.2", "description": "Fast and type safe HTML templates using TypeScript.", "homepage": "https://github.com/kitajs/html/tree/master/packages/html#readme", "bugs": "https://github.com/kitajs/html/issues", diff --git a/packages/html/suspense.d.ts b/packages/html/suspense.d.ts index 474c5534..37ca502f 100644 --- a/packages/html/suspense.d.ts +++ b/packages/html/suspense.d.ts @@ -22,13 +22,6 @@ declare global { */ requestCounter: number; - /** - * If the usage of suspense is enabled. - * - * @default false - */ - enabled: boolean; - /** * If we should automatically stream {@linkcode SuspenseScript} before the first * suspense is rendered. If you disable this setting, you need to manually load the @@ -62,6 +55,10 @@ export type RequestData = { * * The `rid` prop is the one {@linkcode renderToStream} returns, this way the suspense * knows which request it belongs to. + * + * **Warning**: Using `Suspense` without any type of runtime support will _**LEAK + * memory**_. Always use `renderToStream`, `renderToString` or within a specific package + * like `@kitajs/fastify-html-plugin` */ export function Suspense(props: SuspenseProps): JSX.Element; @@ -72,29 +69,22 @@ export function Suspense(props: SuspenseProps): JSX.Element; * @example * * ```tsx - * // Simple nodejs webserver to render html http.createServer((req, res) - * => { res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }) - * - * const stream = renderToStream(r => ) - * - * stream.pipe(res) - * - * }).listen(1227) - * - * // Prints out the rendered stream const stream = renderToStream(r => ) - * - * For await (const html of stream) { console.log(html.toString()) } + * // Prints out the rendered stream (2nd example shows with a custom id) + * const stream = renderToStream(r => ) + * const stream = renderToStream(, myCustomId) * + * for await (const html of stream) { + * console.log(html.toString()) + * } * ``` * - * @param factory The component tree to render. + * @param html The component tree to render or a function that returns the component tree. * @param rid The request id to identify the request, if not provided, a new incrementing * id will be used. * @see {@linkcode Suspense} */ export function renderToStream( - factory: (this: void, rid: number | string) => JSX.Element, + html: JSX.Element | ((rid: number | string) => JSX.Element), rid?: number | string ): Readable; @@ -112,18 +102,21 @@ export function renderToStream( * * ```tsx * // Does not uses suspense benefits! Useful for testing. Prefer to - * // use renderToStream instead. - * const html = await renderToString(r => ); + * // use renderToStream instead. (2nd example shows with a custom id) + * const html = await renderToString(r => ) + * const html = await renderToString(, myCustomId) + * * console.log(html); * ``` * - * @param factory The component tree to render + * @param html The component tree to render or a function that returns the component tree. * @param rid The request id to identify the request, if not provided, a new incrementing - * id will be used. @see {@linkcode renderToStream} + * id will be used. * @returns A promise that resolves to the entire HTML generated by the component tree. + * @see {@linkcode renderToStream} */ export function renderToString( - factory: (this: void, rid: number | string) => JSX.Element, + html: JSX.Element | ((rid: number | string) => JSX.Element), rid?: number | string ): Promise; @@ -161,20 +154,3 @@ export interface SuspenseProps { */ catch?: JSX.Element | ((error: unknown) => JSX.Element); } - -/** - * Internal function used to pipe the HTML to the stream. Users should not use this - * function directly. This function assumes that the stream is available and the request - * id is valid. - * - * This is helpful when integrating @kitajs/html suspense support into your own runtime. - * - * @returns A promise that resolves when the first HTML (fallbacks and static content) is - * resolved or void if the JSX.Element is already resolved. - * @internal - */ -export function pipeHtml( - html: JSX.Element, - stream: Readable, - rid: number | string -): Promise | void; diff --git a/packages/html/suspense.js b/packages/html/suspense.js index ffa4459a..89ad0d2e 100644 --- a/packages/html/suspense.js +++ b/packages/html/suspense.js @@ -1,4 +1,4 @@ -const { contentsToString } = require('./index'); +const { contentsToString, contentToString } = require('./index'); const { Readable } = require('stream'); // Avoids double initialization in case this file is not cached by @@ -8,11 +8,12 @@ if (!globalThis.SUSPENSE_ROOT) { globalThis.SUSPENSE_ROOT = { requests: new Map(), requestCounter: 1, - enabled: false, autoScript: true }; } +function noop() {} + /** * Simple IE11 compatible replace child scripts to replace the template streamed by the * server. @@ -79,23 +80,13 @@ const SuspenseScript = /* html */ ` /** @type {import('./suspense').Suspense} */ function Suspense(props) { - // There's no actual way of knowing if this component is being - // rendered inside a renderToString call, so we have to rely on - // this simple check: If the First Suspense render is called without - // `enabled` being true, it means no renderToString was called before. - // This is not 100% accurate, but it's the best estimation we can do. - if (!SUSPENSE_ROOT.enabled) { - throw new Error('Cannot use Suspense outside of a `renderToStream` call.'); - } - - // fallback may be async. - const fallback = contentsToString([props.fallback]); - - if (!props.children) { - return ''; - } + // Always will be a single children because multiple + // root tags aren't a valid JSX syntax + const fallback = contentToString(props.fallback); - const children = contentsToString([props.children]); + const children = Array.isArray(props.children) + ? contentsToString(props.children) + : contentToString(props.children); // Returns content if it's not a promise if (typeof children === 'string') { @@ -106,12 +97,19 @@ function Suspense(props) { throw new Error('Suspense requires a `rid` to be specified.'); } - const data = SUSPENSE_ROOT.requests.get(props.rid); + let data = SUSPENSE_ROOT.requests.get(props.rid); if (!data) { - throw new Error( - 'Request data was deleted before all suspense components were resolved.' - ); + // Creating the request data lazily allows + // faster render() calls when no suspense + // components are used. + data = { + stream: new Readable({ read: noop, emitClose: true }), + running: 0, + sent: false + }; + + SUSPENSE_ROOT.requests.set(props.rid, data); } // Gets the current run number for this request @@ -130,7 +128,7 @@ function Suspense(props) { let html; - // unwraps error handler + // Unwraps error handler if (typeof props.catch === 'function') { html = props.catch(error); } else { @@ -189,7 +187,7 @@ function Suspense(props) { !SUSPENSE_ROOT.requests.has(props.rid) || // just to typecheck !data || - // Stream was probably already closed/cleared out. + // Stream was already closed/cleared out. data.stream.closed ) { return; @@ -211,48 +209,8 @@ function Suspense(props) { } } -// the real reason this is a extra function is because fastify-html-plugin and -// future plugins may want to reuse streams and this function avoids forcing -// them to reimplement the suspense logic. -/** @type {import('./suspense').pipeHtml} */ -async function pipeHtml(html, stream, rid) { - try { - // When the HTML is a string, it means there were no other async components - // or async Suspense's fallbacks. We can just pipe the HTML to the stream - // and try to close it. - if (typeof html === 'string') { - stream.push(html); - return; - } - - stream.push(await html); - } catch (error) { - // Emits the error down the stream or - // rethrows if there's no listener - // (default node impl always has a listener) - - /* c8 ignore next 4 */ - if (stream.emit('error', error) === false) { - throw error; - } - } finally { - const request = SUSPENSE_ROOT.requests.get(rid); - - // This request data already resolved or no suspenses were used. - if (!request || request.running === 0) { - stream.push(null); - SUSPENSE_ROOT.requests.delete(rid); - } - } -} - /** @type {import('./suspense').renderToStream} */ -function renderToStream(factory, rid) { - // Enables suspense if it's not enabled yet - if (SUSPENSE_ROOT.enabled === false) { - SUSPENSE_ROOT.enabled = true; - } - +function renderToStream(html, rid) { if (!rid) { rid = SUSPENSE_ROOT.requestCounter++; } else if (SUSPENSE_ROOT.requests.has(rid)) { @@ -260,27 +218,62 @@ function renderToStream(factory, rid) { throw new Error(`The provided Request Id is already in use: ${rid}.`); } - const stream = new Readable({ read() {} }); - SUSPENSE_ROOT.requests.set(rid, { stream, running: 0, sent: false }); + if (typeof html === 'function') { + try { + html = html(rid); + } catch (error) { + // Avoids memory leaks by removing the request data + SUSPENSE_ROOT.requests.delete(rid); + throw error; + } + } - try { - // Loads the template - const html = factory(rid); + // If no suspense component was used, this will not be defined. + const requestData = SUSPENSE_ROOT.requests.get(rid); + + // No suspense was used, just return the HTML as a stream + if (!requestData) { + if (typeof html === 'string') { + return Readable.from([html]); + } - // Pipes the HTML to the stream - pipeHtml(html, stream, rid); + const readable = new Readable({ read: noop }); + + html.then( + (result) => { + readable.push(result); + readable.push(null); // self closes + }, + (error) => { + // stream.emit returns true if there's a listener + // Nothing else to do if no catch or listener was found + /* c8 ignore next 3 */ + if (readable.emit('error', error) === false) { + console.error(error); + } + } + ); - return stream; - } catch (renderError) { - // Could not generate even the loading template. - // This means a sync error was thrown and there's - // nothing we can do unless closing the stream - // and re-throwing the error. - stream.push(null); - SUSPENSE_ROOT.requests.delete(rid); + return readable; + } - throw renderError; + if (typeof html === 'string') { + requestData.stream.push(html); + } else { + html.then( + (html) => requestData.stream.push(html), + (error) => { + /* c8 ignore next 6 */ + // stream.emit returns true if there's a listener + // Nothing else to do if no catch or listener was found + if (requestData.stream.emit('error', error) === false) { + console.error(error); + } + } + ); } + + return requestData.stream; } /** @type {import('./suspense').renderToString} */ @@ -295,7 +288,6 @@ async function renderToString(factory, rid) { } module.exports.Suspense = Suspense; -module.exports.pipeHtml = pipeHtml; module.exports.renderToStream = renderToStream; module.exports.renderToString = renderToString; module.exports.SuspenseScript = SuspenseScript; diff --git a/packages/html/test/suspense.test.tsx b/packages/html/test/suspense.test.tsx index 350062ac..8f4185f8 100644 --- a/packages/html/test/suspense.test.tsx +++ b/packages/html/test/suspense.test.tsx @@ -19,7 +19,6 @@ afterEach(() => { assert.equal(SUSPENSE_ROOT.requests.size, 0, 'Suspense root left pending requests'); // Reset suspense root - SUSPENSE_ROOT.enabled = false; SUSPENSE_ROOT.autoScript = true; SUSPENSE_ROOT.requestCounter = 1; SUSPENSE_ROOT.requests.clear(); @@ -569,25 +568,12 @@ describe('Suspense', () => { assert.equal( await renderToString((r) => ( //@ts-expect-error - testing invalid children - 1} /> + 1}> )), '' ); }); - it('throws if no stream is used', () => { - SUSPENSE_ROOT.enabled = true; - - assert.throws( - () => ( - - {Promise.resolve(2)} - - ), - /Request data was deleted before all suspense components were resolved./ - ); - }); - it('works with async error handlers', async () => { assert.equal( await renderToString((r) => ( @@ -600,7 +586,9 @@ describe('Suspense', () => {
1
+ {SuspenseScript} + @@ -884,17 +872,31 @@ describe('Suspense', () => { }); describe('Suspense errors', () => { - it('Throws when called outside of renderToStream', () => { - assert.throws( - () => ( - <> - loading}> -
1
- - - ), - /Cannot use Suspense outside of a `renderToStream` call./ - ); + it('Leaks if rendered outside of renderToStream', () => { + try { + const outside = ( + + {Promise.resolve(2)} + + ); + + assert.equal( + outside, +
+ 1 +
+ ); + + const requestData = SUSPENSE_ROOT.requests.get(1); + + assert.equal(requestData?.running, 1); + assert.equal(requestData?.sent, false); + } finally { + assert.ok(SUSPENSE_ROOT.requests.has(1)); + + // cleans up + SUSPENSE_ROOT.requests.delete(1); + } }); it('tests sync errors are thrown', async () => {