yjs / y-prosemirror

ProseMirror editor binding for Yjs
https://demos.yjs.dev/prosemirror/prosemirror.html
MIT License
335 stars 116 forks source link

Change Y.XmlElement instanceof check to constructor.name #67

Closed TeemuKoivisto closed 3 years ago

TeemuKoivisto commented 3 years ago

In local development, at times you get multiple instances of the same library by npm linking modules back and forth. Using instanceof with two Yjs libraries loaded fails which would seem unnecessary or at least should be handled somewhere else.

Related https://discuss.yjs.dev/t/prosemirror-build-variations-and-yjs-shared-cursor/228/3

dmonad commented 3 years ago

Comparing the constructors is fairly ingrained into Yjs. This is by far the most performant way to compare types and is therefore preferred over constructor.name which basically does a string comparison. This would seriously downgrade performance when we have documents with over a million Structs (I need to distinguish between different kinds of content very often).

There might be a more performant way to implement what you are asking for. However, I'm fairly sure that any approach will hit performance significantly.

I don't want to spend this amount of time when I actually think that the fault is with the user of this library. There is no good reason to have two versions of Yjs installed. I understand that tooling often makes it difficult to prevent having the same package installed multiple times. But then again.. maybe the tooling is wrong for enforcing such behavior. This is part of the reason why web applications nowadays are so bloated.

If you submit a PR that solves this issue and doesn't reduce performance in crdt-benchmarks then I'd be happy to merge it.