[feature request] Add support for toObject in the from function about orderedmap

[marijnh]

Where does this toObject convention come from?

[Tezraine]

I would say it's an implied convention. Generally, "Object" refers to an unstructured data blob, and the existence of a 0 argument toObject function implies that this thing has a proper "Object" form, but it's current form is not it. I can't think of any situation where an object has a 0 arg toObject function, and it's current form would be useful in the OrderedMap.

For example, compatibility with this library could be achieved toObject() method - Collections for JavaScript (collectionsjs.com) if you checked for this behavior for compatibility.

Also basing my thoughts on how Promise libraries generally identify something as a promise. InstanceOf is not a reliable way of checking for a behavior, because different copies/implementations will return false even though they are still compatible.

On a side note, my use case actually stems from starting to learn Prosemirror. This change would not fix the the "double import of prosemirror-model", but it would reduce the severity enough for the doc to still render in read only mode. So I get a "you might have more then one prosemirror-model" error instead of a "missing doc topNode" error. And I would not need to flag orderedmap as a force-share library.

[marijnh]

Sorry, that doesn't sound like a very compelling reason. You'll have other issues with ProseMirror when loading libraries multiple times.

[Tezraine]

The Prosemirror case was just a side note of another library that would get a usability benefit from this change. My main issue is that the from function states If given an ordered map, return that map itself. However, thanks to the subtleties of instanceof and modules and instantiation, under less then perfect circumstances, you will get an ordered map of the the orderedmap instead of getting that orderedmap back because instanceof only returns true if it was created with that specific prototype instance.

I consider that a bug, but the toObject feature would fix that bug in a type safe way, as well as add compatibility with other collections with toObject functions, not just the very specific instance it was created from.

If you like, I can create a separate issue for specifically discussing that bug of instanceOf returning false after an orderedmap instance is cloned.

[marijnh]

If you like, I can create a separate issue for specifically discussing that bug of instanceOf returning false after an orderedmap instance is cloned.

I don't know how you're cloning the instance, but if you use OrderedMap.from then instanceof will continue to return true.

[Tezraine]

I don't know where the clone is coming from. The package-lock only lists 1 orderedmap installed, but instanceof is still returning false. I think one of the commonjs packages I have installed bundled it's own copy of orderedmap into itself, which instanceof returns false against causing everything to die horribly. And grep gets too many results. Right now my only solution looks like starting over from scratch and implementing all the plugins and header myself. (Maybe start over from TipTap)