Add support for async plugins

[wooorm]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and discussions and couldn’t find anything or linked relevant results below
• I made sure the docs are up to date
• I included tests (or that’s not needed)

Description of changes

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.

[wooorm]

worth noting that I tested this on the demo/playground here and it works well

[ChristianMurphy]

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

[remcohaszing]

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.

[wooorm]

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?

[wooorm]

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

[remcohaszing]

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.

[remcohaszing]

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

[wooorm]

you want an array of every field in options?

[wooorm]

that’s why processor is outside.

[wooorm]

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

[remcohaszing]

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.

[wooorm]

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.

[wooorm]

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

[remcohaszing]

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.

[wooorm]

#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.

[remcohaszing]

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]
)

[remcohaszing]

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

[wooorm]

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

[remcohaszing]

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.

[wooorm]

Released in 9.1.0!