fix: scope the transaction compose map to its transaction - #1237
Open
Moonomo wants to merge 1 commit into
Open
Conversation
TextTransaction kept its pending-delta cache in a static map shared by every Transaction in the process, and compose() cleared it only after a fully successful pass. A transaction that was never applied, because EditorState.apply refused it while the editor was read-only or disposed, or because compose() itself threw, left its deltas queued, and every later transaction re-composed them as its own. A refused format landed on the next keystroke; once a leftover no longer fit its node, the inserts-only assertion fired on every transaction in the process until the app restarted. The cache now lives on the Transaction instance (an extension cannot hold state, which is why it was static) and compose() clears it in a finally, so a dropped or refused transaction takes its deltas with it. Chained calls on one transaction still compose together. Two regression tests cover a refused insert leaking into the next transaction on the same node and two refused deletes tripping the assertion; both fail on main. Fixes AppFlowy-IO/AppFlowy#4763
Moonomo
marked this pull request as ready for review
September 4, 2026 00:39
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.
Summary
TextTransaction(_composeMap) from a static on the extension onto theTransactioninstance, so a transaction that is never applied cannot hand its deltas to the next onefinallyinsidecompose(), so a delta that fails to compose is discarded with its transaction instead of being retried by every later onetest/core/transform/transaction_test.dartthat fail onmainand pass with the fixFixes: AppFlowy-IO/AppFlowy#4763
Root cause
TextTransaction(lib/src/core/transform/transaction.dart) queues the deltas built byinsertText,deleteText,mergeText,formatText, andreplaceTextinstatic final Map<Node, List<Delta>> _composeMap, one map shared by everyTransactionin the process.compose()folds each queued delta over its node's current delta, asserts the result contains onlyTextInserts, and clears the map only after the whole loop has succeeded.EditorState.applyreturns before it readstransaction.operationswhen the editor is disposed or not editable. A text transaction refused that way is never composed, so its deltas stay queued, and every later transaction on any node re-composes them as if they were its own:Delta.composeruns past the end of the delta and emits aTextDelete, the inserts-only assertion throws insidecompose(), and because the map is never cleared on that path the same assertion fires on every subsequent transaction in the process. Typing stops landing in every document until the app restarts. The stack in #4763 (Delta.compose→TextTransaction.compose→Transaction.operations) is this path.Host apps reach it through their own toolbars and menus, which call
toggleAttribute/formatDeltaor build text transactions directly against an editor that is read-only or was just disposed. The package's own floating toolbar and character shortcuts are gated oneditable, anddeleteSelectionreadstransaction.operationsfor its debug log beforeapply, so the built-in UI does not trigger it on its own.The cache was static because an extension cannot hold state; the transaction is its natural owner, and chained calls (
transaction..deleteText(..)..insertText(..)) compose together exactly as before. No public API changes.Tests
flutter test test/core/transform/— 38 passing. The two new cases fail onmain: the refused' world'insert composes in and the text readsAhello worldinstead ofAhello; two refused whole-text deletes makeoperationsthrow'composed.every((element) => element is TextInsert)': is not true.flutter analyze— no diagnostics introduced by this change.dart format --set-exit-if-changed .anddart run custom_lint— clean.Manual reproduction (example app)
The Fixed Toolbar page (
example/lib/pages/fixed_toolbar_editor.dart) callseditorState.toggleAttributefrom an external toolbar without checkingeditable, which is the host-app shape. It has no read-only switch, so temporarily make the unusedIcons.codecase in_FixedToolbarruneditorState.editable = !editorState.editable;.A refused format lands later:
</>(editor is now read-only; typing does nothing).</>again, click at the end of a different paragraph, type one character.main: the character appears and the word from step 1 turns bold at the same moment.A refused delete leaves the editor stuck:
Also route the unused
Icons.format_quotecase through a host-style delete of the selection:</>.</>again, click anywhere, type one character.main(debug build): nothing lands, the console shows theTextInsertassertion fromTextTransaction.compose, and typing anywhere else fails the same way until restart.