[lexical-table] Feature: add config for opting in to nested tables by james-atticus · Pull Request #7983 · facebook/lexical

james-atticus · 2025-11-11T05:35:58Z

Description

#7192 made changes to try and prevent nested tables. They weren't removed from the document tree however, and could still be inserted when not using commands. @zurfyx also advised that nested tables are wanted at Meta, even if they're not fully supported (we also want to be able to use them).

This PR adds a config option to the plugin/extension allowing developers to opt in to allowing nested tables.

Note: there are no fixes/improvements for nested tables included (eg: #7154 is still an issue). The config option is marked @experimental to convey that nested tables are not properly supported.

vercel · 2025-11-11T05:36:06Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
lexical	Ready	Preview	Comment	Nov 11, 2025 7:00am
lexical-playground	Ready	Preview	Comment	Nov 11, 2025 7:00am

james-atticus · 2025-11-11T05:38:34Z

packages/lexical-table/src/__tests__/unit/LexicalTableExtension.test.ts

-      if (selection === null) {
-        throw new Error('Expected valid selection');
-      }
-      editor.dispatchCommand(SELECTION_INSERT_CLIPBOARD_NODES_COMMAND, {


This wasn't actually testing anything before: verified by removing the command handler and the test still passed. We need to instead use $insertGeneratedNodes which only progresses if all the handlers for SELECTION_INSERT_CLIPBOARD_NODES_COMMAND return false.

james-atticus · 2025-11-11T05:41:09Z

packages/lexical-table/src/__tests__/unit/LexicalTableExtension.test.ts

+    const extension = getExtensionDependencyFromEditor(editor, TableExtension);
+    extension.output.hasNestedTables.value = true;


@etrepum I couldn't find any examples in the codebase where extension config was being updated after-the-fact. Let me know if this isn't the right way to go about it.

When output properties are signals they're intended to be mutable after the fact, doing it this way is fine. It's a little awkward because this would normally just be configured up front, but it is a sort of partial test that the signal is wired up correctly.

james-atticus · 2025-11-11T05:42:19Z

packages/lexical-table/src/__tests__/unit/LexicalTableExtension.test.ts

+      editor.update(
+        () => {
+          const root = $getRoot().clear();
+          const table = $createTableNode();
+          const row = $createTableRowNode();
+          const cell = $createTableCellNode();
+          const paragraph = $createParagraphNode();
+          cell.append(paragraph);
+          row.append(cell);
+          table.append(row);
+          root.append(table);
+          paragraph.select();
+        },
+        {discrete: true},
+      );


Switched from async to discrete: true, and added a paragraph inside the cell (otherwise $insertGeneratedNodes throws). Test is otherwise the same as it was before.

james-atticus · 2025-11-11T05:43:10Z

packages/lexical-table/src/__tests__/unit/LexicalTableExtension.test.ts

-      root.append(table);
-      cell.select();
-    });
+  it('Prevents nested tables by default', async () => {


Github is struggling a bit with this diff. Recommend reviewing commits individually and with whitespace hidden.

james-atticus · 2025-11-11T05:43:54Z

packages/lexical-table/src/LexicalTablePluginHelpers.ts

  });
 }

+export function registerPreventNestedTablesHandlers(


This might be better named as just registerPreventNestedTables. Open to suggestions.

For the INSERT_TABLE_COMMAND handler this could also be implemented by passing the signal through to the insertTableCommandListener so that it can peek() the current value, which would prevent the need for registering a separate command listener at a higher priority to check this condition

james-atticus · 2025-11-11T05:44:28Z

packages/lexical-playground/__tests__/e2e/Tables.spec.mjs

+      isCollab,
+    }) => {
+      test.skip(isPlainText);
+      await initialize({hasNestedTables: true, isCollab, page});


Same as the tests removed in #7192 but with hasNestedTables: true added.

etrepum

This looks fine, I don't see a good reason to support this experiment in both the extension and the plugin but I don't think that's a blocker just more busywork

etrepum · 2025-11-11T06:05:25Z

packages/lexical-react/src/LexicalTablePlugin.ts

+  // Prevent nested tables being inserted when the feature isn't enabled
+  useEffect(() => {
+    if (!hasNestedTables) {
+      return registerPreventNestedTablesHandlers(editor);
+    }
+  }, [editor, hasNestedTables]);
+


This might be a good opportunity to just stop adding features to the react plugin and only support it in the extension

I took a stab at this but I think it'd require a large refactor of the playground App/Editor components. We'd have to replace TablePlugin with TableExtension, and I wasn't sure how to properly update the extension's config based on the values from useSettings.

I've switched to using signals for registerTablePlugin though, which avoids having to add a new registerPreventNestedTablesHandlers function.

Basically the same way you did in the test should work, from a useEffect to control the timing. Not a big deal either way

etrepum · 2025-11-11T06:06:46Z

packages/lexical-table/src/__tests__/unit/LexicalTableExtension.test.ts

+    editor.getEditorState().read(() => {
+      const root = $getRoot();
+      const table = root.getFirstChild();
+      if (!$isTableNode(table)) {


not that we are using vitest you can use its assert to do this sort of thing in a more concise way

etrepum · 2025-11-11T06:07:48Z

packages/lexical-table/src/index.ts

 export type {TableDOMCell} from './LexicalTableObserver';
 export {$getTableAndElementByKey, TableObserver} from './LexicalTableObserver';
 export {
+  registerPreventNestedTablesHandlers,


If we only support this feature in the extension we could not export this function at all and then its name wouldn't matter

etrepum · 2025-11-11T06:13:48Z

packages/lexical-table/src/LexicalTablePluginHelpers.ts

  });
 }

+export function registerPreventNestedTablesHandlers(


For the INSERT_TABLE_COMMAND handler this could also be implemented by passing the signal through to the insertTableCommandListener so that it can peek() the current value, which would prevent the need for registering a separate command listener at a higher priority to check this condition

zurfyx

Thank you James! I'll defer the stamp to @etrepum since he already started a proper review

james-atticus added 3 commits November 11, 2025 16:05

[lexical-table] Refactor: move tested table tests to extension test

9316946

[lexical-table] Feature: add config for allowing nested tables

d555465

[lexical-playground] Test: revive nested table E2E tests

ea2996f

meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 11, 2025

vercel bot deployed to Preview – lexical November 11, 2025 05:36 View deployment

vercel bot deployed to Preview – lexical-playground November 11, 2025 05:38 View deployment

james-atticus commented Nov 11, 2025

View reviewed changes

james-atticus marked this pull request as ready for review November 11, 2025 05:44

james-atticus requested review from acywatson, etrepum, fantactuka, ivailop7, potatowagon, takuyakanbr and zurfyx as code owners November 11, 2025 05:44

etrepum previously approved these changes Nov 11, 2025

View reviewed changes

etrepum added the extended-tests Run extended e2e tests on a PR label Nov 11, 2025

etrepum review

3b29bea

james-atticus dismissed etrepum’s stale review via 3b29bea November 11, 2025 06:58

vercel bot deployed to Preview – lexical November 11, 2025 06:59 View deployment

vercel bot deployed to Preview – lexical-playground November 11, 2025 07:00 View deployment

zurfyx reviewed Nov 11, 2025

View reviewed changes

etrepum approved these changes Nov 11, 2025

View reviewed changes

etrepum added this pull request to the merge queue Nov 11, 2025

Merged via the queue into facebook:main with commit 6296fe9 Nov 11, 2025
48 checks passed

etrepum mentioned this pull request Dec 10, 2025

v0.39.0 #8021

Merged

		const extension = getExtensionDependencyFromEditor(editor, TableExtension);
		extension.output.hasNestedTables.value = true;

Conversation

james-atticus commented Nov 11, 2025

Description

Uh oh!

vercel bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etrepum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zurfyx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel bot commented Nov 11, 2025 •

edited

Loading