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

setState and useEditorStateWithDispatch #139

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BrianHung
Copy link

  • Adds setState as an optional prop.
  • Exposes a useEditorStateWithDispatch that either uses that setState prop or the internal one.

Related to conversation here #98 (comment).
I've been finding myself doing dynamic registration of plugin states conditional on React components being rendered. useEditorState provides state only; if I wanted to update it, I would have to pass down the controlled setState or pass it down with a context.

Decided to re-use the EditorContext instead.

@BrianHung BrianHung requested review from smoores-dev and a team as code owners September 27, 2024 08:32
@tilgovi
Copy link
Contributor

tilgovi commented Sep 27, 2024

Can you show an example of what you kind of calls to setState() you're doing? I might be okay with just doing this from the regular useEditorState(), to make it more like useState(). I don't like calling this useEditorStateWithDispatch() because setState() is not dispatchTransaction().

@smoores-dev
Copy link
Collaborator

smoores-dev commented Sep 27, 2024

@tilgovi in the issue that Brian referenced, this was about using editorState.reconfigure to add and remove plugins from React components (#98 (comment))

@tilgovi
Copy link
Contributor

tilgovi commented Sep 27, 2024

I'd still like to hear a little bit more about when dynamic plugin registration is useful. I believe it could be, but what I've been waiting and wanting to see more clearly is how useful plugins are and why.

Am I more or less right if I say that plugins can basically do three things? Plugins can augment state, contribute view props, or act as transaction middleware. It is nice that these can be wrapped up together to encapsulate a concern that cuts across them all, and I see the appeal of managing all of it within the mount lifecycle of a React component. I'm just interested in taking a moment to pause and reflect on what a plugin needs to do and whether the ProseMirror plugin API is the best way to do it in react-prosemirror.

The answer should be by default be "yes" because react-prosemirror tries to be close to the ProseMirror API. But I just want to be thoughtful!

@tilgovi
Copy link
Contributor

tilgovi commented Sep 27, 2024

But I just want to be thoughtful!

Not that anyone here isn't being thoughtful, just that I am contributing my part to it by being the skeptical maintainer.

@tilgovi
Copy link
Contributor

tilgovi commented Sep 27, 2024

So, backing up to this first part:

I've been finding myself doing dynamic registration of plugin states conditional on React components being rendered.

What condition determines whether the components are rendered? Is that a build decision or a runtime decision? If it's a runtime decision, could we invert the control and have the React components conditionally rendered based on whether the plugins are loaded?

@smoores-dev
Copy link
Collaborator

I agree with @tilgovi, I think. I think in general I could be convinced that there could be value in supporting the kinds of patterns that exist "in the wild" in existing prosemirror code, but this literal example of a word count plugin is actually quite well suited to living entirely in React state, rather than a plugin!

I think that the most valuable role of a plugin in the React ProseMirror context is as a transaction middleware, since you can't replace that role with React state management or component rendering. Have you found yourself using/wanting this pattern for any plugins that need access to transaction data/metadata?

@BrianHung
Copy link
Author

BrianHung commented Sep 27, 2024

this literal example of a word count plugin is actually quite well suited to living entirely in React state

The implementation of the word count plugin I had was a bit simple, since its state is entirely derived from the EditorState and therefore could be done with useEffect and useEditorState.

A more performant implementation of that would probably use the transaction in the apply plugin state prop, and operate on steps / changes instead to not recompute from scratch every time.

Am I more or less right if I say that plugins can basically do three things? Plugins can augment state, contribute view props, or act as transaction middleware.

I think that the most valuable role of a plugin in the React ProseMirror context is as a transaction middleware, since you can't replace that role with React state management or component rendering.

Yes to both points. ProseMirror plugins are a nice encapsulation when you really have to use all three capabilities.

For example, I was building a @-style autocompletion menu. It needs to

  • store the text range that is matching, along with the RegExpExecArray (state)
  • map that range through transactions, exit if selection has been set tr.selectionSet (state)
  • registers handleTextInput to run the text-matching, decorations as a virtual reference to position a floating menu below the text range (view)
  • appendTransaction to handle deletions (tr middleware)

I have then a separate React component EditorAutocompleteMenu to

  • handle positioning the dropdown menu (useEditorEffect)
  • listening to arrow keys to change active menu item useEditorEventListener
  • insert completions with useEditorEventCallback

The use of both ProseMirror and React state here makes sense because the former is really tied to the EditorState lifecycle, and latter only handles UI state.

To integrate both the plugin and React menu, I have to make the React component a child of ProseMirror and then also modify the plugins field of the initial EditorState to include it.

That DX doesn't feel great as you have to import both vanilla and React code wherever your editor component is kept.

The alternative today is to have the React menu register the plugin dynamically whenever EditorAutocompleteMenu is actually used, passing setState from the editor component. If we have multiple types of these components however, then we would see setState={setState} many times over.

If we modified useEditorState to have setState attached, we would have a cleaner DX of one-line changes

<Editor>
  <EditorAutocompleteMenu/>
</Editor>

where it'd be easy to add or remove React components (and augment the ProseMirror view simultaneously), without having to touch initialization or reach into the editor component.

That itself allows me to build a re-usable Editor component if I wanted a chat interface with specific React components + plugins, or a document interface with a different set.

@BrianHung
Copy link
Author

Updated the PR to just use useEditorState. Note that it would be a breaking change for consumers of this library.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 8, 2024

If we had a way to add transaction middleware, would we still want to store the state of an autocomplete component in editor state? Or would we rather store it in React component state?

@tilgovi
Copy link
Contributor

tilgovi commented Oct 8, 2024

Maybe answering my own question, we would still need a way to add decorations, right? We can't just do it all in the React component. We have to add a plugin somewhere. Is that correct?

@BrianHung
Copy link
Author

BrianHung commented Oct 8, 2024

add transaction middleware

You could do this by having a trUpdate plugin that just emits the tr on state.apply.

can't just do it all in the React component

Not easily. You would basically be reimplementing the ProseMirror plugin system in React user-land.

I find for simple use-cases — where you’re only using one plugin prop, and don’t want to create a new plugin just for that — it does make sense to use the react api like useEditorEffect.

For complicated cases (using state, view, decos etc), bite the bullet and learn how to use ProseMirror plugins instead of a React only abstraction.

The other benefit of sticking with ProseMirror is that you can reuse plugins made by other people, as opposed to a React + ProseMirror only solution.

Speaking from experience where I’ve seen TipTap invent an abstraction on top of ProseMirror but now their components aren’t easily plug and play outside their ecosystem.

@tilgovi
Copy link
Contributor

tilgovi commented Oct 8, 2024

The other benefit of sticking with ProseMirror is that you can reuse plugins made by other people, as opposed to a React + ProseMirror only solution.

Yes, I do like that, although it may be brittle of those plugins manipulate the DOM. What works now might not work in the future if we make the entire editor view React-rendered.

Speaking from experience where I’ve seen TipTap invent an abstraction on top of ProseMirror but now their components aren’t easily plug and play outside their ecosystem.

I'd like to believe that using React ProseMirror leads to creating ProseMirror code that'd be reusable without the React integration, but time will tell if that's true. I almost wonder if that's an argument against making it possible for components to register plugins, because then authors might export the components and not make the plugin itself public.

For the moment, let's focus on solving your problems. How bad is it now if you have to both add the plugin and the component separately?

@BrianHung
Copy link
Author

How bad is it now if you have to both add the plugin and the component separately?

Right now, it means for every different editor surface, they can’t share initialization boilerplate because of different plugins.

Just doesn’t feel React ergonomic if for every editor-dependent component, you’d have to change a parent prop as well.

The solution would be to wrap ProseMirror inside of an editor state context provider, and to use that within your Editor component.

Not that many lines of code which is also why I submitted this PR.

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

Successfully merging this pull request may close these issues.

3 participants