Skip to content

Bug: [lexical-y] A link a collaborator removes comes back when another collaborator sets NodeState on it at the same time #9629

Open

Description

@zurfyx

Lexical version: main at 1853707 (@lexical/y). Also fails with #9601 applied.

Steps To Reproduce

  1. Two clients bind one document (createYDoc, YCollabExtension) holding a paragraph before + a link to https://note.test with the text linked words + after.
  2. Concurrently: client A removes the link (its children move out, the link is removed); client B sets a NodeState value on the link ($setState(link, note, 'checked')).
  3. The clients sync.

Link to code example: the test below. Save it as packages/lexical-y/src/__tests__/unit/Repro.test.ts and run pnpm exec vitest run --project unit packages/lexical-y/src/__tests__/unit/Repro.test.ts. It fails on main with both client id orders: expected 1 to be +0. The same happens when the link's text sits in a mark (link > mark > text) or the link sits in a mark (mark > link > text).

Repro test (fails on main at 1853707, ids [1, 2] and [2, 1])
/**
 * Copyright (c) Meta Platforms, Inc. and affiliates.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 *
 */

// @vitest-environment node

import {buildEditorFromExtensions} from '@lexical/extension';
import {$createLinkNode, $isLinkNode, LinkExtension} from '@lexical/link';
import {RichTextExtension} from '@lexical/rich-text';
import * as Y from '@y/y';
import {
  $createParagraphNode,
  $createTextNode,
  $getRoot,
  $isElementNode,
  $setState,
  configExtension,
  createState,
  defineExtension,
  type ElementNode,
  type LexicalEditor,
} from 'lexical';
import {expect, test} from 'vitest';

import {YCollabExtension} from '../..';
import {createYDoc, seedYState} from '../../headless';

// NodeState an app keeps on a link (a note, a tracking id).
const note = createState('repro-note', {
  parse: (v: unknown) => (typeof v === 'string' ? v : ''),
});

const app = defineExtension({
  dependencies: [RichTextExtension, LinkExtension],
  name: 'repro-app',
});
const config = {extensions: [app], onWarn: () => {}, schemaVersion: 1};

// "before " + link(https://note.test: "linked words") + " after"
function $build() {
  $getRoot()
    .clear()
    .append(
      $createParagraphNode().append(
        $createTextNode('before '),
        $createLinkNode('https://note.test').append(
          $createTextNode('linked words'),
        ),
        $createTextNode(' after'),
      ),
    );
}

function client(seed: Uint8Array, id: number) {
  const doc = createYDoc(seed);
  doc.clientID = id;
  const editor = buildEditorFromExtensions(
    defineExtension({
      $initialEditorState: null,
      dependencies: [
        app,
        configExtension(YCollabExtension, {doc, schemaVersion: 1}),
      ],
      name: `repro-${id}`,
    }),
  );
  return {doc, editor};
}

function links(editor: LexicalEditor): number {
  return editor.read(() => {
    let n = 0;
    const visit = (node: ElementNode) => {
      for (const child of node.getChildren()) {
        if ($isLinkNode(child)) {
          n++;
        }
        if ($isElementNode(child)) {
          visit(child);
        }
      }
    };
    visit($getRoot());
    return n;
  });
}

for (const ids of [
  [1, 2],
  [2, 1],
]) {
  test(`a removed link stays removed when a collaborator sets NodeState on it (ids ${ids})`, async () => {
    const seed = seedYState(config, $build);
    const a = client(seed, ids[0]);
    const b = client(seed, ids[1]);
    await Promise.resolve();
    // A removes the link (the paragraph's children: text, link, text).
    a.editor.update(
      () => {
        const link = (
          $getRoot().getFirstChildOrThrow() as ElementNode
        ).getChildAtIndex(1);
        if (!$isLinkNode(link)) {
          throw new Error('link expected');
        }
        for (const child of link.getChildren()) {
          link.insertBefore(child);
        }
        link.remove();
      },
      {discrete: true},
    );
    expect(links(a.editor)).toBe(0);
    // B, at the same time, sets NodeState on the link.
    b.editor.update(
      () => {
        const link = (
          $getRoot().getFirstChildOrThrow() as ElementNode
        ).getChildAtIndex(1);
        if (!$isLinkNode(link)) {
          throw new Error('link expected');
        }
        $setState(link, note, 'checked');
      },
      {discrete: true},
    );
    const ua = Y.encodeStateAsUpdate(a.doc);
    const ub = Y.encodeStateAsUpdate(b.doc);
    Y.applyUpdate(a.doc, ub);
    Y.applyUpdate(b.doc, ua);
    await Promise.resolve();
    expect(links(a.editor)).toBe(0);
    expect(links(b.editor)).toBe(0);
  });
}

The current behavior

The link is back on both clients after the sync, with B's NodeState.

The expected behavior

Either the removal wins (as a delete wins over concurrent typing into a deleted container), or the outcome is documented as a limit. Today a concurrent write of any of the link's stored values silently undoes a collaborator's removal.

Impact of fix

The binding stores a link as one format value over its text (w:link, its fields and NodeState). B's change writes that whole value again over the link's text, and A's concurrent removal does not reach the new write. It is the same mechanism as #9597 and #9627, triggered by a deliberate change to the link rather than a side effect of formatting. Fixing it needs a storage or merge rule decision (for example, a write to a wrapper's fields does not re-open a wrapper a concurrent change removed), so it is not part of #9601.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions