I’m implementing a custom NodeView, and trying to make sure I clean up resources nicely in destroy().
The NodeView also implements update(), and returns false for cases in which we can’t reuse the existing structure (e.g.: the node type has changed).
However, when we return false, I was under the assumption that a new NodeView will be created and that the existing one should be cleaned up. However, I noticed that destroy on my existing NodeView is not called in this case.
Is this a bug in PM? Or should I manually cleanup (i.e. call destroy) when my update function returns false? Or am I misunderstanding something else?
destroy should definitely be called when a node view isn’t reused in a DOM update. I can’t reproduce the situation where it isn’t. Can you set up a script that demonstrates this issue?
Ah, seems like code sandbox doesn’t show all the previous ProseMirror versions - to clarify reverting prosemirror-model to 1.19.0 and prosemirror-view to 1.30.2 fixed the issue.
Hi, I’m working with @YousefED on this issue, and I think I’ve figured out the issue. I’ve managed to set up a sandbox which replicates the problem:
The editor has a simple schema with basically just doc, paragraph, and 'text nodes. I also added a nodeViewParagraph which is the same as the regular paragraph, but it gets rendered using a node view and logs when update() or destroy() are called. update() returns false if the node’s type has changed, and true otherwise.
Below the editor are 2 buttons, one to delete the first paragraph in the editor and another to toggle it between a nodeViewParagraph and regular paragraph using setNodeMarkup.
If I press the toggle button and type something in the first paragraph, update() is called as expected and returns true. Pressing the delete button also calls update(), which now returns false as the node has been replaced with a regular paragraph, and destroy() is called as expected.
However, pressing the toggle button instead to revert it to a normal paragraph calls update() but not destroy(), despite returning false. Reverting prosemirror-model and prosemirror-view back one version fixes this, and destroy() is called as it should be, so this seems like a bug that which popped up in the last release.
