[Bug]: React renderer is causing update for each nodeview when unique-id extension is used

[bdbch]

Which packages did you experience the bug in?

react, extension/unique-id

What Tiptap version are you using?

2.1.13

What’s the bug you are facing?

This issue was reported on our Discord by Bartlomiej.
https://discord.com/channels/818568566479257641/818569721934774272/1184601997912068167

Basically because the node view renderer will cause a new transaction when it's attributes change and the unique-id extension will add ids to all nodes that don't have one will cause tons of rerenders on all nodes because we flushSync inbetween each update.

I think Bartlomiej already worked on a fix for this as discussed in this message:
https://discord.com/channels/818568566479257641/818569721934774272/1184609761061257236

I'll ask him to open a PR for this issue if his fix is working.

What browser are you using?

Chrome

Code example

No response

What did you expect to happen?

Don't cause 400 rerenders because of 400 node views.

Anything to add? (optional)

No response

Did you update your dependencies?

• Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

• Yes, I’m a sponsor. 💖

[bartlomiej-korpus]

This is relevant here too: #4492

I've prepared an awfully over-engineered solution that is able to preserve 100% of the current behaviour(as far as I can tell) of setRenderer and removeRenderer while avoiding any excessive rerenders. There must be an easier way to achieve this :D

Though, I am not sure whether this is needed. I feel like better solution would be to batch those updates somewhere upstream, perhaps directly before or after applying new state to the editor view? I tried playing with it, but I lack expertise here and I am not sure if it wouldn't require changes at ProseMirror's ViewDesc level to avoid selection/flickering bugs.

Check this out bartlomiej-korpus#1

I can confirm this works and doesn't have a performance penalty of previous solution while working correctly with flushSync. It has one quirk though which I did not cover(it's not a problem in our case but it might be for someone else, I am open to improve upon this), namely: the array inside holdersStatesRef (which is the new incarnation of old renderers state) never shrinks, if a renderer is removed its replaced with a blank space which will be reused for the next setRenderer.

Lets discuss how to make it better from here

[jelling]

Dumb question but does this problem go away if the unique id set on storage instead of as an attribute?

At least in my case, I only want the id to pass to other non-tiptap components. Although, storage doesn't work with TipTap cloud, if I understand correctly.

[bartlomiej-korpus]

Dumb question but does this problem go away if the unique id set on storage instead of as an attribute?

At least in my case, I only want the id to pass to other non-tiptap components. Although, storage doesn't work with TipTap cloud, if I understand correctly.

Storage is only per plugin, not per node instance, right? I'm not sure how that would work, but I'm also not very experienced with both PM and TipTap.

By the way

What did you expect to happen?
Don't cause 400 rerenders because of 400 node views.

This part here is not exactly right, document with 400 react-rendered node views will see 160_000 rerenders during unique-id onCreate, as all of the 400 node views will be rendered for each setNodeMarkup called(so for every node view). This isn't exactly a problem with unique-id, more with react EditorContent itself and this is what i tried to address.

My fix has still one outstanding issue which I'm going to resolve this week

[nperez0111]

This will be resolved by #5273

[nperez0111]

This is now resolved with 2.6.0