Skip to content
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.

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

React warns about flushSync usage when dynamically adding or removing plugins that use widgetViewFactory #70

Open
dragonman225 opened this issue Jun 19, 2024 · 3 comments

Comments

@dragonman225
Copy link

In my editor, I use a piece of React state to control whether a plugin should be added to or removed from ProseMirror's plugins list, to achieve the effect of enabling or disabling.

For example,

// The state to control enable/disable
const [enablePlugin, setEnablePlugin] = useState(true)

// An effect to reconfigure plugins
useEffect(() => {
  if (!enablePlugin) return

  const view = viewRef.current
  if (!view) return

  // Add plugin
  const { state } = view
  const newState = state.reconfigure({
    plugins: state.plugins.concat(decorationPlugin),
  })
  view.updateState(newState)

  return () => {
    // Remove plugin
    const currentState = view.state
    const newState = currentState.reconfigure({
      plugins: currentState.plugins.filter((plugin) => {
        return plugin !== decorationPlugin
      }),
    })
    view.updateState(newState)
  }
}, [enablePlugin, decorationPlugin])

The problem is that whenever a plugin that renders React components with widgetViewFactory is added or removed, I get an error message in the console saying:

Warning: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

Although rendering still works, I wonder if there's a way to suppress the error.

Here is the complete and working code adapted from the react example: https://github.com/dragonman225/prosemirror-adapter/blob/main/examples/react/components/Editor.tsx

@Saul-Mirone
Copy link
Collaborator

That's a problem. I'll try some other solutions like micro task.

@dragonman225
Copy link
Author

After doing some research and experiments, I found that by default ProseMirror doesn't allow appending the contentDOM of a NodeView to the document asynchronously (i.e. some time after returning the NodeView in a NodeViewConstructor). If I do so, both with a task via setTimeout() or a microtask via Promise.resolve().then(), I get DOMObserver.observer infinite loop. I guess this is why you use flushSync to make sure contentDOM is in the document before ProseMirror does anything.

But I also seemed to found a way to add contentDOM asynchronously with the help of ignoreMutation. For example, the NodeView below works without a problem in my experiments so far.

import { NodeViewConstructor } from 'prosemirror-view'

export const createTableNodeView: NodeViewConstructor = () => {
  const container = document.createElement('div')
  container.className = 'table-container'

  const table = document.createElement('table')

  setTimeout(function simulateAsyncRender() {
    const somethingInTheMiddle = document.createElement('div')
    somethingInTheMiddle.append(table)
    container.append(somethingInTheMiddle)
  }, 1000)

  // You can also use a microtask.
  // Promise.resolve().then(function simulateAsyncRender() {
  //   const somethingInTheMiddle = document.createElement('div')
  //   somethingInTheMiddle.append(table)
  //   container.append(somethingInTheMiddle)
  // })

  return {
    dom: container,
    contentDOM: table,
    ignoreMutation: (mutation) => {
      // Prevent DOMObserver.observer infinite loop.
      if (mutation.type === 'childList' && mutation.target === container) {
        return true
      }
      return false
    },
  }
}

I wonder if there are edge cases where this wouldn't work. But if this is robust enough, we may not need flushSync, or we can run it in a separate task or microtask, preventing the React warnings.

@Saul-Mirone
Copy link
Collaborator

After some investigation. I found out that seems you just need to wrap view.updateState in the queueMicroTask:

      queueMicrotask(() => {
        view.updateState(newState)
      })

I also tried to figure out if we can get rid of flushSync. Although it's possible, but it seems need to migrate to react class component for the react renderer. I don't like the idea so I'll look for other solutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants