-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
onUpdate callback does not update after re-render #2403
Comments
ok, it seems like I've been using the Just as a comment, the examples on the documentation page should mention this more clearly Anyways, sorry for the confusion and thanks for the package! |
Facing new issues now, here is the updated version of my component: import Link from "@tiptap/extension-link"
import Placeholder from "@tiptap/extension-placeholder"
import TaskItem from "@tiptap/extension-task-item"
import TaskList from "@tiptap/extension-task-list"
import { EditorContent, EditorEvents, Extension } from "@tiptap/react"
import StarterKit from "@tiptap/starter-kit"
import React, { FC, memo, useState } from "react"
import { useBoolean } from "../hooks/useBoolean"
import { useEditor } from "../hooks/useEditor"
interface IProps {
editable?: boolean
// onClick?: (this: unknown, view: any, pos: number, event: MouseEvent) => boolean
content?: any
onUpdate?: (params: EditorEvents["update"]) => void
autofocus?: boolean | null | "end" | "start"
onFocus?: (params: EditorEvents["focus"]) => void
placeholder?: string
className?: string
}
export const Tiptap: FC<IProps> = memo(
({ editable = true, content, onUpdate, autofocus, onFocus, placeholder, className }) => {
console.warn("tiptap render content", content)
const [isAddingLink, addLinkOn, addLinkOff] = useBoolean()
const [link, setLink] = useState("")
const editor = useEditor(
{
autofocus,
// onFocus: onFocus ? onFocus : () => {},
editorProps: {
attributes: {
class: "prose focus:outline-none dark:prose-dark dark:text-gray-300 text-base",
},
},
content,
onUpdate: onUpdate ? onUpdate : () => {},
extensions: [
StarterKit,
Placeholder.configure({
showOnlyWhenEditable: false,
placeholder,
}),
TaskList.configure({
HTMLAttributes: {
class: "pl-0",
},
}),
TaskItem.configure({
HTMLAttributes: {
class: "before:hidden pl-0 flex items-center dumb-prose-remove",
},
}),
Extension.create({
// Do not insert line break when pressing CMD+Enter
// Most of the time handled by upper components
addKeyboardShortcuts() {
return {
"Cmd-Enter"() {
return true
},
"Ctrl-Enter"() {
return true
},
}
},
}),
Link,
],
},
[onUpdate, content]
)
return (
<div>
<EditorContent editor={editor} className={className} />
</div>
)
}
)
All in all, these seem major issues with tiptap, never thought I would face so many problems by trying to attach a simple onUpdate handler |
I got it working by using refs, which is very inelegant and makes the code quite hard to understand: const [updateProjectMutation] = useMutation(updateProjectNotes)
const tiptapOnUpdateRef = useRef(async ({ editor }: EditorEvents["update"]) => {
await updateProjectMutation({
id,
notes: JSON.stringify(editor.getJSON()),
})
refetch()
})
useEffect(() => {
if (project) {
const editor: Editor = tiptapRef.current?.getEditorInstance()
if (editor && project.notes) {
editor.commands.clearContent() // Needed because the checkboxes do not update
editor.commands.setContent(JSON.parse(project.notes))
tiptapOnUpdateRef.current = async ({ editor }) => {
await updateProjectMutation({
id,
notes: JSON.stringify(editor.getJSON()),
})
refetch()
}
}
}
}, [id, project]) |
I just ran into the same issue, it caught me completely off guard. @ospfranco I've found this seems to work too. Not sure if it's the right / best way to use the
|
@colindb I assumed |
I am not sure I understand the problem correctly. Can you set up an absolutely minimal codesandbox for this? |
|
Hmm if you mean if callback should get updated, this has nothing to do with the node_modules, I already tried updating the latest version, but it is rather a problem of how the hook + editor instance works The hook takes an array of dependencies to re-render the editor, in order for the callbacks to be updated one has to declare them in the dependency array of the hook, but this causes other issues, basically, the internal instance of the editor gets destroyed and re-created, this causes flickering issues and what not The workarounds we found, are basically hard replacing the saved callbacks on the first instantiation of the hook, but it's not ideal because: a) it's not documented (I think) and b) it's just really awkward and hard to understand |
Hi @philippkuehn I have the similar issue. Steps to reproduce:
Looking into core |
I have a similar problem where I created an extension that takes a function in ( |
Hey everyone 👋, I just want to share how we are handling this ourselves, and this is something we did from the beginning and never noticed any issues:
And that's pretty much it... We don't seem to observe any flickers, every callback seems to work as expected, and everything is updated when we need it. |
I faced the same issue and created a codesandbox for it. Expected behavior: be able to continue selecting while This seems to be a working workaround for now:
edit: I tested out https://codesandbox.io/s/gifted-river-1v58kf?file=/src/App.tsx |
Your missing the dependencies array in the Don't forget to wrap |
I didn't get what you mean, I'm using I didn't understand where the |
This was very hard to find. Please add this to the quickStart docs? |
Any updates on this issue? I have the same problem. I cannot use closures inside Tiptap extensions. They are always stale |
For whatever reason, I completely did NOT understand the first 5 times I read this. The answer makes me feel stupid for wasting days. I completely neglected to realize this is a hook, just like useEffect. Use the dependency array.
|
@rfgamaral - would you mind providing a small amount of sample code to show you you can use an external function inside of the onUpdate call? Specifically elaborating on this?
When call the effect useEditor, I define the code I want to call in that callback declaration. But when I call other functions inside of onUpdate, those functions have stale data, which I assume means the function itself is stale, but I don't know. My editor looks like this
Also,
Moving the onUpdate part out entirely like this may actually be working with focus remaining and no flickering...
Last one hopefully, I think I finally understood the gentlemen's comment from above. This is the
|
Any update on this? It seems to still be the case as of today. |
Here is a demo of how I got callbacks working: https://codesandbox.io/s/upbeat-hermann-d3dlw5?file=/src/App.js:1634-1670 |
@joe-pomelo Thanks! That's what I did as well. Would be great if I could use the default onUpdate method for this tho. |
I'm also struggling with this. I have a setup similar to the ones above, with a wrapper passing dynamic content down to the RichTextEditor and handling updates. The problem is when content changes, I call the onChange through the onUpdate, it updates Firestore, comes back and causes the editor to lose focus, so I can't continue typing. I could call onChange through the onBlur but I don't want users to lose data if they type and close the tab without blurring. Here is my code for reference:
And the RichEditor instance:
|
I'm struggling so much with this 😭 |
I've had the same issue with a stale onUpdate function, but I managed to fix it with this:
So far it seems to work as I want it to :) |
Did this work for you? Passing in So I need to 1. pass My code:
|
I had exactly the same problem with the TipTap editor. I wanted to create a generic TextEditor component, which I in turn use from several other components. From these other components I wanted to always be able to retrieve the state of the TextEditor component (e.g. to provide adhoc validation), but as has already been written here, the updates within the editor instance were not correctly propagated out to the parent. I have now found another solution for this. In doing so, I still have a generic TextEditor component, but I exported a method outside the rendering function that returns the useEditor hook and at the same time uses the extensions and options I need. I then have to use this extracted method in my other components and can then use my generic TextEditor component at the same time and have the ability to check the current state at any time. TextEditor.tsx:
EditorContainer.tsx:
here is my working codesandbox: https://codesandbox.io/s/tiptap-editor-state-between-child-and-parent-component-9m0bkh |
@mclazer I really like your approach! I've tried replicating it and although I had some success for the most part I'm finding it difficult to update the contents of the editor; I want to change the text editor when necessary. For example, I initialize |
@RipaltaOriol so you have different components for each row and the initialization of the editor is in row 0? Or do you just have 1 component and different rows and set the text initially to row0 and want to be able to click other rows and update the text accordingly. Could you provide a codesandbox with the code? |
@mclazer its the latter. I solved the problem thanks to you sandbox. I was importing the |
This issues persists inside the addKeyboardShortcuts(). I'm trying to submit data on the click of the Enter button. |
@iandsouza2000 Yes it does indeed persist. I am also trying to submit data on Enter. This seems like an extremely common use case. |
This isn't ideal either, but is probably better than the editor continuously being recreated, blurred, and then putting the old content into the new editor like is done in #2403 (comment)
This way, every time the Ideally, I would like |
I need to have a single source of truth for my content outside of Tiptap, especially because I have my own history system for undo/redo. With Tiptap not reacting to I somehow manage to get Tiptap to react to external const RichTextEditor = ({ value, onChange }) => {
const cursor = useRef<number>();
const editor = useEditor({
content: value,
parseOptions: { preserveWhitespace: "full" },
onUpdate: ({ editor }) => {
onChange(editor.getHTML());
},
onSelectionUpdate: ({ editor }) => {
cursor.current = editor.state.selection.anchor;
},
});
useEffect(() => {
if (cursor.current === undefined) return;
editor?
.chain()
.setContent(value, false, { preserveWhitespace: "full" })
.setTextSelection(cursor.current)
.run();
}, [value, editor])
return <EditorContent editor={editor} />;
}; Basically, we track the cursor position so that we can return it later. Since we're using Note You can get a similar effect by using If the "cursor restoration" is not to your liking, you can modify On another note, I think Tiptap is a great library with the best DX I've seen so far for building rich text editors. But the lack of first-party controlled state support is almost a turn-off for me. I hope we can see a better solution soon, because what I have here is really, really, really hacky 💀 |
This should be resolved with tiptap 2.5 |
While Tiptap 2.5 is may be solving the original problem, it still (as of Tiptap 2.5.7) does not solve the issue described above (#2403 (comment)), and the suggested workaround there remains to be required to maintain the state and make Tiptap not lose focus and not forget the caret placement. Is the fix for this specific issue still in the plans? Or is there a better way to work with Tiptap as a truly reactive component? |
This discussion wildly deviated from the original issue which is, in fact, resolved with Tiptap 2.5. The problem with treating Tiptap as a "controlled component" is that it fundamentally is not how contenteditable & prosemirror work. Rich text editing is not easy, this is why you are leveraging a library like Tiptap to deal with that fact, it is a very different model from how React works with it's ideals. Even in React, they spent a lot of effort to get controlled & uncontrolled behaviors out of normal inputs & textareas, the difference being they have hundreds of engineers and we are just 3 guys. If anyone has ideas on how to do this better, I would be happy to hear about it, but this really should be a separate discussion. My idea for how we could achieve something like this is to have a different API, where you can specify As mentioned by #2403 (comment)
This actually should be much more performant now with the Tiptap 2.5 update, so if you want to try it out please do. |
On that note... I absolutely love Tiptap. Thank you for all the work you all are doing! |
Thank you to everyone who posted solutions. I encountered the same issue and tried all the solutions provided, but unfortunately, none worked for me. After some investigation, I was able to resolve the problem with the following approach: const editor = useEditor({
content: value,
editorProps: {
attributes: {
class: 'px-3 h-40',
},
},
extensions: [StarterKit, Underline, FontFamily, TextStyle],
onFocus: ({ event }) => {
event.preventDefault();
event.stopPropagation();
},
onUpdate: ({ editor }) => {
const html = editor.getHTML();
if (!html || editor.isEmpty) return;
onChange(html);
},
});
useEffect(() => {
if (!editor) return;
editor.commands.setContent(value || '');
}, [value, editor]); The key part of the fix that solved the bug for me was this: useEffect(() => {
if (!editor) return;
editor.commands.setContent(value || '');
}, [value, editor]); By ensuring the editor's content is updated when |
For those saving the value on local storage you can also add the "onCreate" Event handler so sync on load. |
I cannot stress it enough the controlled component pattern is THE WRONG WAY TO USE TIPTAP. Another solution around this is to use the useEditorState hook which can derive state from the editor instance. If the parent of the editor needs this, you can pass the editor instance in a callback or a ref. There are other solutions to this, controlled components is the least performant and most hacky since it goes against the grain of how normal JS works, forcing the editor to reinstantiate because you felt the state should live in a useState instead |
This worked for me without any focus issues. import { isNil } from "lodash-es";
useEffect(() => {
if (!editor || isNil(value) || value === editor.getHTML()) return;
editor.commands.setContent(value || "");
}, [value, editor]);
|
What’s the bug you are facing?
I have a wrapper around tiptap, whenever the text changes I trigger a request to my back-end, this works fine the first time tiptap is mounted but the parent component (where tiptap is mounted) can change its internal variables, and therefore the closure should capture a new context, the problem is that it doesn't after the parent component changes the state, the closure/lambda passed on the
onUpdate
function remains the same and therefore tiptap tries to update the wrong component.Here is some of the code, my high level component on the parent, notice the
id
param, which is the param that changes at some point:My internal TIptap implementation, notice the
onUpdate
function that I'm passing to theuseEditor
hook:In any case, it seems the useEditor hook saves only the first passed
onUpdate
function and does not update it in sub-sequent rendersHow can we reproduce the bug on our side?
Attached the code above, but if necessary I can try to reproduce the issue in a code sandbox
Can you provide a CodeSandbox?
No response
What did you expect to happen?
The passed callback
onUpdate
should be updated when a new value is passed to it, instead of constantly re-using the first memoized valueAnything to add? (optional)
I tried to update tiptap to the latest version but then I faced this other crash: #577 so I reverted to my old/current versions
Did you update your dependencies?
Are you sponsoring us?
The text was updated successfully, but these errors were encountered: