Skip to content

Add support for async plugins #890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 20, 2025
Merged

Add support for async plugins #890

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8128a21
Add support for async plugins
wooorm Feb 14, 2025
609b60a
some more
wooorm Feb 14, 2025
51e0b2b
Remove `use` use
wooorm Feb 18, 2025
2ab6dac
Remove dependencies array
wooorm Feb 18, 2025
e8594ae
.
wooorm Feb 18, 2025
b371a12
Add a somewhat reasonable dependency array
wooorm Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
144 changes: 120 additions & 24 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'
*/

/**
Expand Down Expand Up @@ -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'
Expand All @@ -108,6 +110,7 @@ const changelog =
const emptyPlugins = []
/** @type {Readonly<RemarkRehypeOptions>} */
const emptyRemarkRehypeOptions = {allowDangerousHtml: true}
const resolved = /** @type {Promise<undefined>} */ (Promise.resolve())
const safeProtocol = /^(https?|ircs?|mailto|xmpp)$/i

// Mutable because we `delete` any time it’s used and a message is sent.
Expand Down Expand Up @@ -149,33 +152,108 @@ const deprecations = [
/**
* Component to render markdown.
*
* This is a synchronous component.
* When using async plugins,
* see {@linkcode MarkdownAsync} or {@linkcode MarkdownHooks}.
*
* @param {Readonly<Options>} 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>} options
* Props.
* @returns {Promise<ReactElement>}
* 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>} options
* Props.
* @returns {ReactElement}
* React element.
*/
export function MarkdownHooks(options) {
const processor = createProcessor(options)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The processor is only needed inside the useEffect. We can move it there as well. That means the processor isn’t created on every render and doesn’t need to be in the useEffect dependency array.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t Object.is used for the dependencies? And because Object.is({}, {}) // false, that would mean putting a created processor in dependencies would never be equal?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React also allows you to omit the array. That is what I did now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could also improve the situation by better teaching users. For example, this is not how someone would typically use this library:

import React from 'react'
import {createRoot} from 'react-dom/client'
import Markdown from 'react-markdown'
import remarkGfm from 'remark-gfm'

const markdown = `Just a link: www.nasa.gov.`

createRoot(document.body).render(
  <Markdown remarkPlugins={[remarkGfm]}>{markdown}</Markdown>
)

A more realistic example is this:

import React from 'react'
import Markdown from 'react-markdown'
import remarkGfm from 'remark-gfm'

const markdown = `Just a link: www.nasa.gov.`

function App() {
  return (
    <Markdown remarkPlugins={[remarkGfm]}>{markdown}</Markdown>
  )
}

This is problematic. The remarkPlugins changes for every rerender of <App /> The solution is this to move non-primitive props outside of the React component:

import React from 'react'
import Markdown from 'react-markdown'
import remarkGfm from 'remark-gfm'

const markdown = `Just a link: www.nasa.gov.`
const remarkPlugins = [remarkGfm]

function App() {
  return (
    <Markdown remarkPlugins={remarkPlugins}>{markdown}</Markdown>
  )
}

This was always a problem, but it’s worse when dealing with asynchronous state.

const [promise, setPromise] = useState(
/** @type {Promise<Root | undefined>} */ (resolved)
)

useEffect(
/* c8 ignore next 4 -- hooks are client-only. */
function () {
const file = createFile(options)
setPromise(processor.run(processor.parse(file), file))
},
[options.children]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependency should specify all component local values needed to run the effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you want an array of every field in options?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that’s why processor is outside.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t know what you mean by “component local values” — can you use (pseudo)code to explain your ideas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently processor needs to be in the dependency array, because it’s defined inside MarkdownHooks. That will trigger infinite rerenders.

The real fix is to move the creation of processor inside the useEffect. and add all values needed to create it to the dependency array.

Best would be to enable eslint-plugin-react-hooks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you are suggesting to add every field on options into the dependency array?
The caveats describe to not put objects in dependencies: https://react.dev/reference/react/useEffect#caveats.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React also allows you to omit the array. That is what I did now. Now there are no bugs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That means it will re-run after every render. So this is equivalent to adding every field of the options on the array, including irrelevant ones such as allowedElements and components.

We only need the options used inside the effect as dependencies. That means moving the processor inside and adding all options used by createProcessor() to the dependency array.

I still believe React’s dependency array implementation doesn’t match well with the fact that we allow objects as props. This is solved by #891, but needs fine-tuning and testing. We could even leverage this to improve performance for the synchronous API.

Copy link
Member Author

@wooorm wooorm Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#891 is the same; it recalculates too.

It seems troubling that React only allowed primitives. That sounds like a problem that react should solve—just like lacking async support.

I welcome you spending time in the future to create a good but complex diffing strategy that improves performance for all APIs. But I do not think that this issue needs to wait for that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok let’s go with this for now, but at least add the processor inside the effect and add the appropriate dependency array. We should also handle cancellation.

useEffect(
  () => {
    let cancelled = false
    const processor = createProcessor(options)
    const file = createFile(options)
    processor.run(processor.parse(file), file, function (error, tree) {
      if (!cancelled) {
        setError(error)
        setTree(tree)
      }
    })

    return () => {
      cancelled = true
    }
  },
  [options.children, options.remarkRehypeOptions, options.remarkPlugins, options.rehypePlugins]
)

We also need to think about how to handle loading states. The current implementation shows the loading state until something is rendered. It then only updates the state when the processor was done running. The previous implementation with use also showed the loading state while the processor was running.

Maybe we should add a prop for this to let the user decide:

useEffect(
  () => {
    if (options.reset) {
      setError(null)
      setTree(null)
    }

    let cancelled = false
    const processor = createProcessor(options)
    const file = createFile(options)
    processor.run(processor.parse(file), file, function (error, tree) {
      if (!cancelled) {
        setError(error)
        setTree(tree)
      }
    })

    return () => {
      cancelled = true
    }
  },
  [options.children, options.remarkRehypeOptions, options.remarkPlugins, options.reset, options.rehypePlugins]
)

)

const tree = use(promise)

/* c8 ignore next -- hooks are client-only. */
return tree ? post(tree, options) : createElement(Fragment)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don’t have to return an empty fragment here. It’s sufficient to return undefined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make the types for only this function different from the other ones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct return type for a React component is ReactNode. According to TypeScript 5.0 and before this was ReactElement, which was wrong. It is not wrong for a React component to return ReactElement, string, number, boolean, null, or undefined, but it’s an implementation detail we shouldn’t worry about. TypeScript 5.0 is almost no longer supported by the DefinitelyTyped support window. That’s a pretty good indication we don’t need to support it anymore, especially for new APIs.

This has gotten more relevant with your latest changes, as we now no longer use use. This means we also don’t integrate with <Suspense/>. I think we should add a new prop loader of type ReactNode that’s returned while the processor is still processing.

}

/**
* Set up the `unified` processor.
*
* @param {Readonly<Options>} options
* Props.
* @returns {Processor<MdastRoot, MdastRoot, Root, undefined, undefined>}
* 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)
.use(remarkPlugins)
.use(remarkRehype, remarkRehypeOptions)
.use(rehypePlugins)

return processor
}

/**
* Set up the virtual file.
*
* @param {Readonly<Options>} options
* Props.
* @returns {VFile}
* Result.
*/
function createFile(options) {
const children = options.children || ''
const file = new VFile()

if (typeof children === 'string') {
Expand All @@ -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>} 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)) {
Expand All @@ -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<ElementContent>} */ (
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,
Expand Down
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
51 changes: 50 additions & 1 deletion readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -166,14 +168,57 @@ 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].

### `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<JSX.Element>`).

### `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].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we expand on this a bit? Including that this uses useState, and so will not immediately render on initial page load?


###### Parameters

* `options` ([`Options`][api-options])
Expand Down Expand Up @@ -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
Expand Down
Loading