ProseMirror / prosemirror

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

Node.toJSON() returns object with shallow `attr` property #1474

Closed yykcool closed 2 weeks ago

yykcool commented 2 weeks ago

I built a function to clone and recursively purge the id attribute of any nodes in a slice.

export const cloneAndPurgeIds = (
  slice: Slice,
  schema: Schema<string, string>
) => {
  const sliceJson: JSONContent = slice.toJSON();
  console.log(JSON.stringify(sliceJson));
  const sliceJson2: JSONContent = slice.toJSON();
  recursiveReassignId(sliceJson);
  console.log(sliceJson, sliceJson2, isEqual(sliceJson, sliceJson2));
  console.log(JSON.stringify(sliceJson2));

  return Slice.fromJSON(schema, sliceJson);
};

const recursiveReassignId = (node: JSONContent) => {
  if (node.attrs?.["id"]) {
    delete node.attrs["id"];
  }
  node.content?.forEach((n) => recursiveReassignId(n));
};

What i expected: nodes in sliceJson2 to still contain id attributes with string values

What i got: both sliceJson and sliceJson2 prints the slice with all id attributes purged, and isEqual() returning true.

After some digging, it seems that Node.toJson() populates the json object with a shallow reference of the node's attrs object.

https://github.com/ProseMirror/prosemirror-model/blob/751134cc35481fa69ae8f9215ea3653873c8eea1/src/node.ts#L310

Not sure this is intended behaviour but bringing it up just in case.

yykcool commented 2 weeks ago

telegram-cloud-photo-size-5-6332290814339825983-y

marijnh commented 2 weeks ago

Since attribute objects are JSON-serializeable as they are, they are not cloned by toJSON. As such, you shouldn't mutate the objects it returns. Copy them and then mutate the result.