From 8128a21a5f8dffffba0181670ca3220d8f59164d Mon Sep 17 00:00:00 2001 From: Titus Wormer Date: Fri, 14 Feb 2025 15:38:24 +0100 Subject: [PATCH 1/3] Add support for async plugins This commit adds 2 new components that support turning markdown into react nodes, asynchronously. There are different ways to support async things in React. Component with hooks only run on the client. Components yielding promises are not supported on the client. To support different scenarios and the different ways the future could develop, these choices are made explicit to users. Users can choose whether `MarkdownAsync` or `MarkdownHooks` fits their use case. Closes GH-680. Closes GH-682. --- index.js | 7 ++- lib/index.js | 144 ++++++++++++++++++++++++++++++++++++++++++--------- package.json | 3 ++ readme.md | 51 +++++++++++++++++- test.jsx | 98 +++++++++++++++++++++++++++++++++-- 5 files changed, 274 insertions(+), 29 deletions(-) diff --git a/index.js b/index.js index 174bffe..629aec0 100644 --- a/index.js +++ b/index.js @@ -6,4 +6,9 @@ * @typedef {import('./lib/index.js').UrlTransform} UrlTransform */ -export {Markdown as default, defaultUrlTransform} from './lib/index.js' +export { + MarkdownAsync, + MarkdownHooks, + Markdown as default, + defaultUrlTransform +} from './lib/index.js' diff --git a/lib/index.js b/lib/index.js index 529639c..265f3bf 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,9 +1,10 @@ /** * @import {Element, ElementContent, Nodes, Parents, Root} from 'hast' + * @import {Root as MdastRoot} from 'mdast' * @import {ComponentProps, ElementType, ReactElement} from 'react' * @import {Options as RemarkRehypeOptions} from 'remark-rehype' * @import {BuildVisitor} from 'unist-util-visit' - * @import {PluggableList} from 'unified' + * @import {PluggableList, Processor} from 'unified' */ /** @@ -95,6 +96,7 @@ import {unreachable} from 'devlop' import {toJsxRuntime} from 'hast-util-to-jsx-runtime' import {urlAttributes} from 'html-url-attributes' import {Fragment, jsx, jsxs} from 'react/jsx-runtime' +import {createElement, use, useEffect, useState} from 'react' import remarkParse from 'remark-parse' import remarkRehype from 'remark-rehype' import {unified} from 'unified' @@ -108,6 +110,7 @@ const changelog = const emptyPlugins = [] /** @type {Readonly} */ const emptyRemarkRehypeOptions = {allowDangerousHtml: true} +const resolved = /** @type {Promise} */ (Promise.resolve()) const safeProtocol = /^(https?|ircs?|mailto|xmpp)$/i // Mutable because we `delete` any time it’s used and a message is sent. @@ -149,26 +152,88 @@ const deprecations = [ /** * Component to render markdown. * + * This is a synchronous component. + * When using async plugins, + * see {@linkcode MarkdownAsync} or {@linkcode MarkdownHooks}. + * * @param {Readonly} options * Props. * @returns {ReactElement} * React element. */ export function Markdown(options) { - const allowedElements = options.allowedElements - const allowElement = options.allowElement - const children = options.children || '' - const className = options.className - const components = options.components - const disallowedElements = options.disallowedElements + const processor = createProcessor(options) + const file = createFile(options) + return post(processor.runSync(processor.parse(file), file), options) +} + +/** + * Component to render markdown with support for async plugins + * through async/await. + * + * Components returning promises is supported on the server. + * For async support on the client, + * see {@linkcode MarkdownHooks}. + * + * @param {Readonly} options + * Props. + * @returns {Promise} + * Promise to a React element. + */ +export async function MarkdownAsync(options) { + const processor = createProcessor(options) + const file = createFile(options) + const tree = await processor.run(processor.parse(file), file) + return post(tree, options) +} + +/** + * Component to render markdown with support for async plugins through hooks. + * + * Hooks run on the client. + * For async support on the server, + * see {@linkcode MarkdownAsync}. + * + * @param {Readonly} options + * Props. + * @returns {ReactElement} + * React element. + */ +export function MarkdownHooks(options) { + const processor = createProcessor(options) + const [promise, setPromise] = useState( + /** @type {Promise} */ (resolved) + ) + + useEffect( + /* c8 ignore next 4 -- hooks are client-only. */ + function () { + const file = createFile(options) + setPromise(processor.run(processor.parse(file), file)) + }, + [options.children] + ) + + const tree = use(promise) + + /* c8 ignore next -- hooks are client-only. */ + return tree ? post(tree, options) : createElement(Fragment) +} + +/** + * Set up the `unified` processor. + * + * @param {Readonly} options + * Props. + * @returns {Processor} + * Result. + */ +function createProcessor(options) { const rehypePlugins = options.rehypePlugins || emptyPlugins const remarkPlugins = options.remarkPlugins || emptyPlugins const remarkRehypeOptions = options.remarkRehypeOptions ? {...options.remarkRehypeOptions, ...emptyRemarkRehypeOptions} : emptyRemarkRehypeOptions - const skipHtml = options.skipHtml - const unwrapDisallowed = options.unwrapDisallowed - const urlTransform = options.urlTransform || defaultUrlTransform const processor = unified() .use(remarkParse) @@ -176,6 +241,19 @@ export function Markdown(options) { .use(remarkRehype, remarkRehypeOptions) .use(rehypePlugins) + return processor +} + +/** + * Set up the virtual file. + * + * @param {Readonly} options + * Props. + * @returns {VFile} + * Result. + */ +function createFile(options) { + const children = options.children || '' const file = new VFile() if (typeof children === 'string') { @@ -188,11 +266,27 @@ export function Markdown(options) { ) } - if (allowedElements && disallowedElements) { - unreachable( - 'Unexpected combined `allowedElements` and `disallowedElements`, expected one or the other' - ) - } + return file +} + +/** + * Process the result from unified some more. + * + * @param {Nodes} tree + * Tree. + * @param {Readonly} options + * Props. + * @returns {ReactElement} + * React element. + */ +function post(tree, options) { + const allowedElements = options.allowedElements + const allowElement = options.allowElement + const components = options.components + const disallowedElements = options.disallowedElements + const skipHtml = options.skipHtml + const unwrapDisallowed = options.unwrapDisallowed + const urlTransform = options.urlTransform || defaultUrlTransform for (const deprecation of deprecations) { if (Object.hasOwn(options, deprecation.from)) { @@ -212,26 +306,28 @@ export function Markdown(options) { } } - const mdastTree = processor.parse(file) - /** @type {Nodes} */ - let hastTree = processor.runSync(mdastTree, file) + if (allowedElements && disallowedElements) { + unreachable( + 'Unexpected combined `allowedElements` and `disallowedElements`, expected one or the other' + ) + } // Wrap in `div` if there’s a class name. - if (className) { - hastTree = { + if (options.className) { + tree = { type: 'element', tagName: 'div', - properties: {className}, + properties: {className: options.className}, // Assume no doctypes. children: /** @type {Array} */ ( - hastTree.type === 'root' ? hastTree.children : [hastTree] + tree.type === 'root' ? tree.children : [tree] ) } } - visit(hastTree, transform) + visit(tree, transform) - return toJsxRuntime(hastTree, { + return toJsxRuntime(tree, { Fragment, // @ts-expect-error // React components are allowed to return numbers, diff --git a/package.json b/package.json index ccf0e05..4ab537f 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ ], "dependencies": { "@types/hast": "^3.0.0", + "@types/mdast": "^4.0.0", "devlop": "^1.0.0", "hast-util-to-jsx-runtime": "^2.0.0", "html-url-attributes": "^3.0.0", @@ -65,12 +66,14 @@ "@types/react": "^19.0.0", "@types/react-dom": "^19.0.0", "c8": "^10.0.0", + "concat-stream": "^2.0.0", "esbuild": "^0.25.0", "eslint-plugin-react": "^7.0.0", "prettier": "^3.0.0", "react": "^19.0.0", "react-dom": "^19.0.0", "rehype-raw": "^7.0.0", + "rehype-starry-night": "^2.0.0", "remark-cli": "^12.0.0", "remark-gfm": "^4.0.0", "remark-preset-wooorm": "^11.0.0", diff --git a/readme.md b/readme.md index 05080a9..3fb00ff 100644 --- a/readme.md +++ b/readme.md @@ -32,6 +32,8 @@ React component to render markdown. * [Use](#use) * [API](#api) * [`Markdown`](#markdown) + * [`MarkdownAsync`](#markdownasync) + * [`MarkdownHooks`](#markdownhooks) * [`defaultUrlTransform(url)`](#defaulturltransformurl) * [`AllowElement`](#allowelement) * [`Components`](#components) @@ -166,7 +168,10 @@ createRoot(document.body).render( ## API -This package exports the following identifier: +This package exports the identifiers +[`MarkdownAsync`][api-markdown-async], +[`MarkdownHooks`][api-markdown-hooks], +and [`defaultUrlTransform`][api-default-url-transform]. The default export is [`Markdown`][api-markdown]. @@ -174,6 +179,46 @@ The default export is [`Markdown`][api-markdown]. Component to render markdown. +This is a synchronous component. +When using async plugins, +see [`MarkdownAsync`][api-markdown-async] or +[`MarkdownHooks`][api-markdown-hooks]. + +###### Parameters + +* `options` ([`Options`][api-options]) + — props + +###### Returns + +React element (`JSX.Element`). + +### `MarkdownAsync` + +Component to render markdown with support for async plugins +through async/await. + +Components returning promises is supported on the server. +For async support on the client, +see [`MarkdownHooks`][api-markdown-hooks]. + +###### Parameters + +* `options` ([`Options`][api-options]) + — props + +###### Returns + +Promise to a React element (`Promise`). + +### `MarkdownHooks` + +Component to render markdown with support for async plugins through hooks. + +Hooks run on the client. +For async support on the server, +see [`MarkdownAsync`][api-markdown-async]. + ###### Parameters * `options` ([`Options`][api-options]) @@ -779,6 +824,10 @@ abide by its terms. [api-markdown]: #markdown +[api-markdown-async]: #markdownasync + +[api-markdown-hooks]: #markdownhooks + [api-options]: #options [api-url-transform]: #urltransform diff --git a/test.jsx b/test.jsx index ccc0b7b..fd84776 100644 --- a/test.jsx +++ b/test.jsx @@ -7,21 +7,29 @@ import assert from 'node:assert/strict' import test from 'node:test' -import {renderToStaticMarkup} from 'react-dom/server' -import Markdown from 'react-markdown' +import concatStream from 'concat-stream' +import {renderToPipeableStream, renderToStaticMarkup} from 'react-dom/server' +import Markdown, {MarkdownAsync, MarkdownHooks} from 'react-markdown' import rehypeRaw from 'rehype-raw' +import rehypeStarryNight from 'rehype-starry-night' import remarkGfm from 'remark-gfm' import remarkToc from 'remark-toc' import {visit} from 'unist-util-visit' -test('react-markdown', async function (t) { +const decoder = new TextDecoder() + +test('react-markdown (core)', async function (t) { await t.test('should expose the public api', async function () { assert.deepEqual(Object.keys(await import('react-markdown')).sort(), [ + 'MarkdownAsync', + 'MarkdownHooks', 'default', 'defaultUrlTransform' ]) }) +}) +test('Markdown', async function (t) { await t.test('should work', function () { assert.equal(renderToStaticMarkup(), '

a

') }) @@ -1078,3 +1086,87 @@ test('react-markdown', async function (t) { } }) }) + +test('MarkdownAsync', async function (t) { + await t.test('should support `MarkdownAsync` (1)', async function () { + assert.throws(function () { + renderToStaticMarkup() + }, /A component suspended while responding to synchronous input/) + }) + + await t.test('should support `MarkdownAsync` (2)', async function () { + return new Promise(function (resolve, reject) { + renderToPipeableStream() + .pipe( + concatStream({encoding: 'u8'}, function (data) { + assert.equal(decoder.decode(data), '

a

') + resolve() + }) + ) + .on('error', reject) + }) + }) + + await t.test( + 'should support async plugins w/ `MarkdownAsync` (`rehype-starry-night`)', + async function () { + return new Promise(function (resolve) { + renderToPipeableStream( + + ).pipe( + concatStream({encoding: 'u8'}, function (data) { + assert.equal( + decoder.decode(data), + '
console.log(3.14)\n
' + ) + resolve() + }) + ) + }) + } + ) +}) + +// Note: hooks are not supported on the “server”. +test('MarkdownHooks', async function (t) { + await t.test('should support `MarkdownHooks` (1)', async function () { + assert.throws(function () { + renderToStaticMarkup() + }, /A component suspended while responding to synchronous input/) + }) + + await t.test('should support `MarkdownHooks` (2)', async function () { + return new Promise(function (resolve, reject) { + renderToPipeableStream() + .pipe( + concatStream({encoding: 'u8'}, function (data) { + assert.equal(decoder.decode(data), '') + resolve() + }) + ) + .on('error', reject) + }) + }) + + await t.test( + 'should support async plugins w/ `MarkdownHooks` (`rehype-starry-night`)', + async function () { + return new Promise(function (resolve) { + renderToPipeableStream( + + ).pipe( + concatStream({encoding: 'u8'}, function (data) { + assert.equal(decoder.decode(data), '') + resolve() + }) + ) + }) + } + ) +}) From 609b60a37facc335fd797e63f1248d999db053fb Mon Sep 17 00:00:00 2001 From: Titus Wormer Date: Fri, 14 Feb 2025 16:47:18 +0100 Subject: [PATCH 2/3] some more --- lib/index.js | 5 +++-- readme.md | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/index.js b/lib/index.js index 265f3bf..f83bc8d 100644 --- a/lib/index.js +++ b/lib/index.js @@ -171,7 +171,7 @@ export function Markdown(options) { * Component to render markdown with support for async plugins * through async/await. * - * Components returning promises is supported on the server. + * Components returning promises are supported on the server. * For async support on the client, * see {@linkcode MarkdownHooks}. * @@ -190,7 +190,8 @@ export async function MarkdownAsync(options) { /** * Component to render markdown with support for async plugins through hooks. * - * Hooks run on the client. + * This uses `useEffect` and `useState` hooks. + * Hooks run on the client and do not immediately render something. * For async support on the server, * see {@linkcode MarkdownAsync}. * diff --git a/readme.md b/readme.md index 3fb00ff..d1ff26c 100644 --- a/readme.md +++ b/readme.md @@ -198,7 +198,7 @@ React element (`JSX.Element`). Component to render markdown with support for async plugins through async/await. -Components returning promises is supported on the server. +Components returning promises are supported on the server. For async support on the client, see [`MarkdownHooks`][api-markdown-hooks]. @@ -215,7 +215,8 @@ Promise to a React element (`Promise`). Component to render markdown with support for async plugins through hooks. -Hooks run on the client. +This uses `useEffect` and `useState` hooks. +Hooks run on the client and do not immediately render something. For async support on the server, see [`MarkdownAsync`][api-markdown-async]. From 434dac0be19ac5e352118330148d9b1bed3d3478 Mon Sep 17 00:00:00 2001 From: Remco Haszing Date: Mon, 17 Feb 2025 19:50:45 +0100 Subject: [PATCH 3/3] Avoid bad hooks dependency usage in MarkdownHooks As some comments point out in #890, there are some bad React hook usages in those changes. A `useEffect()`, `useMemo()`, or `useCallback()` should always specify all of its dependencies in the dependency array. But the dependencies need to be referentially equal in order to not trigger the hook, which is often not possible, given that plugin arrays are often created during render. This change replaces the `useEffect()` logic with a `useRef()`. Using `useRef()`, we can persist an object instance across rerenders. We can then perform our own props comparison logic to update or not instead of relying on a dependency array. We also avoid `setState()`, which triggered another render and could cause flashes of empty content. The processor is updated if `remarkRehypeOptions`, `rehypePlugins`, or `remarkPlugins` changed. The promise is updated if the processor or `children` has changed. --- lib/index.js | 123 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 105 insertions(+), 18 deletions(-) diff --git a/lib/index.js b/lib/index.js index f83bc8d..1fe3be2 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1,7 +1,7 @@ /** * @import {Element, ElementContent, Nodes, Parents, Root} from 'hast' * @import {Root as MdastRoot} from 'mdast' - * @import {ComponentProps, ElementType, ReactElement} from 'react' + * @import {ComponentProps, ElementType, ReactElement, ReactNode, RefObject} from 'react' * @import {Options as RemarkRehypeOptions} from 'remark-rehype' * @import {BuildVisitor} from 'unist-util-visit' * @import {PluggableList, Processor} from 'unified' @@ -92,11 +92,18 @@ * Transformed URL (optional). */ +/** + * @typedef RefState + * @property {Options} options + * @property {Processor} processor + * @property {Promise} promise + */ + import {unreachable} from 'devlop' import {toJsxRuntime} from 'hast-util-to-jsx-runtime' import {urlAttributes} from 'html-url-attributes' import {Fragment, jsx, jsxs} from 'react/jsx-runtime' -import {createElement, use, useEffect, useState} from 'react' +import {use, useRef} from 'react' import remarkParse from 'remark-parse' import remarkRehype from 'remark-rehype' import {unified} from 'unified' @@ -110,7 +117,6 @@ const changelog = const emptyPlugins = [] /** @type {Readonly} */ const emptyRemarkRehypeOptions = {allowDangerousHtml: true} -const resolved = /** @type {Promise} */ (Promise.resolve()) const safeProtocol = /^(https?|ircs?|mailto|xmpp)$/i // Mutable because we `delete` any time it’s used and a message is sent. @@ -197,28 +203,38 @@ export async function MarkdownAsync(options) { * * @param {Readonly} options * Props. - * @returns {ReactElement} + * @returns {ReactNode} * React element. */ export function MarkdownHooks(options) { - const processor = createProcessor(options) - const [promise, setPromise] = useState( - /** @type {Promise} */ (resolved) - ) + const ref = /** @type {RefObject} */ (useRef({})) + const prev = ref.current + let processor = prev.processor - useEffect( - /* c8 ignore next 4 -- hooks are client-only. */ - function () { - const file = createFile(options) - setPromise(processor.run(processor.parse(file), file)) - }, - [options.children] - ) + if ( + !processor || + prev.options.remarkRehypeOptions !== options.remarkRehypeOptions || + arePluggableListsEqual(prev.options.rehypePlugins, options.rehypePlugins) || + arePluggableListsEqual(prev.options.remarkPlugins, options.remarkPlugins) + ) { + processor = createProcessor(options) + } + + if ( + prev.processor !== processor || + prev.options.children !== options.children + ) { + const file = createFile(options) + prev.processor = processor + prev.promise = processor.run(processor.parse(file), file) + } - const tree = use(promise) + const tree = use(prev.promise) /* c8 ignore next -- hooks are client-only. */ - return tree ? post(tree, options) : createElement(Fragment) + if (tree) { + return post(tree, options) + } } /** @@ -428,3 +444,74 @@ export function defaultUrlTransform(value) { return '' } + +/** + * Check if two unified pluggable lists are equal. + * + * Pluggable lists are considered equal if: + * - Both are empty or nullish. + * - Both contain the same plugins with the same options. + * + * @param {PluggableList | null | undefined} oldList + * The pluggable list that was passed in the last React render call. + * @param {PluggableList | null | undefined} newList + * The pluggable list that was passed in the new React render call. + * @returns {boolean} + * Whether or not the pluggable lists are equal. + */ +function arePluggableListsEqual(oldList, newList) { + if (oldList === newList) { + return true + } + + // Consider nullish or empty pluggable lists equal. + if (!oldList?.length) { + return !newList?.length + } + + if (!newList?.length) { + return false + } + + if (oldList.length !== newList.length) { + return false + } + + for (let index = 0; index < oldList.length; index++) { + let oldPluggable = oldList[index] + let newPluggable = newList[index] + + if (oldPluggable === newPluggable) { + continue + } + + /** @type {unknown} */ + let oldOptions + /** @type {unknown} */ + let newOptions + + // It’s a tuple. Unwrap it. + if (Array.isArray(oldPluggable)) { + // Consider null and undefined options equal + oldOptions = oldPluggable[1] ?? undefined + oldPluggable = oldPluggable[0] + } + + // It’s a tuple. Unwrap it. + if (Array.isArray(newPluggable)) { + // Consider null and undefined options equal + newOptions = newPluggable[1] ?? undefined + newPluggable = newPluggable[0] ?? undefined + } + + if (oldPluggable !== newPluggable) { + return false + } + + if (oldOptions !== newOptions) { + return false + } + } + + return false +}