fix(richtext-lexical): prevent extra paragraph when inserting blocks or uploadNodes. Add preemptive selection normalization (#12077)

Fixes #11628

PR #6389 caused bug #11628, which is a regression, as it had already
been fixed in #4441

It is likely that some things have changed because [Lexical had recently
made improvements](https://github.com/facebook/lexical/pull/7046) to
address selection normalization.

Although it wasn't necessary to resolve the issue, I added a
`NormalizeSelectionPlugin` to the editor, which makes selection handling
in the editor more robust.

I'm also adding a new collection to the Lexical test suite, intending it
to be used by default for most tests going forward. I've left an
explanatory comment on the dashboard.

___

Looking at #11628's video, it seems users also want to be able to
prevent the first paragraph from being empty. This makes sense to me, so
I think in another PR we could add a button at the top, just [like we
did at the bottom of the
editor](https://github.com/payloadcms/payload/pull/10530).

packages/richtext-lexical/src/features/blocks/client/plugin/index.tsx

@@ -56,22 +56,15 @@ export const BlocksPlugin: PluginComponent = () => {
 
             if ($isRangeSelection(selection)) {
               const blockNode = $createBlockNode(payload)
+
+              // we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node
+              const { focus } = selection
+              const focusNode = focus.getNode()
               // Insert blocks node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
               $insertNodeToNearestRoot(blockNode)
 
-              const { focus } = selection
+              // Delete the node it it's an empty paragraph
-              const focusNode = focus.getNode()
+              if ($isParagraphNode(focusNode) && !focusNode.__first) {
-
-              // First, delete currently selected node if it's an empty paragraph and if there are sufficient
-              // paragraph nodes (more than 1) left in the parent node, so that we don't "trap" the user
-              if (
-                $isParagraphNode(focusNode) &&
-                focusNode.getTextContentSize() === 0 &&
-                focusNode
-                  .getParentOrThrow()
-                  .getChildren()
-                  .filter((node) => $isParagraphNode(node)).length > 1
-              ) {
                 focusNode.remove()
               }
             }

packages/richtext-lexical/src/features/relationship/client/plugins/index.tsx

@@ -53,22 +53,14 @@ export const RelationshipPlugin: PluginComponent<RelationshipFeatureProps> = ({
 
         if ($isRangeSelection(selection)) {
           const relationshipNode = $createRelationshipNode(payload)
+          // we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node
+          const { focus } = selection
+          const focusNode = focus.getNode()
           // Insert relationship node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
           $insertNodeToNearestRoot(relationshipNode)
 
-          const { focus } = selection
+          // Delete the node it it's an empty paragraph
-          const focusNode = focus.getNode()
+          if ($isParagraphNode(focusNode) && !focusNode.__first) {
-
-          // First, delete currently selected node if it's an empty paragraph and if there are sufficient
-          // paragraph nodes (more than 1) left in the parent node, so that we don't "trap" the user
-          if (
-            $isParagraphNode(focusNode) &&
-            focusNode.getTextContentSize() === 0 &&
-            focusNode
-              .getParentOrThrow()
-              .getChildren()
-              .filter((node) => $isParagraphNode(node)).length > 1
-          ) {
             focusNode.remove()
           }
         }

packages/richtext-lexical/src/features/upload/client/plugin/index.tsx

@@ -53,18 +53,14 @@ export const UploadPlugin: PluginComponent<UploadFeaturePropsClient> = ({ client
                   value: payload.value,
                 },
               })
+              // we need to get the focus node before inserting the block node, as $insertNodeToNearestRoot can change the focus node
+              const { focus } = selection
+              const focusNode = focus.getNode()
               // Insert upload node BEFORE potentially removing focusNode, as $insertNodeToNearestRoot errors if the focusNode doesn't exist
               $insertNodeToNearestRoot(uploadNode)
 
-              const { focus } = selection
+              // Delete the node it it's an empty paragraph
-              const focusNode = focus.getNode()
+              if ($isParagraphNode(focusNode) && !focusNode.__first) {
-
-              // Delete the node it it's an empty paragraph and it has at least one sibling, so that we don't "trap" the user
-              if (
-                $isParagraphNode(focusNode) &&
-                !focusNode.__first &&
-                (focusNode.__prev || focusNode.__next)
-              ) {
                 focusNode.remove()
               }
             }

packages/richtext-lexical/src/lexical/LexicalEditor.tsx

@@ -4,13 +4,7 @@ import { LexicalErrorBoundary } from '@lexical/react/LexicalErrorBoundary.js'
 import { HistoryPlugin } from '@lexical/react/LexicalHistoryPlugin.js'
 import { OnChangePlugin } from '@lexical/react/LexicalOnChangePlugin.js'
 import { RichTextPlugin } from '@lexical/react/LexicalRichTextPlugin.js'
-import {
+import { BLUR_COMMAND, COMMAND_PRIORITY_LOW, FOCUS_COMMAND } from 'lexical'
-  $createParagraphNode,
-  $getRoot,
-  BLUR_COMMAND,
-  COMMAND_PRIORITY_LOW,
-  FOCUS_COMMAND,
-} from 'lexical'
 import * as React from 'react'
 import { useEffect, useState } from 'react'
 
@@ -24,6 +18,7 @@ import { AddBlockHandlePlugin } from './plugins/handles/AddBlockHandlePlugin/ind
 import { DraggableBlockPlugin } from './plugins/handles/DraggableBlockPlugin/index.js'
 import { InsertParagraphAtEndPlugin } from './plugins/InsertParagraphAtEnd/index.js'
 import { MarkdownShortcutPlugin } from './plugins/MarkdownShortcut/index.js'
+import { NormalizeSelectionPlugin } from './plugins/NormalizeSelection/index.js'
 import { SlashMenuPlugin } from './plugins/SlashMenu/index.js'
 import { TextPlugin } from './plugins/TextPlugin/index.js'
 import { LexicalContentEditable } from './ui/ContentEditable.js'
@@ -112,6 +107,7 @@ export const LexicalEditor: React.FC<
           }
           ErrorBoundary={LexicalErrorBoundary}
         />
+        <NormalizeSelectionPlugin />
         <InsertParagraphAtEndPlugin />
         <DecoratorPlugin />
         <TextPlugin features={editorConfig.features} />

packages/richtext-lexical/src/lexical/plugins/NormalizeSelection/index.tsx

@@ -0,0 +1,35 @@
+import { useLexicalComposerContext } from '@lexical/react/LexicalComposerContext'
+import { $getSelection, $isRangeSelection, RootNode } from 'lexical'
+import { useEffect } from 'react'
+
+/**
+ * By default, Lexical throws an error if the selection ends in deleted nodes.
+ * This is very aggressive considering there are reasons why this can happen
+ * outside of Payload's control (custom features or conflicting features, for example).
+ * In the case of selections on nonexistent nodes, this plugin moves the selection to
+ * the end of the editor and displays a warning instead of an error.
+ */
+export function NormalizeSelectionPlugin() {
+  const [editor] = useLexicalComposerContext()
+
+  useEffect(() => {
+    return editor.registerNodeTransform(RootNode, (root) => {
+      const selection = $getSelection()
+      if ($isRangeSelection(selection)) {
+        const anchorNode = selection.anchor.getNode()
+        const focusNode = selection.focus.getNode()
+        if (!anchorNode.isAttached() || !focusNode.isAttached()) {
+          root.selectEnd()
+          // eslint-disable-next-line no-console
+          console.warn(
+            'updateEditor: selection has been moved to the end of the editor because the previously selected nodes have been removed and ' +
+              "selection wasn't moved to another node. Ensure selection changes after removing/replacing a selected node.",
+          )
+        }
+      }
+      return false
+    })
+  }, [editor])
+
+  return null
+}

test/lexical/baseConfig.ts

@@ -2,6 +2,7 @@ import { fileURLToPath } from 'node:url'
 import path from 'path'
 import { type Config } from 'payload'
 
+import { LexicalFullyFeatured } from './collections/_LexicalFullyFeatured/index.js'
 import ArrayFields from './collections/Array/index.js'
 import {
   getLexicalFieldsCollection,
@@ -26,6 +27,7 @@ const dirname = path.dirname(filename)
 export const baseConfig: Partial<Config> = {
   // ...extend config here
   collections: [
+    LexicalFullyFeatured,
     getLexicalFieldsCollection({
       blocks: lexicalBlocks,
       inlineBlocks: lexicalInlineBlocks,
@@ -42,10 +44,18 @@ export const baseConfig: Partial<Config> = {
     ArrayFields,
   ],
   globals: [TabsWithRichText],
+
   admin: {
     importMap: {
       baseDir: path.resolve(dirname),
     },
+    components: {
+      beforeDashboard: [
+        {
+          path: './components/CollectionsExplained.tsx#CollectionsExplained',
+        },
+      ],
+    },
   },
   onInit: async (payload) => {
     if (process.env.SEED_IN_CONFIG_ONINIT !== 'false') {

test/lexical/collections/Lexical/e2e/blocks/e2e.spec.ts

@@ -302,7 +302,7 @@ describe('lexicalBlocks', () => {
     await assertLexicalDoc({
       fn: ({ lexicalWithBlocks }) => {
         const rscBlock: SerializedBlockNode = lexicalWithBlocks.root
-          .children[14] as SerializedBlockNode
+          .children[13] as SerializedBlockNode
         const paragraphNode: SerializedParagraphNode = lexicalWithBlocks.root
           .children[12] as SerializedParagraphNode
 
@@ -1133,9 +1133,9 @@ describe('lexicalBlocks', () => {
           ).docs[0] as never
 
           const richTextBlock: SerializedBlockNode = lexicalWithBlocks.root
-            .children[13] as SerializedBlockNode
+            .children[12] as SerializedBlockNode
           const subRichTextBlock: SerializedBlockNode = richTextBlock.fields.richTextField.root
-            .children[1] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
+            .children[0] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
 
           const subSubRichTextField = subRichTextBlock.fields.subRichTextField
           const subSubUploadField = subRichTextBlock.fields.subUploadField
@@ -1163,9 +1163,9 @@ describe('lexicalBlocks', () => {
           ).docs[0] as never
 
           const richTextBlock2: SerializedBlockNode = lexicalWithBlocks.root
-            .children[13] as SerializedBlockNode
+            .children[12] as SerializedBlockNode
           const subRichTextBlock2: SerializedBlockNode = richTextBlock2.fields.richTextField.root
-            .children[1] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
+            .children[0] as SerializedBlockNode // index 0 and 2 are paragraphs created by default around the block node when a new block is added via slash command
 
           const subSubRichTextField2 = subRichTextBlock2.fields.subRichTextField
           const subSubUploadField2 = subRichTextBlock2.fields.subUploadField

test/lexical/collections/Lexical/e2e/main/e2e.spec.ts

@@ -728,7 +728,8 @@ describe('lexicalMain', () => {
     await expect(relationshipListDrawer).toBeVisible()
     await wait(500)
 
-    await expect(relationshipListDrawer.locator('.rs__single-value')).toHaveText('Lexical Field')
+    await relationshipListDrawer.locator('.rs__input').first().click()
+    await relationshipListDrawer.locator('.rs__menu').getByText('Lexical Field').click()
 
     await relationshipListDrawer.locator('button').getByText('Rich Text').first().click()
     await expect(relationshipListDrawer).toBeHidden()
@@ -1203,10 +1204,11 @@ describe('lexicalMain', () => {
     await expect(newUploadNode.locator('.lexical-upload__bottomRow')).toContainText('payload.png')
 
     await page.keyboard.press('Enter') // floating toolbar needs to appear with enough distance to the upload node, otherwise clicking may fail
+    await page.keyboard.press('Enter')
     await page.keyboard.press('ArrowLeft')
     await page.keyboard.press('ArrowLeft')
     // Select "there" by pressing shift + arrow left
-    for (let i = 0; i < 4; i++) {
+    for (let i = 0; i < 5; i++) {
       await page.keyboard.press('Shift+ArrowLeft')
     }
 
@@ -1258,10 +1260,10 @@ describe('lexicalMain', () => {
       const firstParagraph: SerializedParagraphNode = lexicalField.root
         .children[0] as SerializedParagraphNode
       const secondParagraph: SerializedParagraphNode = lexicalField.root
-        .children[1] as SerializedParagraphNode
-      const thirdParagraph: SerializedParagraphNode = lexicalField.root
         .children[2] as SerializedParagraphNode
-      const uploadNode: SerializedUploadNode = lexicalField.root.children[3] as SerializedUploadNode
+      const thirdParagraph: SerializedParagraphNode = lexicalField.root
+        .children[3] as SerializedParagraphNode
+      const uploadNode: SerializedUploadNode = lexicalField.root.children[1] as SerializedUploadNode
 
       expect(firstParagraph.children).toHaveLength(2)
       expect((firstParagraph.children[0] as SerializedTextNode).text).toBe('Some ')
@@ -1391,7 +1393,7 @@ describe('lexicalMain', () => {
       const lexicalField: SerializedEditorState = lexicalDoc.lexicalRootEditor
 
       // @ts-expect-error no need to type this
-      expect(lexicalField?.root?.children[1].fields.someTextRequired).toEqual('test')
+      expect(lexicalField?.root?.children[0].fields.someTextRequired).toEqual('test')
     }).toPass({
       timeout: POLL_TOPASS_TIMEOUT,
     })

test/lexical/collections/_LexicalFullyFeatured/e2e.spec.ts

@@ -0,0 +1,68 @@
+import { expect, test } from '@playwright/test'
+import { AdminUrlUtil } from 'helpers/adminUrlUtil.js'
+import { reInitializeDB } from 'helpers/reInitializeDB.js'
+import { lexicalFullyFeaturedSlug } from 'lexical/slugs.js'
+import path from 'path'
+import { fileURLToPath } from 'url'
+
+import { ensureCompilationIsDone } from '../../../helpers.js'
+import { initPayloadE2ENoConfig } from '../../../helpers/initPayloadE2ENoConfig.js'
+import { TEST_TIMEOUT_LONG } from '../../../playwright.config.js'
+import { LexicalHelpers } from './utils.js'
+const filename = fileURLToPath(import.meta.url)
+const currentFolder = path.dirname(filename)
+const dirname = path.resolve(currentFolder, '../../')
+
+const { beforeAll, beforeEach, describe } = test
+
+// Unlike the other suites, this one runs in parallel, as they run on the `lexical-fully-featured/create` URL and are "pure" tests
+test.describe.configure({ mode: 'parallel' })
+
+const { serverURL } = await initPayloadE2ENoConfig({
+  dirname,
+})
+
+describe('Lexical Fully Featured', () => {
+  beforeAll(async ({ browser }, testInfo) => {
+    testInfo.setTimeout(TEST_TIMEOUT_LONG)
+    process.env.SEED_IN_CONFIG_ONINIT = 'false' // Makes it so the payload config onInit seed is not run. Otherwise, the seed would be run unnecessarily twice for the initial test run - once for beforeEach and once for onInit
+    const page = await browser.newPage()
+    await ensureCompilationIsDone({ page, serverURL })
+    await page.close()
+  })
+  beforeEach(async ({ page }) => {
+    await reInitializeDB({
+      serverURL,
+      snapshotKey: 'fieldsTest',
+      uploadsDir: [
+        path.resolve(dirname, './collections/Upload/uploads'),
+        path.resolve(dirname, './collections/Upload2/uploads2'),
+      ],
+    })
+    const url = new AdminUrlUtil(serverURL, lexicalFullyFeaturedSlug)
+    const lexical = new LexicalHelpers(page)
+    await page.goto(url.create)
+    await lexical.editor.first().focus()
+  })
+  test('prevent extra paragraph when inserting decorator blocks like blocks or upload node', async ({
+    page,
+  }) => {
+    const lexical = new LexicalHelpers(page)
+    await lexical.slashCommand('block')
+    await lexical.slashCommand('relationship')
+    await lexical.drawer.locator('.list-drawer__header').getByText('Create New').click()
+    await lexical.save('drawer')
+    await expect(lexical.decorator).toHaveCount(2)
+    await lexical.slashCommand('upload')
+    await lexical.drawer.locator('.list-drawer__header').getByText('Create New').click()
+    await lexical.drawer.getByText('Paste URL').click()
+    await lexical.drawer
+      .locator('.file-field__remote-file')
+      .fill('https://payloadcms.com/images/universal-truth.jpg')
+    await lexical.drawer.getByText('Add file').click()
+    await lexical.save('drawer')
+    await expect(lexical.decorator).toHaveCount(3)
+    const paragraph = lexical.editor.locator('> p')
+    await expect(paragraph).toHaveText('')
+  })
+})

test/lexical/collections/_LexicalFullyFeatured/index.ts

@@ -0,0 +1,57 @@
+import type { CollectionConfig } from 'payload'
+
+import {
+  BlocksFeature,
+  EXPERIMENTAL_TableFeature,
+  FixedToolbarFeature,
+  lexicalEditor,
+  TreeViewFeature,
+} from '@payloadcms/richtext-lexical'
+
+import { lexicalFullyFeaturedSlug } from '../../slugs.js'
+
+export const LexicalFullyFeatured: CollectionConfig = {
+  slug: lexicalFullyFeaturedSlug,
+  labels: {
+    singular: 'Lexical Fully Featured',
+    plural: 'Lexical Fully Featured',
+  },
+  fields: [
+    {
+      name: 'richText',
+      type: 'richText',
+      editor: lexicalEditor({
+        features: ({ defaultFeatures }) => [
+          ...defaultFeatures,
+          TreeViewFeature(),
+          FixedToolbarFeature(),
+          EXPERIMENTAL_TableFeature(),
+          BlocksFeature({
+            blocks: [
+              {
+                slug: 'myBlock',
+                fields: [
+                  {
+                    name: 'someText',
+                    type: 'text',
+                  },
+                ],
+              },
+            ],
+            inlineBlocks: [
+              {
+                slug: 'myInlineBlock',
+                fields: [
+                  {
+                    name: 'someText',
+                    type: 'text',
+                  },
+                ],
+              },
+            ],
+          }),
+        ],
+      }),
+    },
+  ],
+}

test/lexical/collections/_LexicalFullyFeatured/utils.ts

@@ -0,0 +1,49 @@
+import type { Page } from 'playwright'
+
+import { expect } from '@playwright/test'
+
+export class LexicalHelpers {
+  page: Page
+  constructor(page: Page) {
+    this.page = page
+  }
+
+  async save(container: 'document' | 'drawer') {
+    if (container === 'drawer') {
+      await this.drawer.getByText('Save').click()
+    } else {
+      throw new Error('Not implemented')
+    }
+    await this.page.waitForTimeout(1000)
+  }
+
+  async slashCommand(
+    // prettier-ignore
+    command: 'block' | 'check' | 'code' | 'h1' | 'h2' | 'h3' | 'h4' | 'h5' |'h6' | 'inline' 
+    | 'link' | 'ordered' | 'paragraph' | 'quote' | 'relationship' | 'unordered' | 'upload',
+  ) {
+    await this.page.keyboard.press(`/`)
+
+    const slashMenuPopover = this.page.locator('#slash-menu .slash-menu-popup')
+    await expect(slashMenuPopover).toBeVisible()
+    await this.page.keyboard.type(command)
+    await this.page.keyboard.press(`Enter`)
+    await expect(slashMenuPopover).toBeHidden()
+  }
+
+  get decorator() {
+    return this.editor.locator('[data-lexical-decorator="true"]')
+  }
+
+  get drawer() {
+    return this.page.locator('.drawer__content')
+  }
+
+  get editor() {
+    return this.page.locator('[data-lexical-editor="true"]')
+  }
+
+  get paragraph() {
+    return this.editor.locator('p')
+  }
+}

test/lexical/components/CollectionsExplained.tsx

@@ -0,0 +1,22 @@
+import React from 'react'
+
+export function CollectionsExplained() {
+  return (
+    <div>
+      <h1>Which collection should I use for my tests?</h1>
+
+      <p>
+        By default and as a rule of thumb: "Lexical Fully Featured". This collection has all our
+        features, but it does NOT have (and will never have):
+      </p>
+      <ul>
+        <li>Relationships or dependencies to other collections</li>
+        <li>Seeded documents</li>
+        <li>Features with custom props (except for a block and an inline block included)</li>
+        <li>Multiple richtext fields or other fields</li>
+      </ul>
+
+      <p>If you need any of these features, use another collection or create a new one.</p>
+    </div>
+  )
+}

test/lexical/payload-types.ts

@@ -83,6 +83,7 @@ export interface Config {
   };
   blocks: {};
   collections: {
+    'lexical-fully-featured': LexicalFullyFeatured;
     'lexical-fields': LexicalField;
     'lexical-migrate-fields': LexicalMigrateField;
     'lexical-localized-fields': LexicalLocalizedField;
@@ -101,6 +102,7 @@ export interface Config {
   };
   collectionsJoins: {};
   collectionsSelect: {
+    'lexical-fully-featured': LexicalFullyFeaturedSelect<false> | LexicalFullyFeaturedSelect<true>;
     'lexical-fields': LexicalFieldsSelect<false> | LexicalFieldsSelect<true>;
     'lexical-migrate-fields': LexicalMigrateFieldsSelect<false> | LexicalMigrateFieldsSelect<true>;
     'lexical-localized-fields': LexicalLocalizedFieldsSelect<false> | LexicalLocalizedFieldsSelect<true>;
@@ -153,6 +155,30 @@ export interface UserAuthOperations {
     password: string;
   };
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "lexical-fully-featured".
+ */
+export interface LexicalFullyFeatured {
+  id: string;
+  richText?: {
+    root: {
+      type: string;
+      children: {
+        type: string;
+        version: number;
+        [k: string]: unknown;
+      }[];
+      direction: ('ltr' | 'rtl') | null;
+      format: 'left' | 'start' | 'center' | 'right' | 'end' | 'justify' | '';
+      indent: number;
+      version: number;
+    };
+    [k: string]: unknown;
+  } | null;
+  updatedAt: string;
+  createdAt: string;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "lexical-fields".
@@ -774,6 +800,10 @@ export interface User {
 export interface PayloadLockedDocument {
   id: string;
   document?:
+    | ({
+        relationTo: 'lexical-fully-featured';
+        value: string | LexicalFullyFeatured;
+      } | null)
     | ({
         relationTo: 'lexical-fields';
         value: string | LexicalField;
@@ -864,6 +894,15 @@ export interface PayloadMigration {
   updatedAt: string;
   createdAt: string;
 }
+/**
+ * This interface was referenced by `Config`'s JSON-Schema
+ * via the `definition` "lexical-fully-featured_select".
+ */
+export interface LexicalFullyFeaturedSelect<T extends boolean = true> {
+  richText?: T;
+  updatedAt?: T;
+  createdAt?: T;
+}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "lexical-fields_select".

test/lexical/slugs.ts

@@ -1,5 +1,6 @@
 export const usersSlug = 'users'
 
+export const lexicalFullyFeaturedSlug = 'lexical-fully-featured'
 export const lexicalFieldsSlug = 'lexical-fields'
 export const lexicalLocalizedFieldsSlug = 'lexical-localized-fields'
 export const lexicalMigrateFieldsSlug = 'lexical-migrate-fields'