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

Question regarding the status of next releases #141

Closed
strogonoff opened this issue Oct 7, 2024 · 3 comments
Closed

Question regarding the status of next releases #141

strogonoff opened this issue Oct 7, 2024 · 3 comments

Comments

@strogonoff
Copy link

strogonoff commented Oct 7, 2024

First, thank you for publishing & maintaining this package. From lurking around at discuss.ProseMirror, it stands out as the best among the options for building an extensible PM-powered editor that supports React node views and comes without a mountain of dependencies.

Seeing the next releases being mentioned in a few places as offering a new architecture that addresses some of the issues (like #75), I’m considering starting with that & avoiding a future migration that would otherwise be required… Some related questions (thought I’d ask here before I figure out how to log in to Gitter with my Matrix).

  1. What would a high-level summary of architectural changes look like? Sorry if I’m blind and it’s in the README already.

  2. The next-built branch (if that’s the correct one) has a different README; is it safe to rely on it (as being up-to-date with next architecture)? Any notable omissions?

    1. I see that NodeViewComponentProps in the new README doesn’t match type declaration, which moves nodeProps under a sub-key. Is that stable?
    2. In the section about node views, the new README specifies the nodeViews prop in a way that does not provide referential equality (nodeViews={{ paragraph: Paragraph }}), whereas the old README has a provision about defining node views outside of render cycle. Is having a stable nodeViews reference no longer a concern with the new architecture, or that part is a consequence of branch divergence (I could make a PR fixing README in next-built, if so)?
  3. Is next generally ready for production use, or is in fact being used in production as far as anyone knows? I noticed a commit adding a test suite, which seems like good news.

  4. Both old and next README note that:

    The other way to integrate React and ProseMirror is to have ProseMirror render node views using React components

    Is it correct to assume that this “other” way is the only way of having React components work as node views, and the “original” way would be using ProseMirror in React context but with either no node views or “simple” PM-native node views that are not React components? (Which would make sense, now that I think about it.)

    1. Is it correct to think that even with all the help of this library React-based node views might never be as fully smoothly integrated as PM-native node views, and when architecting for long term anchoring to React-only node views might cause issues down the line? Or, was it the case before, but no longer with the new architecture?

If there are any bits of advice or context that someone starting [with next] should know, they’d be appreciated.

@smoores-dev
Copy link
Collaborator

Hy @strogonoff! We could probably afford to clarify the state of things a bit, hopefully this will help a bit.

What would a high-level summary of architectural changes look like? Sorry if I’m blind and it’s in the README already.

This PR description probably has the best explanation of the changes in the next tag (it's also where the work on the next tag is happening): #47. The short version is that we actually reimplement the ProseMirror View DOM update system with React components. This lets us simplify the update model for the EditorView, along with some other benefits.

The next-built branch (if that’s the correct one) has a different README; is it safe to rely on it (as being up-to-date with next architecture)? Any notable omissions?

Check out the react-node-views branch; that's the one that gets deployed to the next tag on NPM (sorry for the confusing naming). The README is largely up to date, but missing a few small features (e.g. there's a useStopEvent hook now, which I think I forgot to make an entry for), and there's likely one breaking change to the NodeViewComponentProps API (changing pos: number to getPos: () => number) that'll be merged in shortly, maybe this week. Also the nodeProps change isn't in there, as you noted!

The nodeProps key I would consider stable, yes! That allows you to easily pull out all of the props that can't be spread onto the root element of the node view, which is important (because you do need to spread the rest of the props on there for decorations and such to work!)

  • In the section about node views, the new README specifies the nodeViews prop in a way that does not provide referential equality (nodeViews={{ paragraph: Paragraph }}), whereas the old README has a provision about defining node views outside of render cycle. Is having a stable nodeViews reference no longer a concern with the new architecture, or that part is a consequence of branch divergence (I could make a PR fixing README in next-built, if so)?

It is no longer a strict necessity, the way it is for the main branch. At the moment I think it doesn't matter, but once we merge in the performance improvements that we're working on, we'll actually want to encourage keeping that reference stable again to keep from breaking element memoization!

Is next generally ready for production use, or is in fact being used in production as far as anyone knows? I noticed a commit adding a test suite, which seems like good news.

It is; I know of at least two companies using it in production (one is https://dskrpt.de)! The test suite is a port of prosemirror-view's own test suite; as long as it's passing, I'm feeling pretty good about the robustness of the implementation.

Both old and next README note that:

The other way to integrate React and ProseMirror is to have ProseMirror render node views using React components

Is it correct to assume that this “other” way is the only way of having React components work as node views, and the “original” way would be using ProseMirror in React context but with either no node views or “simple” PM-native node views that are not React components? (Which would make sense, now that I think about it.)

Basically yes! The language may be unnecessarily confusing here; I was just trying to express that there are two "kinds" of React/ProseMirror integrations:

  1. Rendering a ProseMirror editor within a React component
  2. Rendering React components as custom node views within a ProseMirror editor

And that both are somewhat challenging, and react-prosemirror provides strategies for both!

Is it correct to think that even with all the help of this library React-based node views might never be as fully smoothly integrated as PM-native node views, and when architecting for long term anchoring to React-only node views might cause issues down the line? Or, was it the case before, but no longer with the new architecture?

I would say that was the case before, but no longer with the new architecture! With the new architecture, even simple toDOM specs are implemented as React components under the hood, so React-based node views are truly first-class citizens.

If there are any bits of advice or context that someone starting [with next] should know, they’d be appreciated.

I think everything is covered here, but I encourage you to leave feedback and ask questions as you go, for sure! Also, both the mainline branch and the next releases are pre-v1, and so not properly stable, there absolutely may be more breaking changes before we feel comfortable declaring the API stable (though we'll try to keep those to a minimum).

Also if you do have some time to open PRs updating any inconsistencies that you find in the README on the react-editor-view branch, that would be great!

@strogonoff
Copy link
Author

strogonoff commented Oct 8, 2024

@smoores-dev Thank you, really appreciated. Noted the correct branch.

And that both are somewhat challenging, and react-prosemirror provides strategies for both!

If I may ask before closing—it may sound as if the strategies are considered to be mutually exclusive… That’s not the case, is it?

@smoores-dev
Copy link
Collaborator

That is not the case; you can absolutely do both!

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