ProseMirror / prosemirror

The ProseMirror WYSIWYM editor
http://prosemirror.net/
MIT License
7.59k stars 335 forks source link

DecorationSet.create unexpectedly modifies input argument array to empty it #1414

Closed Nantris closed 1 year ago

Nantris commented 1 year ago

I have some code like this:

const decorations = findDecorations(
  tr.doc,
  mappedPrevDecorations
);
// `decorations` array has 2 items which appear valid to me
const newDecorationSet = DecorationSet.create(
  tr.doc,
  decorations
);
// `decorations` array is empty

Whether my input decorations are valid or not (they may not be as they overlap) - I would never expect for decorations to be emptied out by this code, but it is.

It appears to occur because of these lines: https://github.com/ProseMirror/prosemirror-view/blob/db2223a88b540a8f381fc2720198342e29a60566/src/decoration.ts#L677-L682

Nantris commented 1 year ago

My suggestion is that the array be cloned before being used in this function: https://github.com/ProseMirror/prosemirror-view/blob/db2223a88b540a8f381fc2720198342e29a60566/src/decoration.ts#L689 @marijnh

marijnh commented 1 year ago

I've added a note warning about this to the docs in attached patch. But since it is not typical for code calling this function to use the array passed to it for anything else, enforcing a copy seems needlessly wasteful.

Nantris commented 1 year ago

Seems fair. Thanks for the speed response @marijnh!