Repository navigation
Revert "Fix OverflowNode configuration" - #6535
Merged
Merged
Conversation
This reverts commit cb0b731.
etrepum
requested review from
Fetz,
Sahejkm,
acywatson,
fantactuka,
ivailop7,
potatowagon and
zurfyx
as code owners
August 21, 2024 00:35
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
size-limit report 📦
|
Collaborator
Author
|
Pretty sure that this test is flaky so I'm going to restart that job: |
Collaborator
Author
|
The tests passed the second time around, it must be flaky. |
etrepum
added a commit
to etrepum/lexical
that referenced
this pull request
Aug 21, 2024
potatowagon
approved these changes
Aug 21, 2024
potatowagon
left a comment
potatowagon
left a comment
Contributor
There was a problem hiding this comment.
hmm its not ideal that the e2e test signal is marked as skipped in the PR checks, with an overall passing signal. something seems to be up with the test workflows.
ivailop7
approved these changes
Aug 21, 2024
zurfyx
added a commit
that referenced
this pull request
Oct 23, 2026
…-list][lexical-overflow][lexical-rich-text][lexical-table][lexical-website] Feature: $config node structure (inline, placement, children, mergeAdjacent, derived) and an opt-in NodeStructureExtension
## Description
Lexical has no declarative way to say what an element may hold or when two
siblings are the same thing: the rules live in transforms and overridden
methods (`isInline()`, `ListNode.splice`, `$linkNodeTransform`,
`shouldMergeAdjacentLink`), so tools that need them (collaboration bindings,
importers) cannot read them.
This adds optional, general `$config()` keys, all experimental:
- `inline?: boolean`: a class that declares it and writes no `isInline()` gets
one returning it (injected at registration; `ElementNode.isInline` itself
is unchanged).
- `placement?: 'inline'`: an element that sits among inline content although
its `isInline()` is false (it keeps a block's caret, selection and deletion
behaviour). `OverflowNode`, which the character limit wraps around the
overflowing part of a paragraph's text, declares it, so a paragraph keeps it.
Nothing else changes (`isInline()`, selection, serialization).
- `children?: 'inline' | 'block' | Array<'inline' | Klass>`: what the class may
hold.
- `mergeAdjacent?: {by, method?} | false`: adjacent siblings of one class
merge when the named schema fields or StateConfigs are equal; `method`
names an overridable method that decides instead (`shouldMergeAdjacentLink`),
`false` turns off an inherited merge.
- `derived: true` on a schema field (`withField`/`withAccessors` option) or a
StateConfig: recomputed by Lexical, so neither named in `by` nor stored or
compared by tools. Serialization is unchanged.
`getDeclaredNodeStructure(klass)` reads them (per-key inheritance) and
`isDerivedNodeField` the derived flag; `$needsNodeChildrenNormalization` and
`$needsNodeStructureNormalization` tell whether the normalization would change
a node (read only). Declarations are checked at editor
creation in development (`by` naming an unknown or derived field, a declared
`inline` disagreeing with an overridden `isInline()`, a merging class listing
child classes that do not hold inline content). Only keys a class declares in
its own `$config()` are checked: an inherited declaration never throws, so
existing subclasses register as before (a `LinkNode` subclass whose
`isInline()` returns false, a `ListNode` subclass with an extra field and a
hand-written `exportJSON`, a `ParagraphNode` subclass whose `isInline()`
returns true). Where a subclass contradicts an inherited `inline` (its own
`isInline()` returns the opposite) or an inherited `mergeAdjacent` (its `by` is invalid for
the subclass), `getDeclaredNodeStructure` reads that key as undeclared for it,
so tools fall back to what the class says. `isInline()` is injected only on a
class that declares `inline` in its own `$config()`.
Lexical's own nodes declare what they already do: paragraph, heading, quote
and code `children: 'inline'`; link `inline`, `children: 'inline'`,
`mergeAdjacent: {by: [url, target, rel, title], method:
'shouldMergeAdjacentLink'}`; autolink `mergeAdjacent: false`; list
`children: [ListItemNode]`, `mergeAdjacent: {by: ['listType']}` (what
`mergeNextSiblingListIfSameType` compares); list item `children: ['inline',
ListNode]` with `value` and `indent` derived; table cell `children: 'block'`;
overflow `placement: 'inline'`.
Why a `placement` fact rather than an exemption: `OverflowNode` is structurally
inline content (it wraps part of a paragraph's text) but has kept
`isInline()` false since #6535 reverted making it inline, for its caret and
deletion behaviour. With `children: 'inline'` on ParagraphNode, enforcement
would split every paragraph that holds one (the character limit's unit and
e2e tests fail with enforcement on). An exemption ("leave me alone") would
say nothing about where the node belongs, so a `children: 'block'` holder
could not wrap it and a tool reading the declarations would still have to
special-case the class. `placement: 'inline'` states the content-model fact
and keeps the behavioural `isInline()` separate, so both readers (the
normalization, a binding's storage) get the same answer from one declaration.
Escape hatches: `mergeAdjacent.method` (an overridable method decides),
`mergeAdjacent: false` (turn off an inherited merge), `placement: 'inline'`,
shadow root instances (always hold blocks), per-instance `isInline()`
(nothing is injected without an `inline` declaration), and classes that
declare nothing are never touched.
Docs: the serialization guide's "Node structure" section (a `$config`
example, the keys, opting in with `NodeStructureExtension`, the dev checks
and the inherited-declaration rule, links to the API reference for the
normalization helpers), and an entry for `NodeStructureExtension` on the
included extensions page.
Enforcement is opt-in: `NodeStructureExtension` (`@lexical/extension`, and
`registerNodeStructure(editor)` for editors built without extensions)
registers one transform on every class that declares `children` or
`mergeAdjacent`: a block in a node holding inline content is lifted out (the
node is split around it), inline content in a node holding blocks is wrapped
in a paragraph, a child a list does not allow is wrapped in its first listed
class (or lifted out when that class cannot hold it either), and equal
adjacent siblings merge. Repairs only move content and are a pure function of
the document. Without the extension nothing changes at runtime: JSON is
byte-identical and every existing unit test passes unchanged.
## Test plan
`NodeStructureExtension.test.ts` also registers existing subclasses that
inherit a declaration, each without throwing: a `LinkNode` subclass whose
`isInline()` returns false (its inherited `inline` reads as undeclared), a
`ListNode` subclass with an extra field and a hand-written `exportJSON`
(round trip), and a `ParagraphNode` subclass whose `isInline()` returns
true. A subclass that declares `inline` itself is still refused when its
`isInline()` disagrees, and `isInline()` is synthesized only on a class that
declares `inline` in its own `$config()`.
### Before
```
$ npx vitest --project unit --no-watch packages/lexical-extension/src/__tests__/unit/NodeStructureExtension.test.ts
TypeError: getDeclaredNodeStructure is not a function
TypeError: isDerivedNodeField is not a function
Tests 25 failed | 2 passed (27)
# The previous revision of this PR validated inherited declarations too:
× a LinkNode subclass whose isInline() returns false
Error: BlockLinkNode: $config inline is true but isInline() returns false
× isInline() is synthesized only for a class that declares inline itself
Tests 2 failed | 25 passed (27)
# CharacterLimit with the normalization forced on every editor (evidence
# patch, not committed), before `placement`:
$ LEXICAL_FORCE_NODE_STRUCTURE=1 npx vitest --project unit --no-watch \
packages/lexical-react/src/__tests__/unit/LexicalCharacterLimitPlugin.test.tsx \
packages/lexical-react/src/__tests__/unit/useLexicalCharacterLimit.test.ts
Tests 7 failed | 3 passed (10)
```
### After
```
$ npx vitest --project unit --no-watch packages/lexical-extension/src/__tests__/unit/NodeStructureExtension.test.ts
Tests 27 passed (27)
$ LEXICAL_FORCE_NODE_STRUCTURE=1 npx vitest --project unit --no-watch \
packages/lexical-react/src/__tests__/unit/LexicalCharacterLimitPlugin.test.tsx \
packages/lexical-react/src/__tests__/unit/useLexicalCharacterLimit.test.ts
Tests 10 passed (10)
$ pnpm run test-unit (unit + scripts-unit, per-test comparison with main)
main: 392 files, 9537 passed, 1 skipped; this branch: 393 files, 9582
passed, 1 skipped: all 9537 of main's tests pass unchanged, plus 45 new (27
NodeStructureExtension, 18 build/tree-shaking/Flow-subpath entries for the
new extension entry)
```
`pnpm run tsc`, `pnpm run flow` (0 errors), eslint and prettier pass; the
tree-shaking test passes. Without the extension, JSON output is
byte-identical to main for a document with every core node (compact and
full), and Chromium e2e (rich text and collab) has identical pass and skip
counts. Real browsers were not exercised with the extension on by this
commit's tests.
zurfyx
added a commit
that referenced
this pull request
Oct 23, 2026
…-list][lexical-overflow][lexical-rich-text][lexical-table][lexical-website] Feature: $config node structure (inline, placement, children, mergeAdjacent, derived) and an opt-in NodeStructureExtension
## Description
Lexical has no declarative way to say what an element may hold or when two
siblings are the same thing: the rules live in transforms and overridden
methods (`isInline()`, `ListNode.splice`, `$linkNodeTransform`,
`shouldMergeAdjacentLink`), so tools that need them (collaboration bindings,
importers) cannot read them.
This adds optional, general `$config()` keys, all experimental:
- `inline?: boolean`: a class that declares it and writes no `isInline()` gets
one returning it (injected at registration; `ElementNode.isInline` itself
is unchanged).
- `placement?: 'inline'`: an element that sits among inline content although
its `isInline()` is false (it keeps a block's caret, selection and deletion
behaviour). `OverflowNode`, which the character limit wraps around the
overflowing part of a paragraph's text, declares it, so a paragraph keeps it.
Nothing else changes (`isInline()`, selection, serialization).
- `children?: 'inline' | 'block' | Array<'inline' | Klass>`: what the class may
hold.
- `mergeAdjacent?: {by, method?} | false`: adjacent siblings of one class
merge when the named schema fields or StateConfigs are equal; `method`
names an overridable method that decides instead (`shouldMergeAdjacentLink`),
`false` turns off an inherited merge.
- `derived: true` on a schema field (`withField`/`withAccessors` option) or a
StateConfig: recomputed by Lexical, so neither named in `by` nor stored or
compared by tools. Serialization is unchanged.
`getDeclaredNodeStructure(klass)` reads them (per-key inheritance) and
`isDerivedNodeField` the derived flag; `$needsNodeChildrenNormalization` and
`$needsNodeStructureNormalization` tell whether the normalization would change
a node (read only). Declarations are checked at editor
creation in development (`by` naming an unknown or derived field, a declared
`inline` disagreeing with an overridden `isInline()`, a merging class listing
child classes that do not hold inline content). Only keys a class declares in
its own `$config()` are checked: an inherited declaration never throws, so
existing subclasses register as before (a `LinkNode` subclass whose
`isInline()` returns false, a `ListNode` subclass with an extra field and a
hand-written `exportJSON`, a `ParagraphNode` subclass whose `isInline()`
returns true). Where a subclass contradicts an inherited `inline` (its own
`isInline()` returns the opposite) or an inherited `mergeAdjacent` (its `by` is invalid for
the subclass), `getDeclaredNodeStructure` reads that key as undeclared for it,
so tools fall back to what the class says. `isInline()` is injected only on a
class that declares `inline` in its own `$config()`.
Lexical's own nodes declare what they already do: paragraph, heading, quote
and code `children: 'inline'`; link `inline`, `children: 'inline'`,
`mergeAdjacent: {by: [url, target, rel, title], method:
'shouldMergeAdjacentLink'}`; autolink `mergeAdjacent: false`; list
`children: [ListItemNode]`, `mergeAdjacent: {by: ['listType']}` (what
`mergeNextSiblingListIfSameType` compares); list item `children: ['inline',
ListNode]` with `value` and `indent` derived; table cell `children: 'block'`;
overflow `placement: 'inline'`.
Why a `placement` fact rather than an exemption: `OverflowNode` is structurally
inline content (it wraps part of a paragraph's text) but has kept
`isInline()` false since #6535 reverted making it inline, for its caret and
deletion behaviour. With `children: 'inline'` on ParagraphNode, enforcement
would split every paragraph that holds one (the character limit's unit and
e2e tests fail with enforcement on). An exemption ("leave me alone") would
say nothing about where the node belongs, so a `children: 'block'` holder
could not wrap it and a tool reading the declarations would still have to
special-case the class. `placement: 'inline'` states the content-model fact
and keeps the behavioural `isInline()` separate, so both readers (the
normalization, a binding's storage) get the same answer from one declaration.
Escape hatches: `mergeAdjacent.method` (an overridable method decides),
`mergeAdjacent: false` (turn off an inherited merge), `placement: 'inline'`,
shadow root instances (always hold blocks), per-instance `isInline()`
(nothing is injected without an `inline` declaration), and classes that
declare nothing are never touched.
Docs: the serialization guide's "Node structure" section (a `$config`
example, the keys, opting in with `NodeStructureExtension`, the dev checks
and the inherited-declaration rule, links to the API reference for the
normalization helpers), and an entry for `NodeStructureExtension` on the
included extensions page.
Enforcement is opt-in: `NodeStructureExtension` (`@lexical/extension`, and
`registerNodeStructure(editor)` for editors built without extensions)
registers one transform on every class that declares `children` or
`mergeAdjacent`: a block in a node holding inline content is lifted out (the
node is split around it), inline content in a node holding blocks is wrapped
in a paragraph, a child a list does not allow is wrapped in its first listed
class (or lifted out when that class cannot hold it either), and equal
adjacent siblings merge. Repairs only move content and are a pure function of
the document. Without the extension nothing changes at runtime: JSON is
byte-identical and every existing unit test passes unchanged.
## Test plan
`NodeStructureExtension.test.ts` also registers existing subclasses that
inherit a declaration, each without throwing: a `LinkNode` subclass whose
`isInline()` returns false (its inherited `inline` reads as undeclared), a
`ListNode` subclass with an extra field and a hand-written `exportJSON`
(round trip), and a `ParagraphNode` subclass whose `isInline()` returns
true. A subclass that declares `inline` itself is still refused when its
`isInline()` disagrees, and `isInline()` is synthesized only on a class that
declares `inline` in its own `$config()`.
### Before
```
$ npx vitest --project unit --no-watch packages/lexical-extension/src/__tests__/unit/NodeStructureExtension.test.ts
TypeError: getDeclaredNodeStructure is not a function
TypeError: isDerivedNodeField is not a function
Tests 25 failed | 2 passed (27)
# The previous revision of this PR validated inherited declarations too:
× a LinkNode subclass whose isInline() returns false
Error: BlockLinkNode: $config inline is true but isInline() returns false
× isInline() is synthesized only for a class that declares inline itself
Tests 2 failed | 25 passed (27)
# CharacterLimit with the normalization forced on every editor (evidence
# patch, not committed), before `placement`:
$ LEXICAL_FORCE_NODE_STRUCTURE=1 npx vitest --project unit --no-watch \
packages/lexical-react/src/__tests__/unit/LexicalCharacterLimitPlugin.test.tsx \
packages/lexical-react/src/__tests__/unit/useLexicalCharacterLimit.test.ts
Tests 7 failed | 3 passed (10)
```
### After
```
$ npx vitest --project unit --no-watch packages/lexical-extension/src/__tests__/unit/NodeStructureExtension.test.ts
Tests 27 passed (27)
$ LEXICAL_FORCE_NODE_STRUCTURE=1 npx vitest --project unit --no-watch \
packages/lexical-react/src/__tests__/unit/LexicalCharacterLimitPlugin.test.tsx \
packages/lexical-react/src/__tests__/unit/useLexicalCharacterLimit.test.ts
Tests 10 passed (10)
$ pnpm run test-unit (unit + scripts-unit, per-test comparison with main)
main: 392 files, 9537 passed, 1 skipped; this branch: 393 files, 9582
passed, 1 skipped: all 9537 of main's tests pass unchanged, plus 45 new (27
NodeStructureExtension, 18 build/tree-shaking/Flow-subpath entries for the
new extension entry)
```
`pnpm run tsc`, `pnpm run flow` (0 errors), eslint and prettier pass; the
tree-shaking test passes. Without the extension, JSON output is
byte-identical to main for a document with every core node (compact and
full), and Chromium e2e (rich text and collab) has identical pass and skip
counts. Real browsers were not exercised with the extension on by this
commit's tests.
Vedthakar
added a commit
to Vedthakar/lexical
that referenced
this pull request
Oct 25, 2026
… inline without the line break caret regression OverflowNode.isInline() now returns true (meta#6027, reverted in meta#6535). The revert was caused by a trailing LineBreakNode inside the now inline OverflowNode: the browser can't put the caret after a <br> that ends an inline element, so the next character landed before the line break. - The reconciler gives a block the managed trailing <br> when its last child is an inline element that itself ends with a LineBreakNode. - $wrapOverflowedNodes moves a caret left at the end of an OverflowNode ending with a line break to the same spot just after the OverflowNode. Closes meta#9454 Signed-off-by: Ved Thakar <111605341+Vedthakar@users.noreply.github.com> Co-authored-by: Ved Thakar <111605341+Vedthakar@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #6027
OverflowNode causes problems with LineBreakNode with this configuration change, causes the tests to fail in plain text mode (the insertion point always ends up before the trailing LineBreakNode) but the same strange behavior comes up if you press shift-return to insert a LineBreakNode into a ParagraphNode in rich text mode. I think we will need to revisit #6027 with additional tests.
Closes #6534
/cc @zurfyx @ivailop7