Skip to content

[lexical-list] Bug Fix: list operations and DOM import/export keep the node's own state - #9050

Open
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:consol/list-node-state
Open

[lexical-list] Bug Fix: list operations and DOM import/export keep the node's own state#9050
LeSingh1 wants to merge 1 commit into
facebook:mainfrom
LeSingh1:consol/list-node-state

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

Description

@lexical/list rebuilds a ListNode or a ListItemNode in a lot of places —
converting a list to another type, removing a list, splitting one around an
inserted block, merging two adjacent lists, splicing a block in, importing and
exporting HTML. In each of these the replacement is supposed to stand in for the
node it replaces, but several of them build it from scratch and drop what the
original carried: its element state, its listType, its numbering, its block
formatting, or its dir. One import path also hand-rolls a ListItemNode
instead of asking the ListNode subclass for one.

Each fix keeps the replacement equivalent to the node it stands in for:

  • $insertList (formatList.ts) converted an existing list by building a
    default-constructed $createListNode(listType) and moving the items across,
    so the replacement lost the list's direction, format, style, indent
    and start — toggling an RTL list from bullets to numbers re-rendered it
    left-to-right, and <ol start="4"> restarted at 1. A $newListFrom helper
    now $copyNodes the list being replaced and sets the new type, at both sites
    in $insertList that replace a ListNode. This is what every other
    ListNode-for-ListNode swap in the package ($handleIndent,
    $handleOutdent, ListItemNode.replace, …) already does.

  • $removeList (formatList.ts) is the inverse of $createListOrMerge, which
    copies a block's format and indent onto the list item it creates. It built
    each replacement paragraph from the selection's text style alone, so the
    item's own format, indent, direction and style were dropped. It now
    carries all four onto the paragraph; indent in particular is the only thing
    that reproduces a nested item's rendering once the list is gone.

  • mergeLists (formatList.ts) and ListItemNode.remove
    (LexicalListItemNode.ts) merged the sublists at the join point
    unconditionally. mergeNextSiblingListIfSameType compares listType at the
    top level, but neither of these did, so a nested <ol> adjacent to a nested
    <ul> lost its type. Both now compare listType before joining.

  • ListItemNode.insertAfter (LexicalListItemNode.ts) splits its list when the
    inserted node is not a ListItemNode — the page-break / horizontal-rule /
    image case. The trailing items move into $copyNode(listNode), which carries
    the original list's start, so the ListNode $transform renumbered them
    from there and the list visibly restarted at 1. It now derives the new start
    from the first item that moves, via the existing $getNewListStart helper
    that $handleListInsertParagraph already uses for the same split. Scoped to
    listType === 'number', so bullet and check lists keep the start they copy
    today. Refs Bug: Numbered List Resets After Page Break in Lexical Editor #7032.

  • ListNode.splice (LexicalListNode.ts) wraps a non-ListItemNode in a list
    item, and for a block-level node it replaced the block with
    $createTextNode(node.getTextContent()) — a string, so every text format and
    style was discarded and inline nodes were flattened (a paragraph holding bold
    text and a link became one unformatted TextNode with the link gone). It now
    unwraps the block into its own children, the same conversion
    $createListOrMerge and ListItemNode.append already perform. Non-block and
    inline nodes take the same path as before and a ListNode child is still left
    alone.

  • ListNode.exportDOM (LexicalListNode.ts) builds its element with
    createDOM and returns it without going through super.exportDOM, which is
    where ElementNode emits element.dir from getDirection(). Both importers
    read dir back off the <ol>/<ul> ($convertListNode and ListRule), and
    ListItemNode.exportDOM right next to it does emit dir, so an RTL list
    exported as <ul><li dir="rtl">…</li></ul> — the item kept its direction and
    the list that owns it silently lost it. It now emits dir when the node has
    an explicit direction; a node with no explicit direction is unaffected, so
    existing output is unchanged.

  • $normalizeListChildren (ListImportExtension.ts) is the DOMImportExtension
    port of the legacy $normalizeChildren. The legacy one synthesizes items with
    listNode.createListItemNode.bind(listNode) — the subclass hook added in
    [lexical-list] Feature: Add the createListItemNode method to the ListNode and use it for children normalization #8427 — and so does ListNode.splice, but this one called the module-level
    $createListItemNode(), so a subclassed list imported through the new
    pipeline got plain ListItemNode wrappers for both of its synthesized-item
    cases. The ListNode being built is now threaded through and
    listNode.createListItemNode() is used. The default implementation returns
    $createListItemNode(), so nothing changes for unsubclassed lists.

Test plan

13 new unit test cases in packages/lexical-list/src/__tests__/unit/ — added to
formatList.test.ts, LexicalListItemNode.test.ts, LexicalListNode.test.ts
and ListImportExtension.test.ts, plus four focused files
(InsertListKeepsListState.test.ts, RemoveListKeepsItemState.test.ts,
Issue7032Repro.test.ts, ListSpliceKeepsFormatting.test.ts). 12 of them fail
on main; the 13th is a guard that a bullet list still keeps its start when
it is split. No existing test expectation was changed.

Before

(source changes reverted, new tests kept)

$ npx vitest run --project unit packages/lexical-list
     × wrapper items come from ListNode.createListItemNode() 53ms
     × does not merge nested sublists of a different listType 108ms
     × ListNode.exportDOM() round-trips the dir attribute 56ms
       × both siblings are nested with a different listType 68ms
     × the paragraphs keep the items format, indent, direction and style 247ms
     × a formatted paragraph round-trips through a list 3ms
     × keeps the text formats of the block it unwraps 129ms
     × keeps an inline node of the block it unwraps 22ms
     × converting a populated list keeps its direction, format and start 22ms
     × converting from an empty list item keeps the list state 2ms
     × inserting a block between list items continues the numbering 30ms
     × a list that does not start at 1 continues from the split point 5ms

 Test Files  8 failed | 6 passed (14)
      Tests  12 failed | 126 passed (138)

After

$ npx vitest run --project unit packages/lexical-list

 Test Files  14 passed (14)
      Tests  138 passed (138)

$ npx vitest run --project unit packages/lexical-markdown packages/lexical-mdast packages/lexical-clipboard packages/lexical-html

 Test Files  22 passed (22)
      Tests  710 passed (710)

$ npx vitest run --project unit packages/lexical-react

 Test Files  30 passed (30)
      Tests  181 passed (181)

$ npx tsc --noEmit -p .
(clean, no output)

Supersedes #8925, #9001, #9002, #9021, #9027, #9031, #9034, consolidated per the
review feedback on #9027 and #9035.

…e node's own state

## Description

`@lexical/list` rebuilds a `ListNode` or a `ListItemNode` in a lot of places —
converting a list to another type, removing a list, splitting one around an
inserted block, merging two adjacent lists, splicing a block in, importing and
exporting HTML. In each of these the replacement is supposed to stand in for the
node it replaces, but several of them build it from scratch and drop what the
original carried: its element state, its `listType`, its numbering, its block
formatting, or its `dir`. One import path also hand-rolls a `ListItemNode`
instead of asking the `ListNode` subclass for one.

Each fix keeps the replacement equivalent to the node it stands in for:

- `$insertList` (`formatList.ts`) converted an existing list by building a
  default-constructed `$createListNode(listType)` and moving the items across,
  so the replacement lost the list's `direction`, `format`, `style`, `indent`
  and `start` — toggling an RTL list from bullets to numbers re-rendered it
  left-to-right, and `<ol start="4">` restarted at 1. A `$newListFrom` helper
  now `$copyNode`s the list being replaced and sets the new type, at both sites
  in `$insertList` that replace a `ListNode`. This is what every other
  `ListNode`-for-`ListNode` swap in the package (`$handleIndent`,
  `$handleOutdent`, `ListItemNode.replace`, …) already does.

- `$removeList` (`formatList.ts`) is the inverse of `$createListOrMerge`, which
  copies a block's `format` and `indent` onto the list item it creates. It built
  each replacement paragraph from the selection's text style alone, so the
  item's own `format`, `indent`, `direction` and `style` were dropped. It now
  carries all four onto the paragraph; `indent` in particular is the only thing
  that reproduces a nested item's rendering once the list is gone.

- `mergeLists` (`formatList.ts`) and `ListItemNode.remove`
  (`LexicalListItemNode.ts`) merged the sublists at the join point
  unconditionally. `mergeNextSiblingListIfSameType` compares `listType` at the
  top level, but neither of these did, so a nested `<ol>` adjacent to a nested
  `<ul>` lost its type. Both now compare `listType` before joining.

- `ListItemNode.insertAfter` (`LexicalListItemNode.ts`) splits its list when the
  inserted node is not a `ListItemNode` — the page-break / horizontal-rule /
  image case. The trailing items move into `$copyNode(listNode)`, which carries
  the *original* list's `start`, so the `ListNode` `$transform` renumbered them
  from there and the list visibly restarted at 1. It now derives the new start
  from the first item that moves, via the existing `$getNewListStart` helper
  that `$handleListInsertParagraph` already uses for the same split. Scoped to
  `listType === 'number'`, so bullet and check lists keep the `start` they copy
  today. Refs facebook#7032.

- `ListNode.splice` (`LexicalListNode.ts`) wraps a non-`ListItemNode` in a list
  item, and for a block-level node it replaced the block with
  `$createTextNode(node.getTextContent())` — a string, so every text format and
  style was discarded and inline nodes were flattened (a paragraph holding bold
  text and a link became one unformatted `TextNode` with the link gone). It now
  unwraps the block into its own children, the same conversion
  `$createListOrMerge` and `ListItemNode.append` already perform. Non-block and
  inline nodes take the same path as before and a `ListNode` child is still left
  alone.

- `ListNode.exportDOM` (`LexicalListNode.ts`) builds its element with
  `createDOM` and returns it without going through `super.exportDOM`, which is
  where `ElementNode` emits `element.dir` from `getDirection()`. Both importers
  read `dir` back off the `<ol>`/`<ul>` (`$convertListNode` and `ListRule`), and
  `ListItemNode.exportDOM` right next to it does emit `dir`, so an RTL list
  exported as `<ul><li dir="rtl">…</li></ul>` — the item kept its direction and
  the list that owns it silently lost it. It now emits `dir` when the node has
  an explicit direction; a node with no explicit direction is unaffected, so
  existing output is unchanged.

- `$normalizeListChildren` (`ListImportExtension.ts`) is the `DOMImportExtension`
  port of the legacy `$normalizeChildren`. The legacy one synthesizes items with
  `listNode.createListItemNode.bind(listNode)` — the subclass hook added in
  facebook#8427 — and so does `ListNode.splice`, but this one called the module-level
  `$createListItemNode()`, so a subclassed list imported through the new
  pipeline got plain `ListItemNode` wrappers for both of its synthesized-item
  cases. The `ListNode` being built is now threaded through and
  `listNode.createListItemNode()` is used. The default implementation returns
  `$createListItemNode()`, so nothing changes for unsubclassed lists.

## Test plan

13 new unit test cases in `packages/lexical-list/src/__tests__/unit/` — added to
`formatList.test.ts`, `LexicalListItemNode.test.ts`, `LexicalListNode.test.ts`
and `ListImportExtension.test.ts`, plus four focused files
(`InsertListKeepsListState.test.ts`, `RemoveListKeepsItemState.test.ts`,
`Issue7032Repro.test.ts`, `ListSpliceKeepsFormatting.test.ts`). 12 of them fail
on `main`; the 13th is a guard that a bullet list still keeps its `start` when
it is split. No existing test expectation was changed.

### Before

(source changes reverted, new tests kept)

```
$ npx vitest run --project unit packages/lexical-list
     × wrapper items come from ListNode.createListItemNode() 53ms
     × does not merge nested sublists of a different listType 108ms
     × ListNode.exportDOM() round-trips the dir attribute 56ms
       × both siblings are nested with a different listType 68ms
     × the paragraphs keep the items format, indent, direction and style 247ms
     × a formatted paragraph round-trips through a list 3ms
     × keeps the text formats of the block it unwraps 129ms
     × keeps an inline node of the block it unwraps 22ms
     × converting a populated list keeps its direction, format and start 22ms
     × converting from an empty list item keeps the list state 2ms
     × inserting a block between list items continues the numbering 30ms
     × a list that does not start at 1 continues from the split point 5ms

 Test Files  8 failed | 6 passed (14)
      Tests  12 failed | 126 passed (138)
```

### After

```
$ npx vitest run --project unit packages/lexical-list

 Test Files  14 passed (14)
      Tests  138 passed (138)

$ npx vitest run --project unit packages/lexical-markdown packages/lexical-mdast packages/lexical-clipboard packages/lexical-html

 Test Files  22 passed (22)
      Tests  710 passed (710)

$ npx vitest run --project unit packages/lexical-react

 Test Files  30 passed (30)
      Tests  181 passed (181)

$ npx tsc --noEmit -p .
(clean, no output)
```

Supersedes facebook#8925, facebook#9001, facebook#9002, facebook#9021, facebook#9027, facebook#9031, facebook#9034, consolidated per the
review feedback on facebook#9027 and facebook#9035.
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

@LeSingh1 is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

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

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant