From aa3dada9a17bd903bfccb835c153b565c904bbd5 Mon Sep 17 00:00:00 2001 From: Suyash Date: Sun, 24 May 2026 23:04:44 +0530 Subject: [PATCH 1/4] fix: wrap withMerging, withNewBatch, and withoutMerging in try/finally to ensure state cleanup on error --- packages/slate-history/src/history-editor.ts | 23 ++++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/slate-history/src/history-editor.ts b/packages/slate-history/src/history-editor.ts index 264ec11354..b9c831b09c 100644 --- a/packages/slate-history/src/history-editor.ts +++ b/packages/slate-history/src/history-editor.ts @@ -82,8 +82,11 @@ export const HistoryEditor = { withMerging(editor: HistoryEditor, fn: () => void): void { const prev = HistoryEditor.isMerging(editor) MERGING.set(editor, true) - fn() - MERGING.set(editor, prev) + try { + fn() + } finally { + MERGING.set(editor, prev) + } }, /** @@ -95,9 +98,12 @@ export const HistoryEditor = { const prev = HistoryEditor.isMerging(editor) MERGING.set(editor, true) SPLITTING_ONCE.set(editor, true) - fn() - MERGING.set(editor, prev) - SPLITTING_ONCE.delete(editor) + try { + fn() + } finally { + MERGING.set(editor, prev) + SPLITTING_ONCE.delete(editor) + } }, /** @@ -108,8 +114,11 @@ export const HistoryEditor = { withoutMerging(editor: HistoryEditor, fn: () => void): void { const prev = HistoryEditor.isMerging(editor) MERGING.set(editor, false) - fn() - MERGING.set(editor, prev) + try { + fn() + } finally { + MERGING.set(editor, prev) + } }, /** From 0d02a9f25714fa2de049b6d9334d49add324aa3d Mon Sep 17 00:00:00 2001 From: Suyash Date: Sun, 24 May 2026 23:52:20 +0530 Subject: [PATCH 2/4] chore: added changeset --- .changeset/silver-scissors-attack.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/silver-scissors-attack.md diff --git a/.changeset/silver-scissors-attack.md b/.changeset/silver-scissors-attack.md new file mode 100644 index 0000000000..45a55b38cc --- /dev/null +++ b/.changeset/silver-scissors-attack.md @@ -0,0 +1,5 @@ +--- +'slate-history': patch +--- + +fix: wrap withMerging, withNewBatch, and withoutMerging in try/finally to ensure state cleanup on error From 37b2a9de9185b10df5a9685b670da0fc9a0d1912 Mon Sep 17 00:00:00 2001 From: Suyash Date: Sun, 31 May 2026 05:54:12 -0700 Subject: [PATCH 3/4] chore: resolved comment --- packages/slate-history/src/history-editor.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/slate-history/src/history-editor.ts b/packages/slate-history/src/history-editor.ts index b9c831b09c..2b2fc61bc7 100644 --- a/packages/slate-history/src/history-editor.ts +++ b/packages/slate-history/src/history-editor.ts @@ -96,13 +96,14 @@ export const HistoryEditor = { */ withNewBatch(editor: HistoryEditor, fn: () => void): void { const prev = HistoryEditor.isMerging(editor) + const prevSplitting = HistoryEditor.isSplittingOnce(editor) MERGING.set(editor, true) SPLITTING_ONCE.set(editor, true) try { fn() } finally { MERGING.set(editor, prev) - SPLITTING_ONCE.delete(editor) + SPLITTING_ONCE.set(editor, prevSplitting) } }, From 154d2e577d64aab6faac7002883731a8aa79b8de Mon Sep 17 00:00:00 2001 From: Suyash Date: Fri, 21 Aug 2026 15:09:18 +0530 Subject: [PATCH 4/4] chore: resolved comment --- .changeset/silver-scissors-attack.md | 2 +- packages/slate-history/src/history-editor.ts | 8 ++-- packages/slate-history/test/history-editor.ts | 42 +++++++++++++++++++ .../test/undo/with_new_batch/nested-empty.tsx | 27 ++++++++++++ .../undo/with_new_batch/nested-throws.tsx | 32 ++++++++++++++ .../test/undo/with_new_batch/nested.tsx | 29 +++++++++++++ .../test/undo/with_new_batch/throws.tsx | 24 +++++++++++ .../test/undo/without_merging/throws.tsx | 24 +++++++++++ 8 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 packages/slate-history/test/history-editor.ts create mode 100644 packages/slate-history/test/undo/with_new_batch/nested-empty.tsx create mode 100644 packages/slate-history/test/undo/with_new_batch/nested-throws.tsx create mode 100644 packages/slate-history/test/undo/with_new_batch/nested.tsx create mode 100644 packages/slate-history/test/undo/with_new_batch/throws.tsx create mode 100644 packages/slate-history/test/undo/without_merging/throws.tsx diff --git a/.changeset/silver-scissors-attack.md b/.changeset/silver-scissors-attack.md index 45a55b38cc..d9959f965f 100644 --- a/.changeset/silver-scissors-attack.md +++ b/.changeset/silver-scissors-attack.md @@ -2,4 +2,4 @@ 'slate-history': patch --- -fix: wrap withMerging, withNewBatch, and withoutMerging in try/finally to ensure state cleanup on error +Fix `HistoryEditor.withMerging`, `HistoryEditor.withNewBatch` and `HistoryEditor.withoutMerging` leaving the history flags corrupted when `fn` throws, and a nested `withNewBatch` that applies no operation clearing the pending split of the enclosing `withNewBatch`. diff --git a/packages/slate-history/src/history-editor.ts b/packages/slate-history/src/history-editor.ts index 2b2fc61bc7..85707f1d31 100644 --- a/packages/slate-history/src/history-editor.ts +++ b/packages/slate-history/src/history-editor.ts @@ -95,15 +95,17 @@ export const HistoryEditor = { * merged as usual. */ withNewBatch(editor: HistoryEditor, fn: () => void): void { - const prev = HistoryEditor.isMerging(editor) + const prevMerging = HistoryEditor.isMerging(editor) const prevSplitting = HistoryEditor.isSplittingOnce(editor) MERGING.set(editor, true) SPLITTING_ONCE.set(editor, true) try { fn() } finally { - MERGING.set(editor, prev) - SPLITTING_ONCE.set(editor, prevSplitting) + MERGING.set(editor, prevMerging) + if (!prevSplitting) { + SPLITTING_ONCE.delete(editor) + } } }, diff --git a/packages/slate-history/test/history-editor.ts b/packages/slate-history/test/history-editor.ts new file mode 100644 index 0000000000..84ded1ccad --- /dev/null +++ b/packages/slate-history/test/history-editor.ts @@ -0,0 +1,42 @@ +import assert from 'assert' +import { createEditor } from 'slate' +import { HistoryEditor, withHistory } from '..' + +const boom = () => { + throw new Error('boom') +} + +describe('HistoryEditor', () => { + describe('withMerging', () => { + it('restores the merging flag when fn throws', () => { + const editor = withHistory(createEditor()) + assert.throws(() => HistoryEditor.withMerging(editor, boom), /boom/) + assert.strictEqual(HistoryEditor.isMerging(editor), undefined) + }) + }) + + describe('withNewBatch', () => { + it('restores the merging and splitting flags when fn throws', () => { + const editor = withHistory(createEditor()) + assert.throws(() => HistoryEditor.withNewBatch(editor, boom), /boom/) + assert.strictEqual(HistoryEditor.isMerging(editor), undefined) + assert.strictEqual(HistoryEditor.isSplittingOnce(editor), undefined) + }) + }) + + describe('withoutMerging', () => { + it('restores the merging flag when fn throws', () => { + const editor = withHistory(createEditor()) + assert.throws(() => HistoryEditor.withoutMerging(editor, boom), /boom/) + assert.strictEqual(HistoryEditor.isMerging(editor), undefined) + }) + }) + + describe('withoutSaving', () => { + it('restores the saving flag when fn throws', () => { + const editor = withHistory(createEditor()) + assert.throws(() => HistoryEditor.withoutSaving(editor, boom), /boom/) + assert.strictEqual(HistoryEditor.isSaving(editor), undefined) + }) + }) +}) diff --git a/packages/slate-history/test/undo/with_new_batch/nested-empty.tsx b/packages/slate-history/test/undo/with_new_batch/nested-empty.tsx new file mode 100644 index 0000000000..1b5373bf97 --- /dev/null +++ b/packages/slate-history/test/undo/with_new_batch/nested-empty.tsx @@ -0,0 +1,27 @@ +/** @jsx jsx */ +import { jsx } from '../..' +import { HistoryEditor } from '../../..' + +export const run = editor => { + editor.insertText('x') + HistoryEditor.withNewBatch(editor, () => { + HistoryEditor.withNewBatch(editor, () => {}) + editor.insertText('y') + }) +} +export const input = ( + + + one + + + +) +export const output = ( + + + onex + + + +) diff --git a/packages/slate-history/test/undo/with_new_batch/nested-throws.tsx b/packages/slate-history/test/undo/with_new_batch/nested-throws.tsx new file mode 100644 index 0000000000..299aba586b --- /dev/null +++ b/packages/slate-history/test/undo/with_new_batch/nested-throws.tsx @@ -0,0 +1,32 @@ +/** @jsx jsx */ +import assert from 'assert' +import { jsx } from '../..' +import { HistoryEditor } from '../../..' + +export const run = editor => { + editor.insertText('x') + HistoryEditor.withNewBatch(editor, () => { + assert.throws(() => { + HistoryEditor.withNewBatch(editor, () => { + throw new Error('boom') + }) + }, /boom/) + editor.insertText('y') + }) +} +export const input = ( + + + one + + + +) +export const output = ( + + + onex + + + +) diff --git a/packages/slate-history/test/undo/with_new_batch/nested.tsx b/packages/slate-history/test/undo/with_new_batch/nested.tsx new file mode 100644 index 0000000000..3d5d9cb8d6 --- /dev/null +++ b/packages/slate-history/test/undo/with_new_batch/nested.tsx @@ -0,0 +1,29 @@ +/** @jsx jsx */ +import { jsx } from '../..' +import { HistoryEditor } from '../../..' + +export const run = editor => { + editor.insertText('x') + HistoryEditor.withNewBatch(editor, () => { + HistoryEditor.withNewBatch(editor, () => { + editor.insertText('a') + }) + editor.insertText('b') + }) +} +export const input = ( + + + one + + + +) +export const output = ( + + + onex + + + +) diff --git a/packages/slate-history/test/undo/with_new_batch/throws.tsx b/packages/slate-history/test/undo/with_new_batch/throws.tsx new file mode 100644 index 0000000000..c8ec302543 --- /dev/null +++ b/packages/slate-history/test/undo/with_new_batch/throws.tsx @@ -0,0 +1,24 @@ +/** @jsx jsx */ +import assert from 'assert' +import { jsx } from '../..' +import { HistoryEditor } from '../../..' +import { cloneDeep } from 'lodash' + +export const run = editor => { + editor.insertText('x') + assert.throws(() => { + HistoryEditor.withNewBatch(editor, () => { + throw new Error('boom') + }) + }, /boom/) + editor.insertText('y') +} +export const input = ( + + + one + + + +) +export const output = cloneDeep(input) diff --git a/packages/slate-history/test/undo/without_merging/throws.tsx b/packages/slate-history/test/undo/without_merging/throws.tsx new file mode 100644 index 0000000000..8f3e3e2834 --- /dev/null +++ b/packages/slate-history/test/undo/without_merging/throws.tsx @@ -0,0 +1,24 @@ +/** @jsx jsx */ +import assert from 'assert' +import { jsx } from '../..' +import { HistoryEditor } from '../../..' +import { cloneDeep } from 'lodash' + +export const run = editor => { + assert.throws(() => { + HistoryEditor.withoutMerging(editor, () => { + throw new Error('boom') + }) + }, /boom/) + editor.insertText('a') + editor.insertText('b') +} +export const input = ( + + + one + + + +) +export const output = cloneDeep(input)