Skip to content

[lexical-history] Bug Fix: SharedHistoryExtension forwards the parent maxDepth - #9044

Merged
etrepum merged 5 commits into
facebook:mainfrom
LeSingh1:history-shared-maxdepth
Aug 9, 2026
Merged

[lexical-history] Bug Fix: SharedHistoryExtension forwards the parent maxDepth#9044
etrepum merged 5 commits into
facebook:mainfrom
LeSingh1:history-shared-maxdepth

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

SharedHistoryExtension redirects a nested editor's history signals at the
parent's, so both editors share one undo stack. It forwards four of the five
writable signals:

batch(() => {
  output.delay.value = parentOutput.delay.value;
  output.historyState.value = parentOutput.historyState.value;
  output.now.value = parentOutput.now.value;
  // Note that toggling the parent history will force this to be changed
  output.disabled.value = parentOutput.disabled.value;
});                                   // maxDepth is never copied

maxDepth is a config-derived, writable Signal<number | null> on
HistoryExtensionOutput exactly like the others, and it caps the stack in
applyChange. The child's registerHistory therefore runs with the parent's
historyState but the child's own maxDepth, which defaults to null
no cap.

So every history event originating in the nested editor pushes onto the shared
undoStack without applying the limit the application configured on the
parent. An app that sets maxDepth to bound memory silently loses that bound
for anything typed in a nested editor (the playground's sticky notes use
SharedHistoryExtension).

The HistoryExtensionOutput docstring has the same omission — it enumerates
delay, disabled, historyState and now as the signals
SharedHistoryExtension redirects. maxDepth was added later and was missed
in both places, so this updates the doc alongside the code.

Test plan

New unit test packages/lexical-history/src/__tests__/unit/SharedHistoryMaxDepth.test.ts.
Deliberately a new file: PR #8953 (also mine) edits the HistoryExtension maxDepth block in LexicalHistory.test.tsx, and keeping these apart avoids a
self-conflict. The second case asserts the four signals that already forward —
it passes before and after, which is what pins the gap to maxDepth alone.

Before

$ npx vitest run packages/lexical-history/src/__tests__/unit/SharedHistoryMaxDepth.test.ts

     × the child adopts the parent maxDepth 7ms

AssertionError: expected null to be 7 // Object.is equality

      Tests  1 failed | 1 passed (2)

After

$ npx vitest run packages/lexical-history/src/__tests__/unit/SharedHistoryMaxDepth.test.ts

      Tests  2 passed (2)

… maxDepth

## Description

`SharedHistoryExtension` redirects a nested editor's history signals at the
parent's, so both editors share one undo stack. It forwards four of the five
writable signals:

```js
batch(() => {
  output.delay.value = parentOutput.delay.value;
  output.historyState.value = parentOutput.historyState.value;
  output.now.value = parentOutput.now.value;
  // Note that toggling the parent history will force this to be changed
  output.disabled.value = parentOutput.disabled.value;
});                                   // maxDepth is never copied
```

`maxDepth` is a config-derived, writable `Signal<number | null>` on
`HistoryExtensionOutput` exactly like the others, and it caps the stack in
`applyChange`. The child's `registerHistory` therefore runs with the *parent's*
`historyState` but the *child's* own `maxDepth`, which defaults to `null` —
no cap.

So every history event originating in the nested editor pushes onto the shared
`undoStack` without applying the limit the application configured on the
parent. An app that sets `maxDepth` to bound memory silently loses that bound
for anything typed in a nested editor (the playground's sticky notes use
`SharedHistoryExtension`).

The `HistoryExtensionOutput` docstring has the same omission — it enumerates
`delay`, `disabled`, `historyState` and `now` as the signals
`SharedHistoryExtension` redirects. `maxDepth` was added later and was missed
in both places, so this updates the doc alongside the code.

## Test plan

New unit test `packages/lexical-history/src/__tests__/unit/SharedHistoryMaxDepth.test.ts`.
Deliberately a new file: PR facebook#8953 (also mine) edits the `HistoryExtension
maxDepth` block in `LexicalHistory.test.tsx`, and keeping these apart avoids a
self-conflict. The second case asserts the four signals that already forward —
it passes before and after, which is what pins the gap to `maxDepth` alone.

### Before

```
$ npx vitest run packages/lexical-history/src/__tests__/unit/SharedHistoryMaxDepth.test.ts

     × the child adopts the parent maxDepth 7ms

AssertionError: expected null to be 7 // Object.is equality

      Tests  1 failed | 1 passed (2)
```

### After

```
$ npx vitest run packages/lexical-history/src/__tests__/unit/SharedHistoryMaxDepth.test.ts

      Tests  2 passed (2)
```
@vercel

vercel Bot commented Aug 9, 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.

@meta-cla 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 Aug 9, 2026
@mayrang

mayrang commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Confirmed the bug on main (null vs 7) and 27/27 tests pass on this branch. The fix is correct and matches the pattern of the four sibling signals. Two small notes inline.

Comment thread packages/lexical-history/src/index.ts Outdated
@etrepum
etrepum added this pull request to the merge queue Aug 9, 2026
Merged via the queue into facebook:main with commit b439fd6 Aug 9, 2026
40 of 42 checks passed
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.

3 participants