Prevent collab improperly clearing text blocks on save

packages/hash/api/src/collab/Instance.ts

@@ -15,17 +15,13 @@ import {
   entityStorePluginState,
 } from "@hashintel/hash-shared/entityStorePlugin";
 import {
-  LatestEntityRef,
   GetBlocksQuery,
   GetBlocksQueryVariables,
   GetPageQuery,
   GetPageQueryVariables,
+  LatestEntityRef,
 } from "@hashintel/hash-shared/graphql/apiTypes.gen";
-import {
-  getComponentNodeAttrs,
-  isComponentNode,
-  isEntityNode,
-} from "@hashintel/hash-shared/prosemirror";
+import { isEntityNode } from "@hashintel/hash-shared/prosemirror";
 import { ProsemirrorSchemaManager } from "@hashintel/hash-shared/ProsemirrorSchemaManager";
 import {
   getBlocksQuery,
@@ -39,7 +35,7 @@ import { Response } from "express";
 import { isEqual, memoize, pick } from "lodash";
 import { Schema } from "prosemirror-model";
 import { EditorState } from "prosemirror-state";
-import { Mapping, Step, Transform } from "prosemirror-transform";
+import { Step } from "prosemirror-transform";
 import { logger } from "../logger";
 import { EntityWatcher } from "./EntityWatcher";
 import { InvalidVersionError } from "./errors";
@@ -81,7 +77,6 @@ export class Instance {
   lastActive = Date.now();
   waiting: Waiting[] = [];
   saveChain = Promise.resolve();
-  saveMapping: Mapping | null = null;
   clientIds = new WeakMap<Step, string>();
 
   positionPollers: CollabPositionPoller[] = [];
@@ -288,9 +283,6 @@ export class Instance {
 
         const result = tr.maybeStep(steps[i]);
         if (!result.doc) return false;
-        if (this.saveMapping) {
-          this.saveMapping.appendMap(steps[i].getMap());
-        }
       }
 
       for (const action of actions) {
@@ -317,8 +309,6 @@ export class Instance {
     };
 
   save = (apolloClient: ApolloClient<unknown>) => (clientID: string) => {
-    const mapping = new Mapping();
-
     this.saveChain = this.saveChain
       .catch()
       .then(async () => {
@@ -340,7 +330,6 @@ export class Instance {
         return createdEntities;
       })
       .then((createdEntities) => {
-        this.saveMapping = mapping;
         const { doc } = this.state;
         const store = entityStorePluginState(this.state);
 
@@ -353,13 +342,6 @@ export class Instance {
           apolloClient,
           createdEntities,
         ).then((newPage) => {
-          /**
-           * This is purposefully based on the current doc, not the doc at
-           * the time of save, because we need to apply transforms to the
-           * current doc based on the result of the save query (in order to
-           * insert entity ids for new blocks)
-           */
-          const transform = new Transform<Schema>(this.state.doc);
           const actions: EntityStorePluginAction[] = [];
 
           /**
@@ -426,23 +408,9 @@ export class Instance {
                   },
                 });
               }
-            } else if (isComponentNode(node) && !node.attrs.blockEntityId) {
-              transform.setNodeMarkup(
-                mapping.map(pos),
-                undefined,
-                getComponentNodeAttrs(blockEntity),
-              );
             }
           });
 
-          /**
-           * We're posting actions and steps from the post-save
-           * transformation process for maximum safety – as we need to
-           * ensure the actions are processed before the steps, and that's
-           * not easy to do in one message right now
-           *
-           * @todo combine the two calls to addEvents
-           */
           if (actions.length) {
             this.addEvents(apolloClient)(
               this.version,
@@ -453,27 +421,11 @@ export class Instance {
             );
           }
 
-          if (transform.docChanged) {
-            this.addEvents(apolloClient)(
-              this.version,
-              transform.steps,
-              `${clientID}-server`,
-              [],
-              false,
-            );
-          }
-
           this.updateSavedContents(newPage.properties.contents);
         });
       })
       .catch((err) => {
         logger.error("could not save", err);
-      })
-
-      .finally(() => {
-        if (this.saveMapping === mapping) {
-          this.saveMapping = null;
-        }
       });
   };
 

packages/hash/frontend/src/blocks/page/BlockView.tsx

@@ -1,25 +1,25 @@
-import { BlockVariant } from "blockprotocol";
 import { BlockMeta } from "@hashintel/hash-shared/blockMeta";
+import { EntityStore } from "@hashintel/hash-shared/entityStore";
+import {
+  entityStorePluginState,
+  subscribeToEntityStore,
+} from "@hashintel/hash-shared/entityStorePlugin";
+import { isEntityNode } from "@hashintel/hash-shared/prosemirror";
 import { ProsemirrorSchemaManager } from "@hashintel/hash-shared/ProsemirrorSchemaManager";
-import { findComponentNodes } from "@hashintel/hash-shared/prosemirror";
+import { BlockVariant } from "blockprotocol";
 import { ProsemirrorNode, Schema } from "prosemirror-model";
 import { NodeSelection } from "prosemirror-state";
 import { EditorView, NodeView } from "prosemirror-view";
 import { createRef, forwardRef, RefObject, useMemo, useState } from "react";
 import { useOutsideClick } from "rooks";
 import { tw } from "twind";
-import { EntityStore } from "@hashintel/hash-shared/entityStore";
-import {
-  entityStorePluginState,
-  subscribeToEntityStore,
-} from "@hashintel/hash-shared/entityStorePlugin";
 import { BlockContextMenu } from "../../components/BlockContextMenu/BlockContextMenu";
-import { BlockSuggesterProps } from "./createSuggester/BlockSuggester";
 import { DragVerticalIcon } from "../../components/icons";
+import { BlockViewContext } from "./BlockViewContext";
+import { CollabPositionIndicators } from "./CollabPositionIndicators";
+import { BlockSuggesterProps } from "./createSuggester/BlockSuggester";
 import styles from "./style.module.css";
 import { RenderPortal } from "./usePortals";
-import { CollabPositionIndicators } from "./CollabPositionIndicators";
-import { BlockViewContext } from "./BlockViewContext";
 
 type BlockHandleProps = {
   entityId: string | null;
@@ -85,14 +85,19 @@ export class BlockView implements NodeView<Schema> {
   private unsubscribe: Function;
 
   getBlockEntityIdFromNode = (node: ProsemirrorNode<Schema>) => {
-    const componentNodes = findComponentNodes(node);
-    if (componentNodes.length !== 1) {
+    const blockEntityNode = node.firstChild;
+
+    if (!blockEntityNode || !isEntityNode(blockEntityNode)) {
+      throw new Error("Unexpected prosemirror structure");
+    }
+
+    if (!blockEntityNode.attrs.draftId) {
       return null;
     }
-    const { blockEntityId } = componentNodes[0][0].attrs;
 
-    // The current default for this is an empty string, not null.
-    return blockEntityId === "" ? null : blockEntityId;
+    const draftEntity = this.store.draft[blockEntityNode.attrs.draftId];
+
+    return draftEntity?.entityId ?? null;
   };
 
   constructor(

packages/hash/frontend/src/blocks/page/ComponentView.tsx

@@ -126,10 +126,11 @@ export class ComponentView implements NodeView<Schema> {
         .resolve(this.getPos())
         .node(2).attrs.draftId;
 
-      // @todo handle entity id not being defined
-      const entityId = node.attrs.blockEntityId ?? "";
-      const entity = this.store.draft[blockDraftId];
       const mappedUrl = componentIdToUrl(this.componentId);
+      const entity = this.store.draft[blockDraftId];
+
+      // @todo handle entity id not being defined
+      const entityId = entity.entityId ?? "";
 
       /** used by collaborative editing feature `FocusTracker` */
       this.target.setAttribute("data-entity-id", entityId);

packages/hash/playwright/tests/page-creation.spec.ts

@@ -76,20 +76,19 @@ test("user can create page", async ({ page }) => {
 
   // TODO: Move the cursor below the new divider and update the test?
 
-  // TODO: Uncomment after fixing flaky ProseMirror behavior
-  // // Insert a paragraph creation with newlines
-  // await sleep(100); // TODO: investigate flakiness in FF and Webkit
-  // await page.keyboard.type("Second paragraph");
-  // await sleep(100); // TODO: investigate flakiness in FF and Webkit
-  // await page.keyboard.press("Shift+Enter");
-  // await sleep(100); // TODO: investigate flakiness in FF and Webkit
-  // await page.keyboard.press("Shift+Enter");
-  // await sleep(100); // TODO: investigate flakiness in FF and Webkit
-  // await page.keyboard.type("with");
-  // await page.keyboard.press("Shift+Enter");
-  // await sleep(100); // TODO: investigate flakiness in FF and Webkit
-  // await page.keyboard.type("line breaks");
-  // await sleep(100); // TODO: investigate flakiness in FF and Webkit
+  // Insert a paragraph creation with newlines
+  await sleep(100); // TODO: investigate flakiness in FF and Webkit
+  await page.keyboard.type("Second paragraph");
+  await sleep(100); // TODO: investigate flakiness in FF and Webkit
+  await page.keyboard.press("Shift+Enter");
+  await sleep(100); // TODO: investigate flakiness in FF and Webkit
+  await page.keyboard.press("Shift+Enter");
+  await sleep(100); // TODO: investigate flakiness in FF and Webkit
+  await page.keyboard.type("with");
+  await page.keyboard.press("Shift+Enter");
+  await sleep(100); // TODO: investigate flakiness in FF and Webkit
+  await page.keyboard.type("line breaks");
+  await sleep(100); // TODO: investigate flakiness in FF and Webkit
 
   // Expect just inserted content to be present on the page
   await expect(blockRegionLocator).toContainText(
@@ -124,11 +123,10 @@ test("user can create page", async ({ page }) => {
     blockRegionLocator.locator("p").nth(0).locator("em"),
   ).toContainText("italics");
 
-  // TODO: Uncomment after fixing flaky ProseMirror behavior
-  // await expect(blockRegionLocator.locator("p").nth(1)).toContainText(
-  //   "Second paragraph\n\nwith\nline breaks",
-  //   { useInnerText: true },
-  // );
+  await expect(blockRegionLocator.locator("p").nth(1)).toContainText(
+    "Second paragraph\n\nwith\nline breaks",
+    { useInnerText: true },
+  );
 
   await expect(blockRegionLocator.locator("hr")).toBeVisible();