Merge pull request #74 from nytimes/key-plugin-handle-new-node

Map positions forward to maintain key stability

.vscode/settings.json

@@ -7,18 +7,11 @@
   "typescript.enablePromptUseWorkspaceTsdk": true,
   "prettier.prettierPath": ".yarn/sdks/prettier/index.js",
   "eslint.nodePath": ".yarn/sdks",
-  "editor.formatOnSave": true,
   "[javascript]": {
-    "editor.defaultFormatter": "esbenp.prettier-vscode",
-    "editor.codeActionsOnSave": {
-      "source.fixAll": true
-    }
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
   },
   "[typescript]": {
-    "editor.defaultFormatter": "esbenp.prettier-vscode",
-    "editor.codeActionsOnSave": {
-      "source.fixAll": true
-    }
+    "editor.defaultFormatter": "esbenp.prettier-vscode"
   },
   "[markdown]": {
     "editor.defaultFormatter": "esbenp.prettier-vscode"

.yarn/versions/1f4b945b.yml

@@ -0,0 +1,2 @@
+releases:
+  "@nytimes/react-prosemirror": patch

package.json

@@ -71,6 +71,7 @@
     "prosemirror-model": "^1.18.3",
     "prosemirror-schema-list": "^1.2.2",
     "prosemirror-state": "^1.4.2",
+    "prosemirror-transform": "^1.8.0",
     "prosemirror-view": "^1.29.1",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",

src/plugins/__tests__/react.test.ts

@@ -1,6 +1,7 @@
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 import { Schema } from "prosemirror-model";
 import { EditorState } from "prosemirror-state";
+import { findWrapping } from "prosemirror-transform";
 
 import { react, reactPluginKey } from "../react.js";
 
@@ -9,7 +10,7 @@ const schema = new Schema({
     doc: { content: "block+" },
     paragraph: { group: "block", content: "inline*" },
     list: { group: "block", content: "list_item+" },
-    list_item: { content: "inline*" },
+    list_item: { content: "paragraph+" },
     text: { group: "inline" },
   },
 });
@@ -46,8 +47,8 @@ describe("reactNodeViewPlugin", () => {
     );
     const nextPluginState = reactPluginKey.getState(nextEditorState)!;
 
-    expect(Array.from(initialPluginState.keyToPos.keys())).toEqual(
-      Array.from(nextPluginState.keyToPos.keys())
+    expect(Array.from(nextPluginState.keyToPos.keys())).toEqual(
+      Array.from(initialPluginState.keyToPos.keys())
     );
   });
 
@@ -69,10 +70,78 @@ describe("reactNodeViewPlugin", () => {
     const nextPluginState = reactPluginKey.getState(nextEditorState)!;
 
     // Adds new keys for new nodes
-    expect(nextPluginState.keyToPos.size).toBe(5);
+    expect(nextPluginState.keyToPos.size).toBe(6);
     // Maintains keys for previous nodes that are still there
     Array.from(initialPluginState.keyToPos.keys()).forEach((key) => {
       expect(Array.from(nextPluginState.keyToPos.keys())).toContain(key);
     });
   });
+
+  it("should maintain key stability when splitting a node", () => {
+    const initialEditorState = EditorState.create({
+      doc: schema.topNodeType.create(null, [
+        schema.nodes.list.create(null, [
+          schema.nodes.list_item.create(null, [
+            schema.nodes.paragraph.create(null, [schema.text("first")]),
+          ]),
+        ]),
+      ]),
+      plugins: [react()],
+    });
+
+    const initialPluginState = reactPluginKey.getState(initialEditorState)!;
+
+    const nextEditorState = initialEditorState.apply(
+      initialEditorState.tr.insert(
+        1,
+        schema.nodes.list_item.create(null, [
+          schema.nodes.paragraph.create(null, [schema.text("second")]),
+        ])!
+      )
+    );
+    const nextPluginState = reactPluginKey.getState(nextEditorState)!;
+
+    // The new list item was inserted before the original one,
+    // pushing it further into the document. The original list
+    // item should keep its original key, and the new list item
+    // should be assigned a new one
+    expect(nextPluginState.posToKey.get(11)).toBe(
+      initialPluginState.posToKey.get(1)
+    );
+    expect(nextPluginState.posToKey.get(1)).not.toBe(
+      initialPluginState.posToKey.get(1)
+    );
+  });
+
+  it("should maintain key stability when wrapping a node", () => {
+    const initialEditorState = EditorState.create({
+      doc: schema.topNodeType.create(null, [
+        schema.nodes.paragraph.create(null, [schema.text("content")]),
+      ]),
+      plugins: [react()],
+    });
+
+    const initialPluginState = reactPluginKey.getState(initialEditorState)!;
+    const start = 1;
+    const end = 9;
+    const tr = initialEditorState.tr.delete(start, end);
+    const $start = tr.doc.resolve(start);
+
+    const range = $start.blockRange()!;
+    const wrapping = range && findWrapping(range, schema.nodes.list, null)!;
+    tr.wrap(range, wrapping);
+    const nextEditorState = initialEditorState.apply(tr);
+    const nextPluginState = reactPluginKey.getState(nextEditorState)!;
+
+    // The new list and list item nodes were wrapped around the
+    // paragraph, pushing it further into the document. The paragraph
+    // should keep its original key, and the new nodes
+    // should be assigned a new one
+    expect(nextPluginState.posToKey.get(2)).toBe(
+      initialPluginState.posToKey.get(0)
+    );
+    expect(nextPluginState.posToKey.get(0)).not.toBe(
+      initialPluginState.posToKey.get(0)
+    );
+  });
 });

src/plugins/react.ts

@@ -8,12 +8,6 @@ export const ROOT_NODE_KEY = Symbol("@nytimes/react-prosemirror/root-node-key");
 
 export type NodeKey = string | typeof ROOT_NODE_KEY;
 
-/**
- * Identifies a node view constructor as having been created
- * by @nytimes/react-prosemirror
- */
-export const REACT_NODE_VIEW = Symbol("react node view");
-
 export function createNodeKey() {
   return Math.floor(Math.random() * 0xffffff).toString(16);
 }
@@ -68,19 +62,20 @@ export function react() {
           posToKey: new Map<number, string>(),
           keyToPos: new Map<string, number>(),
         };
-        const nextKeys = new Set<string>();
+        for (const [pos, key] of value.posToKey.entries()) {
+          const { pos: newPos, deleted } = tr.mapping.mapResult(pos);
+          if (deleted) continue;
+
+          next.posToKey.set(newPos, key);
+          next.keyToPos.set(key, newPos);
+        }
         newState.doc.descendants((node, pos) => {
           if (node.isText) return false;
+          if (next.posToKey.has(pos)) return true;
 
-          const prevPos = tr.mapping.invert().map(pos);
-          const prevKey = value.posToKey.get(prevPos) ?? createNodeKey();
-          // If this transaction adds a new node, there will be multiple
-          // nodes that map back to the same initial position. In this case,
-          // create new keys for new nodes.
-          const key = nextKeys.has(prevKey) ? createNodeKey() : prevKey;
+          const key = createNodeKey();
           next.posToKey.set(pos, key);
           next.keyToPos.set(key, pos);
-          nextKeys.add(key);
           return true;
         });
         return next;

yarn.lock

@@ -1251,6 +1251,7 @@ __metadata:
     prosemirror-model: ^1.18.3
     prosemirror-schema-list: ^1.2.2
     prosemirror-state: ^1.4.2
+    prosemirror-transform: ^1.8.0
     prosemirror-view: ^1.29.1
     react: ^18.2.0
     react-dom: ^18.2.0
@@ -6991,6 +6992,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"prosemirror-transform@npm:^1.8.0":
+  version: 1.8.0
+  resolution: "prosemirror-transform@npm:1.8.0"
+  dependencies:
+    prosemirror-model: ^1.0.0
+  checksum: 6d16ca4f954ad7b040a4adbb5ddfa8c8ad14b0514f15e1ecfd5e32f08eb3f8696492975b9e599b4776e991ab76df114166dcf6ec7b966a67b02b2069a28415f1
+  languageName: node
+  linkType: hard
+
 "prosemirror-view@npm:^1.27.0, prosemirror-view@npm:^1.29.1":
   version: 1.30.1
   resolution: "prosemirror-view@npm:1.30.1"