Make sure editor is available on first render

[ryanto]

This is a fix for #2182 - it makes sure that the editor is available on the first render of useEditor in the react package.

The reason for this fix is that the first render never has an editor and that causes the page's content to jump and flash. First render has no editor, so nothing is rendered. Then the second render has the editor and the WYSIWYG editor is rendered causing all the content on the page to shift down.

The effect will also re-create the editor if anything in the deps list changes.

cc @philippkuehn

[netlify]

✔️ Deploy Preview for tiptap-embed ready!

🔨 Explore the source changes: 0dd23f9

🔍 Inspect the deploy log: https://app.netlify.com/sites/tiptap-embed/deploys/61ba2458ec7039000a7baf23

😎 Browse the preview: https://deploy-preview-2282--tiptap-embed.netlify.app

[philippkuehn]

Oh, didn't think that was possible. Great! And it seems like this also fixes #2040

[vpontis]

Should we update the types:

export declare const useEditor: (options?: Partial<EditorOptions>, deps?: DependencyList) => Editor | null;

to not specify the null option?

[vpontis]

Actually could we revert this change?

It totally breaks any Server Side Rendering with Next.js because Tiptap can't be rendered in Node. Or it should at least be an option...

I think the new behavior doesn't make sense but it's going to break a bunch of apps that rely on SSR.

[philippkuehn]

Oh no. That’s bad. Revert seems like the best option until we found another fix for it. I’m on the way right now. Will do it later.

[ryanto]

@vpontis Nice catch.

That's a bummer! What do you guys think of having some sort of API for opting into having the editor available during the first render. Just throwing this out there but something like...

useEditor({
  availableOnFirstRender: true
});

When true it would init the editor right away and it would always be available. When false it would init the editor inside of an effect (like before) so the page can be generated on the server. We would default it to false, since that's how it worked before this PR and users can opt into true when they need it.

Trying to think of a way we can get best of both worlds. Allow folks writing client side apps to avoid the page jumping, but also allowing this hook to work seamless on the server.

[vpontis]

@ryanto yea, that makes sense to me.

[philippkuehn]

@vpontis I always thought that tiptap doesn’t work with SSR at all (#1334) Do you do anything special? You probably don’t use React node views, right?

@ryanto I must say that I’m not that familiar with React. Can you explain to me why this error happens with your change? If there is no other option, I would be ok with an availableOnFirstRender option. Could you create a PR for it? 😬🙏

[vpontis]

@vpontis I always thought that tiptap doesn’t work with SSR at all (#1334) Do you do anything special? You probably don’t use React node views, right?

Hmm, we do use React Node views and it does work with SSR.

We do some smart stuff like not render toolbars if editor == null

CleanShot.2021-12-16.at.11.54.56.mp4

[philippkuehn]

Should we update the types:

export declare const useEditor: (options?: Partial<EditorOptions>, deps?: DependencyList) => Editor | null;

to not specify the null option?

@vpontis Strange. I don’t see this null. The types are already updated correctly.

Hmm, we do use React Node views and it does work with SSR.

Good to know. Need to test this.

[vpontis]

Oh @philippkuehn I could have been looking at an old version of the types.

Lmk if there's anything w/ SSR that I can help w/ since we rely on it and it works well!

[ryanto]

Yup, I'll absolutely PR. Will have it to you in an hour or so.

So tiptap doesn't render on the server since it references document, which isn't defined in Node.js (SSR uses Node to do server side rendering). When node encounters a document it thinks your code is referencing an undefined variable and throws throws an error. The screenshot @vpontis posted is the classic example of this :)

In React, useEffect does not run on the server. So previously when the editor was being created inside a useEffect it was never running on the server. The editor was null on the server, null on the first render, and then the useEffect would create it and it would become available on the second render.

Here's some pseudo code to help explain.

function Component() {
  let [name, setName] = useState("");
  useEffect(() => {
    setName("Alice");
  });

  return <p>hi {name}!</p>
});

1. Server render: "hi !"
2. Initial browser render "hi !"
3. useEffects now run
4. useEffect sets name and triggers a re-render
5. Second render: "hi Alice!"

One rule of react is that the server render must match the first browser render, so that react can re-hydrate the page's HTML correctly.

Let me know if that clears it up! Happy to jump on a chat too, React+SSR+Initial render can absolutely be confusing.

[ryanto]

Oh also, I think long term making tiptap run on the server by generating html, but not setting up any of the browser APIs/events/conenteditable (as pointed out in #1334) is a great idea! That way you can do something like....

function Component() {
  // if were on the server, or the first render, opt of editing APIs on the first render
  let [isEditable, setIsEditable] = useState(false);
  let [editor, setEditor] = useState(() => new Editor());

  useEffect(() => {
    // this will only run in the browser, so we can safely now opt into editing
    setIsEditable(true);
  });

  // Editor can render an HTML only version or WYSIWYG editor with all the browser APIs based on isEditable.
  return <Editor editor={editor} isEditable={isEditable} />
});

However, removing all of the calls to document and having a SSR compatible version of Editor() seems like it could be a bigger task. I don't know enough about tiptap to say how easy/hard it would be.

[philippkuehn]

@ryanto oh thanks, got it! 👍

[philippkuehn]

Oh also, I think long term making tiptap run on the server by generating html, but not setting up any of the browser APIs/events/conenteditable (as pointed out in #1334) is a great idea! That way you can do something like....

function Component() {
  // if were on the server, or the first render, opt of editing APIs on the first render
  let [isEditable, setIsEditable] = useState(false);
  let [editor, setEditor] = useState(() => new Editor());

  useEffect(() => {
    // this will only run in the browser, so we can safely now opt into editing
    setIsEditable(true);
  });

  // Editor can render an HTML only version or WYSIWYG editor with all the browser APIs based on isEditable.
  return <Editor editor={editor} isEditable={isEditable} />
});

However, removing all of the calls to document and having a SSR compatible version of Editor() seems like it could be a bigger task. I don't know enough about tiptap to say how easy/hard it would be.

Sounds great. But I think we should try to tackle that after stable 2.0!

[ryanto]

Yes absolutely, I'll do a quickfix PR for now.

[ryanto]

Hey @philippkuehn Almost ready for a PR! Got everything working, but I'd like to write a test for this just to make sure it renders.

I'm familiar with Cypress, is there anything I need to know about the testing setup for tiptap? Do I need to create a new React app under demos/src/???. Let me know how you would test this and I'll get it all setup :)

Thanks!

[philippkuehn]

Great! 👍

You can add it here for now:
/demos/src/Examples/Default/React/index.spec.js

Or somewhere under /tests/.

I can also move this around later. We need to find a better structure for tests at some time 🙃

[ryanto]

#2287 is ready if you guys want to review.

[cusxio]

@vpontis ,

Lmk if there's anything w/ SSR that I can help w/ since we rely on it and it works well!

How did you get SSR to work with React for TipTap? Currently I'm running generateHTML on the server side, and mount TipTap on the client side.

Did you achieve SSR by extending the extension such as Document, Paragraph, and Text with addNodeView to render React components?

[vpontis]

@cusxio oh gosh, I think I may be confused. I tested it here and you are right that SSR isn't working.

But SSR seems to be working locally. I'm not sure why...

[cusxio]

@vpontis, no worries! I don't think SSR will ever work with TipTap because the internals relies on createElement("div").

[hanspagel]

@cusxio it does, it’s just not part of the core: https://tiptap.dev/api/utilities/html

[cusxio]

This PR can actually work with a slight adjustment to handle the server,

- const [editor, setEditor] = useState<Editor>(() => new Editor(options))
+ const [editor, setEditor] = useState<Editor | null>(
+   typeof window === "undefined" ? null : () => new Editor(options)
+ );

[hanspagel]

According to this comment it wouldn’t:

#2287 (comment)

[cusxio]

@hanspagel, but we are not solving for SSR in this PR.

We are solving first render issue.

The first render issue currently persist in the latest tiptap build.

This PR technically fixes that, but this PR does not work on the server (because there's no document on the server). Hence it was reverted.

The fix I propose above, makes it so that this PR does not crash on the server, but can still work on the client.

[rfgamaral]

Just wanted to provide feedback that I would love to see @cusxio proposal back into Tiptap, to avoid having to have a custom useEditor on our side.

[philippkuehn]

@ryanto any thoughts about this?

[rfgamaral]

Actually, looking more closely, I don't think @cusxio is the right solution because the return type still includes null, but the instance returned by useEditor on a browser would never return null with the code above.

I haven't put much thought into this, but maybe 2 React hooks would be the most optional solution, one for each scenario. I don't know, just thinking out loud.

[ryanto]

@philippkuehn I believe it will cause the client/server mismatch warning. Everything will render fine, but there will be an error/warning in the console. However, I think seeing a proof-of-concept to verify this would be good here.

On that note, it might be good to get a test suite setup to test all the scenarios we're uncovering in this PR.

There's lots were discussing here with SSR, CSR first render, client/server matching html, new Editor on server, etc. I think codifying each of the behaviors we want into tests so we can show which of these suggestions will and won't work is best. I'm more than happy to help with that after the holidays, I might need some guidance from y'all because I'm not too familiar with vite.

Have a great holiday everyone! :D

[benjie]

Sorry to necro this; not sure where else to comment. It seems to me that this isn't doing quite the right thing for React/TipTap integration - should we really be recreating a new editor every time deps changes (e.g. an extension gets passed a new callback)? Also expecting the user to provide a deps array when the "rules of hooks" ESLint plugin isn't automatically picking up their mistakes seems a recipe for you having a lot of support tickets 😉

I wonder if this might be a better approach - remove deps completely, and use editor.setOptions rather than recreating the editor each time:

const useTipTapEditor = (options: Partial<EditorOptions> = {}): Editor | null => {
  const editorRef = useRef<Editor | null>(null);
  const forceUpdate = useForceUpdate();

  if (!editorRef.current && typeof window !== "undefined") {
    // Created editor on initial browser render
    editorRef.current = new Editor(options);
    editorRef.current.on("transaction", () => {
      requestAnimationFrame(() => {
        requestAnimationFrame(() => {
          forceUpdate();
        });
      });
    });
  } else if (!editorRef.current) {
    // Server; ignore
  } else if (editorRef.current.isDestroyed) {
    // Attempted to update options after editor was destroyed; this shouldn't occur.
  } else {
    editorRef.current.setOptions(options);
  }

  // Destroy editor on unmount
  useEffect(() => {
    return () => {
      editorRef.current?.destroy();
    };
  }, []);

  return editorRef.current;
};

This will result in a different result on first render on browser versus SSR, but it doesn't render something different, so that can be handled elsewhere.

I am extremely unfamiliar with tiptap so I don't know if the use of setOptions like this is equivalent?

[benjie]

Oh, also your useForceUpdate should memoize the callback so it doesn't differ on every call:

function useForceUpdate() {
  const [, setValue] = useState(0);
  return useCallback(() => setValue((value) => value + 1), []);
}

[rfgamaral]

@benjie Are you using that useForceUpdate hook with the useCallback yourself? What, exactly, is the difference supposed to be?

I did some testing, and I can't see any difference with or without it. Just trying to understand if there's a real benefit in adding that useCallback or if this is one of those cases where we shouldn't actually be using it (ref).

[benjie]

It's beyond my knowledge if that's required or not, it's just a copy from the existing implementation. I imagine it is probably required to synchronise editor state with React state, without it the editor might still work but React might "lag behind" state-wise.

[dilizarov]

Hmm. I think it is important to denote two separate issues here.

1. SSR
2. First render within the browser

While both are often related, they're not always. In my case, I'm focused on at least having the first render work kindly.

I tried leveraging generateHTML(initialContent, extensions) as the docs here suggest, but I don't think that function actually works on the server. It seems to require window. Am I doing something wrong there?

I've gotten to something that resembles this:

const Editor: React.FC = () => {
  const [initialRender, setInitialRender] = useState<string>(HTML_STRING)

  const editor = useEditor(...);

  return editor ? (
    <EditorContent editor={editor} />
  ) : (
    <div className="ProseMirror" dangerouslySetInnerHTML={{ __html: initialRender }} />
  )
}

As you can imagine... the important tidbit is getting HTML_STRING.

Now, of course useState(generateHTML(initialContent, extensions)) would be ideal, but again on the server this won't work because we don't have window.

I tried leveraging useLayoutEffect like:

useLayoutEffect(() => {
  if (initialContent) {
    // use generateHTML here
  }
}, [])

That works, but of course defeats the purpose because I still see the jumpy render situation.

I'm curious if it doesn't matter how quickly I get the window object... we'll always have this jumpy issue.

That leads me to my current solution:

useEditor({
  onUpdate({ editor }) {
    // persist editor.getJSON AND editor.getHTML to db on server.
  },
  ...
})

At least this way in my case the only thing I now have to fake is an empty editor if there is no data to preload into it, which is as simple as setting up a div with appropriate height, etc.

[dilizarov]

Actually, my solution might not work for custom made nodes.

For example, if you have a node named "enhancedImage", and the tag it renders is enhanced-image, then via generateHTML you get something like "..." which only the Editor can understand...

Back to the drawing board.

[stevecastaneda]

@dilizarov Your solution works, and you're right about custom components not working as intended. I think I found a work around.

When creating your custom component extension, instead of giving it a non-standard HTML tag like react-component, give it: div[data-react-component]. Then, when you renderHTML, add that as an attribute so that it can find the component and parse it.

Now here's the trick. If you want animation libraries to work, you'll need to manually set the height on these components as they're not actually rendered , but the HTML will carry over with a style tag explicitly defining the height. This works for my use case as the custom component will always be 28px.

parseHTML() {
    return [
      {
        tag: "div[data-react-component]",
      },
    ];
  },
  renderHTML({ HTMLAttributes }) {
    return ["div", mergeAttributes(HTMLAttributes, { "data-react-component": "", "style": "height: 28px;" })];
  },

This, along with using dangerouslySetInnerHTML allows the component to have the correct height on the first render, which libraries like framer motion rely on to animate correctly.

Can anyone identify any major issues with this approach?

[RealAlphabet]

Does this work for Vue? Because useEditor always returns undefined.