Remove intermediate render cycle from useEditor

[C-Hess]

~~May or may not be a breaking change depending on if any people actually required this behavior. That would surprise me though.~~

Was a breaking change because it broke SSR, though that's probably not something TipTap should support long term anyways. Instead, opted for an alternative hook to be used until a major version change.

Please describe your changes

Possibly fixes #3345 (issue can probably be re-opened)

We've ran issues getting TipTap operating smoothly with virtual scrolling libraries as well because they ideally expect the height of a component be defined after the first render cycle. While we can't guarantee that Prosemirror doesn't introduce their own delays in rendering, I think the least we can do is remove the unnecessary delay in the React TipTap wrapper.

How did you accomplish your changes

The change really is just moving initialization to the first render cycle. As a nice side effect, this also means you also no longer have to check if the editor is null before using it ❤

How have you tested your changes

~~I ran through most of the tests and they seem to be passing. I spot checked a few test failures and they appear unrelated to the changes made.~~

No longer relevant. New approach does not touch existing hook.

How can we verify your changes

Verification might be a little involved. Didn't really want to get react virtual scrolling all setup in an example at this time, plus like I said, there is no guarantee's it would eliminate the problem entirely. At a minimum though, it removes an unnecessary render cycle.

Checklist

• [X] The changes are not breaking the editor
• [X] Added tests where possible
  • It doesn't really seam testable with the current fixures
• [X] Followed the guidelines
• [X] Fixed linting issues

Related issues

#3345

[netlify[bot]]

✅ Deploy Preview for tiptap-embed ready!

Name	Link
🔨 Latest commit	fbf13bd87345eed9638055b9af9f14b1650b63a1
🔍 Latest deploy log	https://app.netlify.com/sites/tiptap-embed/deploys/65cfce5f29a47f0008d07249
😎 Deploy Preview	https://deploy-preview-4579--tiptap-embed.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[C-Hess]

@janthurau , Sorry to put pressure, but any chance that we could get this reviewed? It would be awesome for us because it could potentially help resolve issues with virtual scrolling libraries.

[janthurau]

hey @C-Hess, absolutely. We haven't had a look here yet as the tests / build steps are still failing:

7:37:54 AM: ../packages/react/src/Context.tsx(36,26): error TS2322: Type "Editor | null" is not assignable to type "Editor".
7:37:54 AM:   Type "null" is not assignable to type "Editor".

Do you think you can fix them?

[C-Hess]

Oops 😳, at one point I thought the build was failing for unrelated reasons. I'll resolve the issue, sorry

[ondrejsevcik]

@C-Hess thanks for taking initiative to fix this issue 🙏🏼 highly appreciated.

[rfgamaral]

@C-Hess This was implemented before but reverted because it doesn't work with SSR. You may want to go through that PR discussion, and perhaps find a solution that doesn't break SSR, and doesn't introduce others (IIRC, nobody was able to come up with a solution without issues).

For reference, we have a custom useEditor hook in @doist/typist - we don't care for SSR for the time being - but it's outdated compared to the one in Tiptap. So, if we could have the useEditor in Tiptap fixed (SSR support, init on first render, don't cause additional issues), that would be awesome.

Good luck!

[ondrejsevcik]

It would be good to hear from the owners of the TipTap to understand their perspective on this topic. Maybe @janthurau knows?

In my opinion it would be better to split it up into two separate use cases. SSR for TipTap editor does not make much sense. The WYSIWYG is component meant to be used in browser and it will always struggle in server side environment. Trying to bend it will bring unnecessary complexity. I would rather encourage to use HTML helpers for SSR instead.

[C-Hess]

Thanks for all of the useful comments. I didn't know it was attempted before, but it definitely helps to know.

I changed the approach to be non-breaking. In a future major version of TipTap, I'm hoping that useEditorForImmediateRender can replace the logic within useEffect with the caveat that TipTap will no longer be usable through server side rendering (which from my understanding, given the comments above, it shouldn't have been usable via server-side rendering anyways)

[C-Hess]

Going to rely on CI for testing as I'm not home yet to verify my updates. I updated the React editor context and main demo file to optionally use the newer hook.

I'll test this later tonight

[C-Hess]

Should be good to review @bdbch and @svenadlung. Let me know if you want to tweak the approach I went with here. Thanks!

[corbinkems]

I'm also running into this issue and it's causing a lot of headaches. @janthurau

[C-Hess]

@bdbch , @svenadlung, @janthurau sorry to bother you all. Is there any chance this could be looked at? I see two other PRs introduced by you both that both have different ways of doing this on top of this one. Each approach has pros and cons, but it would be awesome for us to be able to resolve this issue. This is a high priority for my organization to help hopefully resolve virtual scrolling issues when using TipTap, so I'd be more than happy to make any suggested changes as soon as possible if requested.

Thank you all for your hard work on this Repo!

[hjoelh]

Ran into this issue today, until this or another fix is merged - my current workaround is like this

  {/* 
      due to https://github.com/ueberdosis/tiptap/pull/4579
      editor is null on initial render 😢, to prevent a flicker we render 
      an empty box with the same styles as the edtior 
      */}
      {editor ? (
        <EditorContent editor={editor} />
      ) : (
        <Box className={styles["editor"]} />
      )}

[Nantris]

@C-Hess would this potentially address #5166? I think probably not based on quickly looking over it, but figured I'd ask to be sure.

[nperez0111]

I took a stab at this, taking a slightly different approach for it: https://github.com/ueberdosis/tiptap/pull/5161

[nperez0111]

Resolved with: https://github.com/ueberdosis/tiptap/pull/5161

[nperez0111]

@C-Hess Very much appreciate your contribution, was definitely a source of inspiration for my change