diff --git a/.changeset/emit-repair-patches-immediately.md b/.changeset/emit-repair-patches-immediately.md new file mode 100644 index 000000000..ac9cc5d99 --- /dev/null +++ b/.changeset/emit-repair-patches-immediately.md @@ -0,0 +1,18 @@ +--- +'@portabletext/editor': major +--- + +fix: emit structural repair patches immediately and make engine normalization the sole repairer + +The editor repairs structural defects in any value passed to it, initial or updated: it generates a `_key` for a block or child that has none or duplicates a sibling's, and it inserts an empty span into a text block with no children. Until now those repairs stayed in the editor's memory and the fixing patches were only emitted once the user made their first edit. Now they are emitted as soon as the value is applied. The practical consequence: loading a broken document can produce a `mutation` to persist before any user action. + +- An editable editor that receives a broken value emits the fixing `patch` events and their `mutation` right away. A host persisting mutations stores the repaired document immediately instead of at the user's first edit. +- A read-only editor that receives a broken value emits the `patch` events right away too, but holds the `mutation` until the editor becomes editable, because hosts reject writes to read-only documents. +- If a newer value arrives while a repair `mutation` is still pending, the outdated repair is discarded, whether the editor is read-only and holding it or editable and simply hasn't flushed it yet: a repair is never mistaken for unsaved edit work, so a host that already fixed the same defect with its own key supersedes ours regardless of which arrives first. When the newer value is broken too, a fresh repair for it is emitted instead. A repair computed against a value that is no longer current is discarded rather than delivered, with two exceptions: a repair whose target block can't yet be identified keeps flushing on its usual schedule instead of risking a silent drop, and a repair overtaken by another client's own patches to the same document is still delivered and converges with the later write, the same as any other concurrent edit. A later value that still shows the exact shape a block was just repaired from is recognized as the host not having caught up yet rather than a fresh defect: no repair is recomputed for it, and a repair already pending for that block is not discarded as superseded. +- Mutations still held when the editor unmounts are delivered to `mutation` listeners during unmount. A host that rejects mutations while read-only loses them there, as it did before. +- A block missing its `_key` no longer triggers the "invalid value" prompt. It is repaired like the other defects, emitting the same events. The prompt still appears for defects that need a human decision (an unknown `_type`, a non-object block), and its resolution patches now address the defective block's position instead of possibly addressing the wrong block. A keyless child with such a defect is now named by its position in the prompt too, instead of showing an `undefined` key. +- `markDefs` that no span references are no longer removed when a value enters the editor. They are removed when the user next edits that block. + +`InvalidValueResolution.autoResolve` is removed. The editor repairs mechanically fixable defects itself on intake, so no resolution is ever auto-resolvable; hosts that branched on the flag can delete that path. + +A `mutation` event's `value` now reflects the value the editor holds at the moment the mutation flushes, not the value that was current when its patches were produced. A mutation held read-only and flushed later carries the up-to-date value, not a stale snapshot from before the hold. diff --git a/.changeset/value-sync-read-only-repair-latch.md b/.changeset/value-sync-read-only-repair-latch.md new file mode 100644 index 000000000..8ee8562c2 --- /dev/null +++ b/.changeset/value-sync-read-only-repair-latch.md @@ -0,0 +1,13 @@ +--- +'@portabletext/plugin-sdk-value': patch +--- + +fix: don't latch the value-sync machine on patches emitted while read-only + +A read-only editor that receives a structurally invalid value (for example a block missing its `_key`) repairs it and emits the repair patch immediately. Previously the plugin mistook that repair for an unsaved user edit and stopped applying store updates until it was pushed, which cannot happen while read-only, so the editor stopped receiving remote changes for the rest of the read-only session. It now keeps applying store updates and pushes the repair once the editor becomes editable. + +One additional change: a background sync pass no longer mistakes the still-unpushed repair for the store having drifted and reverts it; the repaired content stays stable until the editor becomes editable and the repair pushes. + +Another additional change: a handful of Behavior events (`select`, `mouse.click`, `clipboard.copy`, and the serialization events) still run their actions while read-only, so a custom Behavior that mutates on one of those is now protected the same way as any other unpushed edit, instead of being mistaken for a held repair and risking reversion by the background sync pass. + +One more additional change: a store update applied while a read-only repair is still held now supersedes it instead of racing it. Previously the held repair could still flush after the newer update, pushing stale content (or, with `pushPatches`, a stale key) back over it. diff --git a/apps/studio/README.md b/apps/studio/README.md index a3c42387c..7532a63a4 100644 --- a/apps/studio/README.md +++ b/apps/studio/README.md @@ -85,12 +85,15 @@ under the Studio's own auth-token key, so no interactive login happens. - **`repair-on-load.spec.ts`**, run serially against `pte-lab.repairs.keyless-child` and `pte-lab.repairs.clean`: - - **loading a structurally broken document mutates nothing**: resets - the keyless-child article, opens it, and asserts the editor renders - without an "Invalid value" dialog (the missing key is repaired in - the editor's memory), that zero editor-emitted repair transactions - reach the dataset, and that the stored body is unchanged afterwards. - See "The contract on this branch" below. + - **structural repair persists exactly once on load**: resets the + keyless-child article, opens it, and asserts the editor renders + without an "Invalid value" dialog. It listens for mutations on + `drafts.pte-lab.repairs.keyless-child`, groups them into + transactions, and asserts exactly one transaction contains an + editor-emitted repair (with no further one following it), then + re-fetches the draft and asserts every child now has a string `_key` + with the seeded text unchanged. See "Known shape" below for why more + than one mutation transaction on load is expected. - **clean document stays silent**: resets the well-formed control article, opens it, and asserts zero mutations arrive and no draft gets created. @@ -163,22 +166,30 @@ CHOKIDAR_USEPOLLING=1 pnpm --filter studio dev --port 3391 `webServer` it manages, so `pnpm --filter studio e2e` only needs the raised `ulimit`. -### The contract on this branch: loading never mutates - -On this stable line, the editor repairs structural defects (missing or -duplicate `_key`s, empty `children`) in memory only, and the fixing -patches wait for the user's first local edit. Opening a broken document -therefore writes nothing: no draft, no mutation, and the stored body is -byte-identical afterwards. `repair-on-load.spec.ts` pins exactly that, -so any change that starts persisting repairs on load turns this suite -red here. - -The prerelease line (`next`) makes the opposite choice by design: -repairs are persisted immediately when a value is applied, and its copy -of this suite asserts exactly one editor-emitted repair transaction on -load (alongside Content Lake's own server-side key enrichment during -draft creation). Same rig, opposite contracts, each pinning its line's -intended behavior. +### Known shape: two mutation transactions on a broken doc's first load + +Opening a structurally broken document like `pte-lab.repairs.keyless-child` +produces two mutation transactions on the draft, not one, and that's +expected: + +1. The Actions API draft-create itself. Content Lake enriches array + items with `_key`s server-side as part of creating the draft, so this + transaction bundles the `create`, `_system.*` bookkeeping patches, and + index-addressed `_key` sets (`body[0].children[1]._key`) into one + transaction. +2. The editor's own intake repair, addressing the same child by its + block's key instead of an index (a `set`/`diffMatchPatch` patch on + `body[_key=="..."]...children...._key`), which runs once per load and + supersedes the server's generated key with its own. + +The two transactions are independent and expected: the server enriches +keys at draft-creation time as a general Content Lake behavior unrelated +to this editor, and the editor repairs the same structural gap on intake +regardless of what the server already did. The editor's key wins because +it lands second. `repair-on-load.spec.ts` asserts on this shape directly: +exactly one transaction contains an editor-emitted repair, and no further +one follows it. Document this here so the two-transaction shape doesn't +get re-diagnosed as a bug. ## Adding a new family diff --git a/apps/studio/e2e/repairs/repair-on-load.spec.ts b/apps/studio/e2e/repairs/repair-on-load.spec.ts index 7b8df016e..9cfc171fb 100644 --- a/apps/studio/e2e/repairs/repair-on-load.spec.ts +++ b/apps/studio/e2e/repairs/repair-on-load.spec.ts @@ -9,12 +9,9 @@ import {wiretap} from '../support/wiretap' const client = createLabClient() test.describe.serial('repair-on-load', () => { - test('loading a structurally broken document mutates nothing', async ({ - page, - }) => { + test('structural repair persists exactly once on load', async ({page}) => { const id = 'pte-lab.repairs.keyless-child' await resetDoc(client, repairsFixtures, id) - const seeded = await client.getDocument(id) const tap = wiretap(client, [id], 40_000) @@ -27,17 +24,29 @@ test.describe.serial('repair-on-load', () => { await expect(page.getByText('Invalid value')).toHaveCount(0) const entries = await tap.done - const events = entries.map(({event}) => event) + const draftEvents = entries + .filter(({event}) => event.documentId === `drafts.${id}`) + .map(({event}) => event) - const transactions = groupByTransaction(events) - const repairTransactions = transactions.filter(hasEditorRepairPatch) - expect(repairTransactions).toEqual([]) + expect(draftEvents.length).toBeGreaterThan(0) - const published = await client.getDocument(id) - expect(published?.body).toEqual(seeded?.body) + const transactions = groupByTransaction(draftEvents) + const repairTransactions = transactions.filter(hasEditorRepairPatch) + expect(repairTransactions).toHaveLength(1) const draft = await client.getDocument(`drafts.${id}`) - expect(draft?.body ?? seeded?.body).toEqual(seeded?.body) + expect(draft).toBeDefined() + const body = draft?.body as Array<{ + children: Array<{_key?: string; text: string}> + }> + for (const block of body) { + for (const child of block.children) { + expect(typeof child._key).toBe('string') + } + } + expect( + body.flatMap((block) => block.children.map((child) => child.text)), + ).toEqual(['This span has a `_key`, but ', 'this one does not', '.']) }) test('clean document stays silent', async ({page}) => { diff --git a/packages/editor/src/editor.ts b/packages/editor/src/editor.ts index 4b9c5cec1..f95f42778 100644 --- a/packages/editor/src/editor.ts +++ b/packages/editor/src/editor.ts @@ -38,9 +38,10 @@ export type EditorEvent = * if the editor's content has since diverged from it through local * edits. * - * Reconciliation is not an edit: it emits no `patch` or `mutation` - * events and adds no history step. While local changes are in - * flight, it is deferred until they have flushed. `undefined` and + * Reconciliation itself is not an edit and adds no history step. + * Repairs of structurally invalid content that it triggers do emit + * `patch` and `mutation` events. While local changes are in flight, + * it is deferred until they have flushed. `undefined` and * `[]` are the same empty snapshot: sending either when the previous * snapshot was also empty is a no-op and never clears locally typed * content. diff --git a/packages/editor/src/editor/create-editor-engine.tsx b/packages/editor/src/editor/create-editor-engine.tsx index 36a33d1fb..f9e766af2 100644 --- a/packages/editor/src/editor/create-editor-engine.tsx +++ b/packages/editor/src/editor/create-editor-engine.tsx @@ -10,6 +10,7 @@ import type {EditorActor} from './editor-machine' import {setupRemotePatches} from './remote-patches' import {subscribeHistory} from './subscriber.history' import {subscribePatchGeneration} from './subscriber.patch-generation' +import {subscribeRepairJournal} from './subscriber.repair-journal' import {subscribeUpdateValue} from './subscriber.update-value' type EditorEngineConfig = { @@ -72,10 +73,13 @@ export function createEditorEngine( editor.selectorChannelsPending = {registrations: false} editor.verifiedUniqueChildGroups = new Set() + editor.repairJournal = new Map() editor.remotePatches = [] editor.undoStepId = undefined editor.isDeferringMutations = false + editor.notifyInboundSyncStarted = null + editor.notifyInboundStateApplied = null editor.lastSyncedValue = undefined editor.valueUnsetEmitted = false editor.isPatching = true @@ -87,6 +91,7 @@ export function createEditorEngine( }) subscribeUpdateValue(context, editorEngine) + subscribeRepairJournal(editorEngine) subscribePatchGeneration({ editorActor: config.editorActor, editor: editorEngine, diff --git a/packages/editor/src/editor/create-editor.ts b/packages/editor/src/editor/create-editor.ts index 2007595ed..42743ecea 100644 --- a/packages/editor/src/editor/create-editor.ts +++ b/packages/editor/src/editor/create-editor.ts @@ -272,9 +272,6 @@ function createActors(config: { input: { initialValue: config.editorActor.getSnapshot().context.initialValue, keyGenerator: config.editorActor.getSnapshot().context.keyGenerator, - readOnly: config.editorActor - .getSnapshot() - .matches({'edit mode': 'read only'}), schema: config.editorActor.getSnapshot().context.schema, editorEngine: config.editorEngine, }, @@ -311,12 +308,16 @@ function createActors(config: { case 'value changed': config.relay.send(event) break - case 'patch': - config.editorActor.send({ - ...event, - type: 'internal.patch', - value: config.editorEngine.snapshot.context.value, - }) + case 'inbound sync started': + // The mutation batcher's generation bump, not forwarded to + // `editorActor` like the other cases: nothing there needs it. + config.editorEngine.notifyInboundSyncStarted?.() + break + + case 'inbound state applied': + // The mutation batcher's superseded-repair drop, not forwarded + // to `editorActor` like the other cases: nothing there needs it. + config.editorEngine.notifyInboundStateApplied?.(event.echoedBlockKeys) break default: @@ -334,7 +335,6 @@ function createActors(config: { const subscription = config.editorActor.subscribe((snapshot) => { const readOnly = snapshot.matches({'edit mode': 'read only'}) - syncActor.send({type: 'update readOnly', readOnly}) if (readOnly !== previousReadOnly) { previousReadOnly = readOnly diff --git a/packages/editor/src/editor/editor-machine.ts b/packages/editor/src/editor/editor-machine.ts index e6fbcf308..b8e9cf9be 100644 --- a/packages/editor/src/editor/editor-machine.ts +++ b/packages/editor/src/editor/editor-machine.ts @@ -16,14 +16,12 @@ import type { ExternalBehaviorEvent, } from '../behaviors/behavior.types.event' import type {Converter} from '../converters/converter.types' -import {isInNormalization} from '../engine/core/apply-context' import {DOMEditor} from '../engine/dom/plugin/dom-editor' import {normalize} from '../engine/editor/normalize' import {debug} from '../internal-utils/debug' import type {EventPosition} from '../internal-utils/event-position' import {sortByPriority} from '../priority/priority.sort' import type {RegistrableNode} from '../renderers/renderer.types' -import {pathContains} from '../traversal/path-contains' import type {NamespaceEvent, OmitFromUnion} from '../type-utils' import type {EditorSelection} from '../types/editor' import type {PortableTextEditorEngine} from '../types/editor-engine' @@ -58,6 +56,12 @@ export type ExternalEditorEvent = type InternalPatchEvent = NamespaceEvent & { operationId?: string value: Array + // The exact signature `subscriber.repair-journal.ts` uses to identify a + // startup repair: normalization firing while a remote frame is on the + // stack. Carried on the event so the mutation batcher can classify the + // patch by what produced it instead of by the actor state at receipt, + // which the deferred-events replay in `setup` decouples from emission. + intakeRepair: boolean } /** @@ -312,30 +316,6 @@ export const editorMachine = setup({ 'clear pending events': assign({ pendingEvents: [], }), - 'discard conflicting pending patches': assign({ - pendingEvents: ({context, event}) => { - if (event.type !== 'patches') { - return context.pendingEvents - } - - const incomingPaths = event.patches.map((patch) => patch.path) - - return context.pendingEvents.filter((pendingEvent) => { - if (pendingEvent.type !== 'internal.patch') { - return true - } - - return !incomingPaths.some( - (incomingPath) => - pathContains(pendingEvent.patch.path, incomingPath) || - pathContains(incomingPath, pendingEvent.patch.path), - ) - }) - }, - }), - 'discard all pending events': assign({ - pendingEvents: [], - }), 'defer incoming patches': assign({ pendingIncomingPatchesEvents: ({context, event}) => { return event.type === 'patches' @@ -453,13 +433,6 @@ export const editorMachine = setup({ return context.editorEngine.operations.length > 0 }, - 'engine is normalizing node': ({context}) => { - if (!context.editorEngine) { - return false - } - - return isInNormalization(context.editorEngine.applyContext) - }, }, }).createMachine({ id: 'editor', @@ -717,6 +690,8 @@ export const editorMachine = setup({ 'emit ready', 'emit pending incoming patches', 'clear pending incoming patches', + 'emit pending events', + 'clear pending events', ], on: { 'internal.patch': { @@ -788,78 +763,12 @@ export const editorMachine = setup({ }, }, 'writing': { - initial: 'pristine', - states: { - pristine: { - initial: 'idle', - states: { - idle: { - entry: [ - () => { - debug.state( - 'entry: setup->set up->writing->pristine->idle', - ) - }, - ], - exit: [ - () => { - debug.state( - 'exit: setup->set up->writing->pristine->idle', - ) - }, - ], - on: { - 'internal.patch': [ - { - guard: 'engine is normalizing node', - actions: 'defer event', - }, - { - actions: 'defer event', - target: '#editor.setup.set up.writing.dirty', - }, - ], - 'mutation': [ - { - guard: 'engine is normalizing node', - actions: 'defer event', - }, - { - actions: 'defer event', - target: '#editor.setup.set up.writing.dirty', - }, - ], - 'patches': { - actions: 'discard conflicting pending patches', - }, - 'syncing value': { - actions: 'discard all pending events', - }, - }, - }, - }, + on: { + 'internal.patch': { + actions: 'emit patch event', }, - dirty: { - entry: [ - () => { - debug.state('entry: setup->set up->writing->dirty') - }, - 'emit pending events', - 'clear pending events', - ], - exit: [ - () => { - debug.state('exit: setup->set up->writing->dirty') - }, - ], - on: { - 'internal.patch': { - actions: 'emit patch event', - }, - 'mutation': { - actions: 'emit mutation event', - }, - }, + 'mutation': { + actions: 'emit mutation event', }, }, }, diff --git a/packages/editor/src/editor/mutation-batcher.test.ts b/packages/editor/src/editor/mutation-batcher.test.ts index 803146a13..56db2f63a 100644 --- a/packages/editor/src/editor/mutation-batcher.test.ts +++ b/packages/editor/src/editor/mutation-batcher.test.ts @@ -1,15 +1,20 @@ import type {Patch} from '@portabletext/patches' +import {compileSchema, defineSchema} from '@portabletext/schema' +import {createTestKeyGenerator} from '@portabletext/test' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' +import {createActor} from 'xstate' import { emitOperationEvent, type OperationEvent, } from '../engine/core/operation-channel' import {createEditor} from '../engine/create-editor' +import type {Node} from '../engine/interfaces/node' import type {EngineOperation} from '../engine/interfaces/operation' import type {PortableTextEditorEngine} from '../types/editor-engine' import type {EditorActor} from './editor-machine' import {createMutationBatcher} from './mutation-batcher' import {createRelay} from './relay' +import {syncMachine} from './sync-machine' const FLUSH_INTERVAL = 500 const TYPE_DEBOUNCE = 250 @@ -17,6 +22,13 @@ const TYPE_DEBOUNCE = 250 function createTestHarness({readOnly = false}: {readOnly?: boolean} = {}) { const editorEngine = createEditor() as PortableTextEditorEngine editorEngine.isDeferringMutations = false + // `flush` reads `editorEngine.snapshot.context.value`; the bare engine `createEditor` + // returns has no `snapshot` (only `editor/create-editor.ts` wires one up). + editorEngine.snapshot = { + blockIndexMap: new Map(), + context: {value: []}, + decoratorState: {}, + } as unknown as PortableTextEditorEngine['snapshot'] let isReadOnly = readOnly let patchListener: @@ -24,9 +36,11 @@ function createTestHarness({readOnly = false}: {readOnly?: boolean} = {}) { patch: Patch operationId?: string value: Array + intakeRepair: boolean }) => void) | undefined const mutationSends: Array<{patches: Array}> = [] + const mutationValues: Array = [] const editorActor = { getSnapshot: () => ({ @@ -52,13 +66,19 @@ function createTestHarness({readOnly = false}: {readOnly?: boolean} = {}) { patch: Patch operationId?: string value: Array + intakeRepair: boolean }) => void, ) => { patchListener = listener return {unsubscribe: () => {}} }, - send: (event: {type: 'mutation'; patches: Array}) => { + send: (event: { + type: 'mutation' + patches: Array + value: unknown + }) => { mutationSends.push({patches: event.patches}) + mutationValues.push(event.value) }, } as unknown as EditorActor @@ -75,10 +95,15 @@ function createTestHarness({readOnly = false}: {readOnly?: boolean} = {}) { return { editorEngine, mutationSends, + mutationValues, relayedPatches, unsubscribe, - sendPatch: (patch: Patch, operationId?: string) => { - patchListener?.({patch, operationId, value: []}) + sendPatch: ( + patch: Patch, + operationId?: string, + {intakeRepair = false}: {intakeRepair?: boolean} = {}, + ) => { + patchListener?.({patch, operationId, value: [], intakeRepair}) }, setReadOnly: (value: boolean) => { isReadOnly = value @@ -96,6 +121,15 @@ function createPatch(path: string): Patch { return {type: 'set', path: [{_key: path}], value: path, origin: 'local'} } +// A numeric first path segment resolves to a block key only by indexing into +// the bulk's own snapshot `value`. The test harness always sends `value: []`, +// so this patch's block key is unresolvable, the same way a repair targeting +// a still-keyless block is unresolvable before the arm that mints its key +// has run. +function createUnkeyedPatch(index: number): Patch { + return {type: 'set', path: [index], value: index, origin: 'local'} +} + function createOperationEvent(operation: EngineOperation): OperationEvent { return { operation, @@ -151,6 +185,175 @@ describe('mutation batcher', () => { expect(harness.editorEngine.isDeferringMutations).toBe(false) }) + test('a flushed mutation carries the value the engine currently holds, not the value captured when the bulk formed', () => { + const harness = createTestHarness() + + harness.sendPatch(createPatch('a'), 'op-1') + + const laterValue = [ + { + _type: 'block', + _key: 'later', + style: 'normal', + markDefs: [], + children: [], + }, + ] as unknown as Array + harness.editorEngine.snapshot.context.value = laterValue + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationValues).toEqual([laterValue]) + }) + + test('an intake-repair patch received while editable does not set `isDeferringMutations`', () => { + const harness = createTestHarness() + + harness.sendPatch(createPatch('a'), 'op-1', {intakeRepair: true}) + + expect(harness.editorEngine.isDeferringMutations).toBe(false) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([{patches: [createPatch('a')]}]) + }) + + test('an intake-repair bulk is dropped by `dropSupersededRepairs` even though it arrived while editable', () => { + const harness = createTestHarness() + + harness.editorEngine.notifyInboundSyncStarted?.() + harness.sendPatch(createPatch('a'), 'op-1', {intakeRepair: true}) + + // First inbound settle: the bulk survives its own pass (the + // current-generation exemption). + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + // A second pass starts and settles: the bulk is now superseded and + // dropped. + harness.editorEngine.notifyInboundSyncStarted?.() + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([]) + }) + + test('a repair bulk minted before a settling pass started is dropped by that pass, not exempted by its generation', () => { + const harness = createTestHarness() + + // Minted between passes (remote-patch fallout normalization), before + // any pass has bumped the generation. + harness.sendPatch(createPatch('a'), undefined, {intakeRepair: true}) + + harness.editorEngine.notifyInboundSyncStarted?.() + // Minted during the pass that's now settling: exempt as the pass's + // own mint. + harness.sendPatch(createPatch('b'), undefined, {intakeRepair: true}) + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([{patches: [createPatch('b')]}]) + }) + + test('a bulk mixing an intake-repair patch with a real edit still sets `isDeferringMutations` and survives `dropSupersededRepairs`', () => { + const harness = createTestHarness() + + harness.sendPatch(createPatch('a'), 'op-1', {intakeRepair: true}) + harness.sendPatch(createPatch('b'), 'op-1') + + expect(harness.editorEngine.isDeferringMutations).toBe(true) + + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([ + {patches: [createPatch('a'), createPatch('b')]}, + ]) + }) + + test('a bulk mixing an intake-repair patch with a real edit survives `dropSupersededRepairs` no matter what the echoed set contains', () => { + const harness = createTestHarness() + + harness.sendPatch(createPatch('a'), 'op-1', {intakeRepair: true}) + harness.sendPatch(createPatch('b'), 'op-1') + + // Neither settle's echoed set names block `a`: the bulk survives on + // its user-work patch alone, not on an echo match. + harness.editorEngine.notifyInboundStateApplied?.(new Set(['unrelated'])) + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([ + {patches: [createPatch('a'), createPatch('b')]}, + ]) + }) + + test('intake-repair patches for two different blocks form two separate bulks', () => { + const harness = createTestHarness() + + harness.sendPatch(createPatch('a'), undefined, {intakeRepair: true}) + harness.sendPatch(createPatch('b'), undefined, {intakeRepair: true}) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([ + {patches: [createPatch('a')]}, + {patches: [createPatch('b')]}, + ]) + }) + + test('an unresolved-key intake-repair patch followed by a resolved-key one forms two separate bulks', () => { + const harness = createTestHarness() + + harness.sendPatch(createUnkeyedPatch(0), undefined, {intakeRepair: true}) + harness.sendPatch(createPatch('b'), undefined, {intakeRepair: true}) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([ + {patches: [createUnkeyedPatch(0)]}, + {patches: [createPatch('b')]}, + ]) + }) + + test('`dropSupersededRepairs` drops a stale keyed bulk while keeping an unresolved-key bulk', () => { + const harness = createTestHarness() + + harness.sendPatch(createPatch('a'), undefined, {intakeRepair: true}) + harness.sendPatch(createUnkeyedPatch(0), undefined, {intakeRepair: true}) + + // A pass starts and settles without echoing block `a`: the keyed bulk + // is superseded and dropped. The unresolved-key bulk can't be matched + // against any block in the echoed set, so it survives regardless. + harness.editorEngine.notifyInboundSyncStarted?.() + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([{patches: [createUnkeyedPatch(0)]}]) + }) + + test('`dropSupersededRepairs` keeps an intake-repair bulk whose block key is in the echoed set and drops the others', () => { + const harness = createTestHarness() + + harness.editorEngine.notifyInboundSyncStarted?.() + harness.sendPatch(createPatch('a'), undefined, {intakeRepair: true}) + harness.sendPatch(createPatch('b'), undefined, {intakeRepair: true}) + + // First settle: both bulks survive on the current-generation exemption. + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + // A second pass starts and settles: only `a` is still echoing. + harness.editorEngine.notifyInboundSyncStarted?.() + harness.editorEngine.notifyInboundStateApplied?.(new Set(['a'])) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([{patches: [createPatch('a')]}]) + }) + test('relays individual patch events immediately while batching mutations', () => { const harness = createTestHarness() @@ -160,22 +363,20 @@ describe('mutation batcher', () => { expect(harness.mutationSends).toHaveLength(0) }) - test('defers patch events and mutations while read-only, flushing once editable', () => { + test('relays patch events immediately even while read-only, holding the mutation until editable', () => { const harness = createTestHarness({readOnly: true}) harness.sendPatch(createPatch('a'), 'op-1') - expect(harness.relayedPatches).toEqual([]) + expect(harness.relayedPatches).toEqual([createPatch('a')]) vi.advanceTimersByTime(FLUSH_INTERVAL * 3) - expect(harness.relayedPatches).toEqual([]) expect(harness.mutationSends).toHaveLength(0) harness.setReadOnly(false) vi.advanceTimersByTime(FLUSH_INTERVAL) - expect(harness.relayedPatches).toEqual([createPatch('a')]) expect(harness.mutationSends).toEqual([{patches: [createPatch('a')]}]) }) @@ -188,6 +389,15 @@ describe('mutation batcher', () => { expect(harness.mutationSends).toEqual([{patches: [createPatch('a')]}]) }) + test('flushes pending mutations on unsubscribe even while read-only', () => { + const harness = createTestHarness({readOnly: true}) + + harness.sendPatch(createPatch('a'), 'op-1') + harness.unsubscribe() + + expect(harness.mutationSends).toEqual([{patches: [createPatch('a')]}]) + }) + test('defers mutations while normalization is suspended, flushing once it resumes', () => { const harness = createTestHarness() @@ -230,6 +440,155 @@ describe('mutation batcher', () => { ]) }) + test('clears the flush interval once dropping superseded repairs empties the queue', () => { + const harness = createTestHarness({readOnly: true}) + + harness.editorEngine.notifyInboundSyncStarted?.() + harness.sendPatch(createPatch('a'), 'op-1', {intakeRepair: true}) + + // First inbound settle: the bulk survives its own pass (the + // current-generation exemption). + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + // A second pass starts and settles: the bulk is now superseded and + // dropped, leaving the queue empty. + harness.editorEngine.notifyInboundSyncStarted?.() + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(vi.getTimerCount()).toBe(0) + expect(harness.mutationSends).toEqual([]) + }) + + test('a non-repair patch received while read-only is never dropped by the cull', () => { + const harness = createTestHarness({readOnly: true}) + + harness.sendPatch(createPatch('a'), 'op-1') + + // A settled value-sync pass starting and ending after the read-only + // patch was received: under the old read-only-at-receipt + // classification this would look exactly like a superseded repair + // (no generation match, no echoed block key) and get dropped. + harness.editorEngine.notifyInboundSyncStarted?.() + harness.editorEngine.notifyInboundStateApplied?.(new Set()) + + harness.setReadOnly(false) + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([{patches: [createPatch('a')]}]) + }) + + test('an intake-repair bulk for a block that keeps echoing survives repeated `dropSupersededRepairs` calls and flushes once editable', () => { + const harness = createTestHarness({readOnly: true}) + const keyGenerator = createTestKeyGenerator() + const schema = compileSchema(defineSchema({})) + const blockKey = keyGenerator() + + const beforeShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: '', marks: []} as unknown as Node], + } + const afterShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: '', marks: []}], + } + + harness.editorEngine.containers = new Map() + harness.editorEngine.blockIndexMap = new Map() + harness.editorEngine.verifiedUniqueChildGroups = new Set() + harness.editorEngine.repairJournal = new Map([ + [blockKey, {beforeShape, afterShape}], + ]) + harness.editorEngine.snapshot = { + blockIndexMap: harness.editorEngine.blockIndexMap, + context: { + containers: new Map(), + converters: [], + keyGenerator, + readOnly: true, + schema, + selection: null, + value: [afterShape as unknown as never], + }, + decoratorState: {}, + } as PortableTextEditorEngine['snapshot'] + + // The intake repair itself, held because the editor is read-only. + // Its path targets `blockKey` directly, the same block the journal + // below keeps echoing. + harness.sendPatch(createPatch(blockKey), 'op-1', {intakeRepair: true}) + + // Wired exactly like `create-editor.ts`'s one-line glue: every pass + // start bumps the batcher's generation, and every settle tells the + // batcher an inbound state applied, carrying the pass's + // echoed-block-keys set. + const syncActor = createActor(syncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: harness.editorEngine, + }, + }) + syncActor.on('inbound sync started', () => { + harness.editorEngine.notifyInboundSyncStarted?.() + }) + syncActor.on('inbound state applied', (event) => { + harness.editorEngine.notifyInboundStateApplied?.(event.echoedBlockKeys) + }) + syncActor.start() + + // Two stale echoes of the pre-repair shape, each paired with a + // different well-formed control block so the machine treats every + // send as a new value instead of no-opping. The journal recognizes + // `blockKey`'s echo both times, so both settles report it in their + // echoed-block-keys set and `dropSupersededRepairs` keeps the bulk on + // that echo match: the repair was minted before this test's first + // pass even started, so it never qualifies for the current-generation + // exemption at all. + syncActor.send({ + type: 'update value', + value: [ + beforeShape as unknown as never, + { + _type: 'block', + _key: 'control-a', + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'ca0', text: 'a', marks: []}], + } as unknown as never, + ], + }) + syncActor.send({ + type: 'update value', + value: [ + beforeShape as unknown as never, + { + _type: 'block', + _key: 'control-b', + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'cb0', text: 'b', marks: []}], + } as unknown as never, + ], + }) + + vi.advanceTimersByTime(FLUSH_INTERVAL * 3) + + expect(harness.mutationSends).toHaveLength(0) + + harness.setReadOnly(false) + vi.advanceTimersByTime(FLUSH_INTERVAL) + + expect(harness.mutationSends).toEqual([{patches: [createPatch(blockKey)]}]) + }) + test('a non-typing operation flushes eagerly, ending an in-progress typing session', () => { const harness = createTestHarness() diff --git a/packages/editor/src/editor/mutation-batcher.ts b/packages/editor/src/editor/mutation-batcher.ts index 13add535a..4cdc581f4 100644 --- a/packages/editor/src/editor/mutation-batcher.ts +++ b/packages/editor/src/editor/mutation-batcher.ts @@ -3,13 +3,77 @@ import type {PortableTextBlock} from '@portabletext/schema' import {subscribeToOperations} from '../engine/core/operation-channel' import {isNormalizing} from '../engine/editor/is-normalizing' import type {PortableTextEditorEngine} from '../types/editor-engine' +import {isKeyedSegment} from '../utils/util.is-keyed-segment' import type {EditorActor} from './editor-machine' import type {Relay} from './relay' +const UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY = Symbol( + 'unresolved intake-repair block key', +) + type PendingMutation = { operationId?: string - value: Array | undefined patches: Array + // Whether any patch in this bulk is genuine user work: not an intake + // repair. Read-only state gates almost every editor mutation, but not + // every one: a handful of behavior events (`select`, `mouse.click`, + // `clipboard.copy`, `serialize`, `serialization.failure`, + // `serialization.success`) still run their actions while read-only, and + // a mutating behavior on one of those produces user work the cull must + // protect exactly like an editable-time edit. An intake repair is the + // only patch that's never user work, regardless of when the batcher + // receives it (`setup`'s deferred-events replay can hand it over after + // the editor has already left read-only): the engine re-derives it from + // whatever value arrives next, so a bulk holding only repairs needs no + // snapshot protection. + holdsUserWork: boolean + // The generation current when this bulk was created. `currentGeneration` + // bumps when a value-sync pass starts, not when it ends, so it marks + // which pass (if any) was in progress at creation. `dropSupersededRepairs` + // drops a non-editable bulk once its generation falls behind the + // current one, proving it wasn't minted by the pass that's now settling. + // A bulk tagged with the current generation was minted during the pass + // that's now settling (the pass's own repair re-mints included), so it's + // exempt regardless of `holdsUserWork`; a repair minted between passes + // (remote-patch fallout) carries an older generation and gets no such + // exemption. + generation: number + // The block an intake-repair bulk repairs, derived from its patches' + // first path segment. `undefined` for a non-repair bulk. + // `UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY` for an intake-repair bulk whose + // block couldn't be resolved (a numeric first segment indexing into a + // value where the block itself has no `_key` yet, e.g. a keyless block + // mid-normalization, before the arm that mints its key has run). Keeping + // that case distinct from `undefined` matters: `handlePatch` never lets a + // resolved-key patch merge into an unresolved bulk or vice versa (an + // unresolved bulk can belong to any block, so backfilling a key onto it + // from a later patch could mislabel someone else's repair), and + // `dropSupersededRepairs` can never echo-match an unresolved bulk to any + // block, so it never drops one. Distinct resolved block keys never merge + // into the same bulk either, so a repair superseded for one block can't + // smuggle a stale repair for another out past the cull. + intakeRepairBlockKey: + | string + | undefined + | typeof UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY +} + +function deriveIntakeRepairBlockKey( + patch: Patch, + value: Array | undefined, +): string | typeof UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY { + const [firstSegment] = patch.path + + if (isKeyedSegment(firstSegment)) { + return firstSegment._key + } + + if (typeof firstSegment === 'number') { + const key = value?.[firstSegment]?._key + return key ?? UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY + } + + return UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY } const TYPE_DEBOUNCE = 250 @@ -28,17 +92,57 @@ const FLUSH_INTERVAL = /** * Batches `internal.patch` events into debounced `mutation` events. * - * Individual `patch` events relay to consumers immediately (deferred while - * the editor is read-only); the patches themselves accumulate into bulks - * keyed by `operationId` and flush as `mutation` events on an interval, or - * eagerly when typing stops or a non-typing operation applies. + * Individual `patch` events relay to consumers immediately, including + * while the editor is read-only. The patches themselves accumulate into + * bulks keyed by `operationId` (and, for intake-repair patches, also by + * the block they repair, so repairs for different blocks never share a + * bulk) and flush as `mutation` events on an interval, or eagerly when + * typing stops or a non-typing operation applies, except while the editor + * is read-only: hosts following the documented `onChange` contract reject + * mutations against a read-only document, so bulks hold and flush on the + * first tick after the editor becomes editable again. + * + * `editorEngine.isDeferringMutations` reflects only bulks that hold at + * least one patch of genuine user work: that's the unflushed work a + * remote snapshot must not clobber. A patch counts as user work whenever + * it isn't an intake repair (a normalization fix minted while adopting a + * remote value, tagged `intakeRepair` at emission so classification + * survives `setup`'s deferred-events replay landing after read-only has + * already lifted), regardless of read-only state: a handful of behavior + * events still run their actions while read-only (`select`, `mouse.click`, + * `clipboard.copy`, `serialize`, `serialization.failure`, + * `serialization.success`), and a mutating behavior on one of those is + * exactly as unrecoverable as an editable-time edit if the cull drops it. + * A bulk made entirely of intake-repair patches carries nothing worth + * protecting, so it never sets the flag. + * + * `editorEngine.notifyInboundSyncStarted` bumps `currentGeneration` when + * a value-sync pass starts, before that pass's own invoked sync can mint + * anything. Marking pass membership at the start rather than the end + * matters: a repair minted between passes (remote-patch fallout, fired + * from inside the `patches` remote frame's own normalization, outside any + * settling pass) is minted before the *next* pass's bump, so it carries + * that older generation and is judged like any other held repair instead + * of masquerading as the next pass's own mint. + * + * `editorEngine.notifyInboundStateApplied` runs this batcher's + * `dropSupersededRepairs`, passed the set of block keys the sync pass + * found still echoing their pre-repair shape: once an inbound value sync + * has settled, a held repair bulk with no editable-time patch is + * superseded (the full snapshot either already contains its repair or + * still echoes it, both proven per block by the echoed-block-keys set, or + * the bulk carries the generation this pass bumped to at its start, + * proving the pass's own normalization re-minted it) and is dropped + * instead of flushing. Only value syncs drop superseded repairs: a remote + * patch batch is a delta that can change engine state without superseding + * a held repair, so it must leave held bulks alone. * * The flush interval keeps running when a flush bails on its guard and is - * only cleared once pending work has actually drained. Both guard branches - * rely on this: read-only-deferred mutations flush on the first tick after - * the editor becomes editable, and work arriving while normalization is - * suppressed flushes on the first tick after the `withoutNormalizing` - * block exits. + * only cleared once pending work has actually drained. Both guard + * branches rely on this: read-only-held mutations flush on the first + * tick after the editor becomes editable, and work arriving while + * normalization is suppressed flushes on the first tick after the + * `withoutNormalizing` block exits. */ export function createMutationBatcher({ editorActor, @@ -54,10 +158,20 @@ export function createMutationBatcher({ // Closure state lives outside `subscribe` so pending work survives a // StrictMode unmount/remount, like the persisted actor snapshot did. let pendingMutations: Array = [] - let pendingPatchEvents: Array = [] let flushInterval: ReturnType | undefined let typeDebounce: ReturnType | undefined let isTyping = false + // Bumped by `notifyInboundSyncStarted` when a value-sync pass starts, + // not by `dropSupersededRepairs` when one ends: a bulk only ever + // carries the generation current at its own creation, so tagging pass + // *start* is what makes a bulk's generation mean "minted during this + // pass" rather than "minted before the next pass's cull happened to + // run", which a fallout repair minted between passes would otherwise + // satisfy by accident. A bulk left behind by an older generation (one + // that has since seen a later pass start) is unambiguously stale, and + // `handlePatch` below refuses to extend it with a newer generation's + // patch. + let currentGeneration = 0 function isReadOnly() { return editorActor.getSnapshot().matches({'edit mode': 'read only'}) @@ -67,47 +181,130 @@ export function createMutationBatcher({ patch: Patch operationId?: string value: Array + intakeRepair: boolean }) { - editorEngine.isDeferringMutations = true + // A patch counts as user work whenever it isn't an intake repair, + // regardless of what the actor state reads at receipt: a handful of + // behavior events still run their actions while read-only (`select`, + // `mouse.click`, `clipboard.copy`, `serialize`, + // `serialization.failure`, `serialization.success`), so read-only + // receipt is no proof a patch is a repair. `intakeRepair` is instead a + // provenance flag set at emission (normalization firing inside a + // remote frame), which also survives `setup`'s deferred-events replay + // landing after read-only has already lifted, unlike receipt-time + // state. + const holdsUserWork = !event.intakeRepair + const intakeRepairBlockKey = event.intakeRepair + ? deriveIntakeRepairBlockKey(event.patch, event.value) + : undefined - if (isReadOnly()) { - pendingPatchEvents.push(event.patch) - } else { - relay.send({type: 'patch', patch: event.patch}) - } + relay.send({ + type: 'patch', + patch: event.patch, + intakeRepair: event.intakeRepair, + }) const lastBulk = pendingMutations.at(-1) - if (lastBulk && lastBulk.operationId === event.operationId) { - lastBulk.value = event.value + // Two intake-repair patches never share a bulk unless they agree on + // block-key resolution, even sharing the same `operationId` (`undefined` + // is common outside a behavior, and normalization can repair several + // broken blocks in the same pass): two different resolved keys are + // obviously different blocks, and a resolved key crossing with + // `UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY` in either direction never merges + // either, since backfilling a bulk's resolution state from a later + // patch could mislabel a different block's repair under interleaving. + // Two unresolved patches *do* merge (both sides of the comparison are + // the same `UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY` singleton), which is + // safe only because `dropSupersededRepairs` never drops an unresolved + // bulk. A non-repair patch merges by `operationId` exactly as before, + // regardless of what an earlier repair in the same bulk claimed. + const blockKeyMismatch = + event.intakeRepair && + lastBulk?.intakeRepairBlockKey !== undefined && + lastBulk.intakeRepairBlockKey !== intakeRepairBlockKey + + if ( + lastBulk && + !blockKeyMismatch && + lastBulk.operationId === event.operationId && + lastBulk.generation === currentGeneration + ) { lastBulk.patches.push(event.patch) + lastBulk.holdsUserWork ||= holdsUserWork } else { pendingMutations.push({ operationId: event.operationId, - value: event.value, patches: [event.patch], + holdsUserWork, + generation: currentGeneration, + intakeRepairBlockKey, }) } + updateIsDeferringMutations() + if (flushInterval === undefined) { flushInterval = setInterval(flush, FLUSH_INTERVAL) } } - function flush() { - if (isReadOnly() || !isNormalizing(editorEngine)) { - // Leave the interval running: read-only-deferred mutations flush on - // the first tick after the editor becomes editable again. + function updateIsDeferringMutations() { + editorEngine.isDeferringMutations = pendingMutations.some( + (bulk) => bulk.holdsUserWork, + ) + } + + // Called after every value sync pass that walked every block and + // settled, and only then: a full snapshot either already contains a + // held repair, the pass re-minted it (that + // re-mint carries the generation this pass's own `notifyInboundSyncStarted` + // bump set, matching `currentGeneration` here), or the pass's snapshot + // still echoes the pre-repair shape for that specific block (in which + // case the pass reports the block's key in `echoedBlockKeys`), which is + // what makes dropping safe (a remote patch batch proves none of these, + // so it never drops anything). A bulk still tagged with an older + // generation, whose block (if it has one) isn't in this pass's echoed + // set, held nothing but a repair the settled state has superseded (or a + // between-pass fallout repair this pass's snapshot has moved past), so + // it's dropped. An unresolved-key bulk (`UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY`) + // is exempt outright: it can't be matched against any block in + // `echoedBlockKeys`, so there's no way to prove it's superseded, only ways + // to wrongly assume it. It keeps flushing on cadence instead; worst case + // that's one redundant mutation, never a silently dropped repair. + function dropSupersededRepairs(echoedBlockKeys: Set) { + pendingMutations = pendingMutations.filter( + (bulk) => + bulk.holdsUserWork || + bulk.generation === currentGeneration || + bulk.intakeRepairBlockKey === UNRESOLVED_INTAKE_REPAIR_BLOCK_KEY || + (typeof bulk.intakeRepairBlockKey === 'string' && + echoedBlockKeys.has(bulk.intakeRepairBlockKey)), + ) + updateIsDeferringMutations() + } + + editorEngine.notifyInboundSyncStarted = () => { + currentGeneration++ + } + editorEngine.notifyInboundStateApplied = dropSupersededRepairs + + function flush({ignoreReadOnly = false}: {ignoreReadOnly?: boolean} = {}) { + if (pendingMutations.length === 0) { + if (flushInterval !== undefined) { + clearInterval(flushInterval) + flushInterval = undefined + } return } - if (pendingPatchEvents.length === 0 && pendingMutations.length === 0) { + if ((isReadOnly() && !ignoreReadOnly) || !isNormalizing(editorEngine)) { + // Leave the interval running: read-only-held mutations flush on the + // first tick after the editor becomes editable again. return } - const patchEvents = pendingPatchEvents const mutations = pendingMutations - pendingPatchEvents = [] pendingMutations = [] if (flushInterval !== undefined) { @@ -115,19 +312,13 @@ export function createMutationBatcher({ flushInterval = undefined } - for (const patch of patchEvents) { - relay.send({type: 'patch', patch}) - } - - editorEngine.isDeferringMutations = false + updateIsDeferringMutations() for (const bulk of mutations) { - // The editor machine still gates mutations through its setup states - // and re-emits them to the relay. editorActor.send({ type: 'mutation', patches: bulk.patches, - value: bulk.value, + value: editorEngine.snapshot.context.value, }) } } @@ -173,20 +364,21 @@ export function createMutationBatcher({ ) if (pendingMutations.length > 0 && flushInterval === undefined) { - // Re-mounted with work deferred from before the unmount (e.g. a - // read-only editor): resume the flush cadence. + // Re-mounted with pending mutations from before the unmount: + // resume the flush cadence. flushInterval = setInterval(flush, FLUSH_INTERVAL) } return () => { - // Flush pending patches and mutations before unmounting, while the - // editor-actor-to-relay routing is still subscribed. A read-only - // editor's deferred work intentionally stays unemitted here — - // read-only deferral holds through teardown. The normalizing - // branch of the guard cannot bail at this point: `normalizing` is - // only ever `false` inside a synchronous `withoutNormalizing` - // block, which no effect cleanup can interleave with. - flush() + // Flush pending mutations before unmounting, while the + // editor-actor-to-relay routing is still subscribed, ignoring the + // read-only guard: a host tearing down a read-only editor (e.g. on + // disconnect) must still receive work typed just before the + // flip, or it's lost for good. The normalizing branch of the guard + // cannot bail at this point: `normalizing` is only ever `false` + // inside a synchronous `withoutNormalizing` block, which no effect + // cleanup can interleave with. + flush({ignoreReadOnly: true}) patchSubscription.unsubscribe() unsubscribeFromOperations() diff --git a/packages/editor/src/editor/relay.ts b/packages/editor/src/editor/relay.ts index 4aa4c29ec..f5fb2595c 100644 --- a/packages/editor/src/editor/relay.ts +++ b/packages/editor/src/editor/relay.ts @@ -31,9 +31,11 @@ export type EditorEmittedEvent = * Emitted synchronously for every document-changing operation the * engine applies (`set.selection` is excluded; the `selection` event * serves selection observers), including operations from initial - * value sync and normalization, unlike `patch` and `mutation` - * events, which are held back until the editor is dirty. Do not - * dispatch editor events from a listener; read current state via + * value sync and normalization. `patch` and `mutation` events cover + * these operations too, including repairs the editor makes to a + * structurally invalid incoming value: `patch` emits as each patch + * is produced, `mutation` batches patches on a debounced flush. Do not dispatch editor + * events from a listener; read current state via * `editor.getSnapshot()`. * * The `operation` object is the engine's own, passed by reference: @@ -75,6 +77,14 @@ export type EditorEmittedEvent = /** * @public + * + * Emitted at each debounced flush with the patches produced since the + * previous flush, batched so a user action and its normalization fallout + * arrive together. Patches the editor produced on its own, repairing a + * structurally invalid incoming value (a minted `_key`, a placeholder + * span), arrive the same way. While the editor is read-only, mutations + * hold until it becomes editable; a held repair that a newer incoming + * value supersedes is dropped rather than delivered late. */ export type MutationEvent = { type: 'mutation' @@ -82,9 +92,26 @@ export type MutationEvent = { value: Array | undefined } +/** + * Emitted synchronously as each patch is produced, including repair + * patches for a structurally invalid incoming value, and regardless of + * read-only state; the `mutation` event batches patches separately on + * its own debounced schedule. + */ export type PatchEvent = { type: 'patch' patch: Patch + /** + * Whether this patch is an engine repair of a structurally invalid + * incoming value, as opposed to user work (an edit, or a mutating + * Behavior action). Read-only state alone doesn't tell them apart: a + * handful of Behavior events still run their actions while read-only + * (`select`, `mouse.click`, `clipboard.copy`, `serialize`, + * `serialization.failure`, `serialization.success`), and a mutating + * Behavior on one of those produces a patch that's genuine user work + * even though it arrived read-only. + */ + intakeRepair: boolean } type RelayListener = (event: EditorEmittedEvent) => void diff --git a/packages/editor/src/editor/remote-patches.ts b/packages/editor/src/editor/remote-patches.ts index 0d93b83bc..c269e7d3b 100644 --- a/packages/editor/src/editor/remote-patches.ts +++ b/packages/editor/src/editor/remote-patches.ts @@ -42,10 +42,11 @@ export function setupRemotePatches({ pluginWithoutHistory(editor, () => { for (const patch of patches) { try { - changed = applyPatch(editor, patch) + const patchChanged = applyPatch(editor, patch) + changed ||= patchChanged if (debug.syncPatch.enabled) { - if (changed) { + if (patchChanged) { debug.syncPatch(`(applied) ${safeStringify(patch, 2)}`) } else { debug.syncPatch(`(ignored) ${safeStringify(patch, 2)}`) @@ -62,9 +63,19 @@ export function setupRemotePatches({ }) if (changed) { normalize(editor) - editor.onChange() } }) + + if (changed) { + // No superseded-repair drop here, unlike a value sync: patches are + // deltas, not full snapshots, so a batch can change engine state + // without superseding a held repair, and the engine's own tree is + // already repaired, so normalization re-mints nothing. Dropping here + // orphans the store's only copy of the fix. A batch that genuinely + // supersedes a held repair (another client's key mint) leaves it to + // flush anyway, converging by last write like any concurrent mint. + editor.onChange() + } } const handlePatches = ({patches}: {patches: Patch[]}) => { diff --git a/packages/editor/src/editor/subscriber.patch-generation.ts b/packages/editor/src/editor/subscriber.patch-generation.ts index cdc7a5e60..189a4aa32 100644 --- a/packages/editor/src/editor/subscriber.patch-generation.ts +++ b/packages/editor/src/editor/subscriber.patch-generation.ts @@ -5,6 +5,7 @@ import { unset, type Patch, } from '@portabletext/patches' +import {hasRemoteFrame, isInNormalization} from '../engine/core/apply-context' import {subscribeToOperations} from '../engine/core/operation-channel' import {isEqualValues} from '../internal-utils/equality' import { @@ -158,12 +159,16 @@ export function subscribePatchGeneration({ // Emit all patches if (patches.length > 0) { + const intakeRepair = + isInNormalization(event.context) && hasRemoteFrame(event.context) + for (const patch of patches) { editorActor.send({ type: 'internal.patch', patch: {...patch, origin: 'local'}, operationId: event.undoStepId, value: editor.snapshot.context.value, + intakeRepair, }) } } diff --git a/packages/editor/src/editor/subscriber.repair-journal.test.ts b/packages/editor/src/editor/subscriber.repair-journal.test.ts new file mode 100644 index 000000000..6ca201c35 --- /dev/null +++ b/packages/editor/src/editor/subscriber.repair-journal.test.ts @@ -0,0 +1,375 @@ +import type {PortableTextBlock} from '@portabletext/schema' +import {describe, expect, test} from 'vitest' +import type {EngineOperation} from '../engine/interfaces/operation' +import { + updateRepairJournal, + type RepairJournalEntry, +} from './subscriber.repair-journal' + +/** + * `updateRepairJournal` is what `subscribeRepairJournal` calls per + * operation. This pins its two responsibilities directly, against plain + * fixture data, without needing a running engine: chaining a block's + * successive intake repairs into one entry that spans the whole repair + * session, and retiring an entry the moment anything other than an intake + * repair touches its block. + */ + +type Journal = Map + +function journal(...entries: Array<[string, RepairJournalEntry]>): Journal { + return new Map(entries) +} + +function record( + journalMap: Journal, + operation: Exclude, + beforeValue: ReadonlyArray, + afterValue: ReadonlyArray, + blockIndexMap: ReadonlyMap = new Map(), +): void { + updateRepairJournal(journalMap, operation, { + isIntakeRepair: true, + beforeValue, + afterValue, + blockIndexMap, + }) +} + +function retire( + journalMap: Journal, + operation: Exclude, + beforeValue: ReadonlyArray = [], +): void { + updateRepairJournal(journalMap, operation, { + isIntakeRepair: false, + beforeValue, + afterValue: [], + blockIndexMap: new Map(), + }) +} + +describe('updateRepairJournal: chain merging', () => { + test('a keyless block minted a key, then repaired again, is one entry carrying the original before-shape', () => { + const journalMap: Journal = journal() + + const keylessBlock: PortableTextBlock = { + _type: 'block', + children: [], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock + const keyedEmptyBlock: PortableTextBlock = { + _key: 'k2', + _type: 'block', + children: [], + markDefs: [], + style: 'normal', + } + + // First repair: mint the missing `_key` (numeric segment, the block + // itself resolved directly by index). + record( + journalMap, + {type: 'set', path: [0, '_key'], value: 'k2'}, + [keylessBlock], + [keyedEmptyBlock], + ) + + expect(journalMap).toEqual( + journal(['k2', {beforeShape: keylessBlock, afterShape: keyedEmptyBlock}]), + ) + + // Second repair on the same block, now keyed: insert the placeholder + // span. Resolved via `blockIndexMap` since the segment is keyed. + const repairedBlock: PortableTextBlock = { + _key: 'k2', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + } + + record( + journalMap, + { + type: 'insert', + path: [{_key: 'k2'}, 'children', 0], + node: {_key: 's0', _type: 'span', text: '', marks: []}, + position: 'before', + }, + [keyedEmptyBlock], + [repairedBlock], + new Map([['k2', 0]]), + ) + + // One entry, not two: `beforeShape` still points at the very first + // (keyless) shape, `afterShape` at the latest repair's result. + expect(journalMap).toEqual( + journal(['k2', {beforeShape: keylessBlock, afterShape: repairedBlock}]), + ) + }) + + test('a chained repair that changes the top-level key re-keys the map entry', () => { + const journalMap: Journal = journal() + + const duplicateKeyBlock: PortableTextBlock = { + _key: 'b0', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + } + const reKeyedBlock: PortableTextBlock = { + ...duplicateKeyBlock, + _key: 'k2', + } + + record( + journalMap, + {type: 'set', path: [1, '_key'], value: 'k2'}, + [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + duplicateKeyBlock, + ], + [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + reKeyedBlock, + ], + ) + + expect(journalMap).toEqual( + journal([ + 'k2', + {beforeShape: duplicateKeyBlock, afterShape: reKeyedBlock}, + ]), + ) + }) + + test('a repair on a block matching no chain opens a new entry', () => { + const journalMap: Journal = journal([ + 'other', + { + beforeShape: {_key: 'other', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'other', _type: 'block'} as PortableTextBlock, + }, + ]) + + const beforeBlock: PortableTextBlock = { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock + const afterBlock: PortableTextBlock = { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + } + + record( + journalMap, + {type: 'set', path: [{_key: 'b0'}, 'children', 0, '_key'], value: 'k2'}, + [beforeBlock], + [afterBlock], + new Map([['b0', 0]]), + ) + + expect(journalMap.get('b0')).toEqual({ + beforeShape: beforeBlock, + afterShape: afterBlock, + }) + expect(journalMap.size).toBe(2) + }) + + test('the block absent from `beforeValue` is skipped: no entry is recorded', () => { + const journalMap: Journal = journal() + + record( + journalMap, + {type: 'set', path: [0, '_key'], value: 'k2'}, + [], + [{_key: 'k2', _type: 'block'} as PortableTextBlock], + ) + + expect(journalMap.size).toBe(0) + }) +}) + +describe('updateRepairJournal: retirement', () => { + test('an operation touching a journaled keyed block retires its entry, and leaves the others', () => { + const entryA: RepairJournalEntry = { + beforeShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + } + const entryB: RepairJournalEntry = { + beforeShape: {_key: 'b', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'b', _type: 'block'} as PortableTextBlock, + } + const journalMap: Journal = journal(['a', entryA], ['b', entryB]) + + retire(journalMap, { + type: 'insert.text', + path: [{_key: 'a'}, 'children', {_key: 's0'}], + offset: 0, + text: '!', + }) + + expect(journalMap).toEqual(journal(['b', entryB])) + }) + + test('an operation touching a journaled block by numeric segment retires it via `beforeValue`', () => { + const entry: RepairJournalEntry = { + beforeShape: {_key: 'k2', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'k2', _type: 'block'} as PortableTextBlock, + } + const journalMap: Journal = journal(['k2', entry]) + + retire(journalMap, {type: 'unset', path: [0]}, [ + {_key: 'k2', _type: 'block'} as PortableTextBlock, + ]) + + expect(journalMap.size).toBe(0) + }) + + test("`replaceBlock`'s unset-then-insert sequence retires the replaced block by the inserted node's own key, not the sibling that shifted into its index", () => { + const entryR: RepairJournalEntry = { + beforeShape: {_key: 'r', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'r', _type: 'block'} as PortableTextBlock, + } + const entryS: RepairJournalEntry = { + beforeShape: {_key: 's', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 's', _type: 'block'} as PortableTextBlock, + } + const journalMap: Journal = journal(['r', entryR], ['s', entryS]) + + // `replaceBlock` (sync-machine.ts) unsets the old block by its keyed + // segment, then inserts the replacement at the same numeric index. `S` + // has shifted into that index by the time the insert operation fires, + // so `beforeValue[0]` is `S`, not the block the insert actually touches. + retire(journalMap, {type: 'unset', path: [{_key: 'r'}]}) + retire( + journalMap, + { + type: 'insert', + path: [0], + node: {_key: 'r2', _type: 'block'} as PortableTextBlock, + position: 'before', + }, + [{_key: 's', _type: 'block'} as PortableTextBlock], + ) + + expect(journalMap).toEqual(journal(['s', entryS])) + }) + + test('a root-path set clears the whole journal', () => { + const journalMap: Journal = journal([ + 'a', + { + beforeShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + }, + ]) + + retire(journalMap, {type: 'set', path: [], value: []}) + + expect(journalMap.size).toBe(0) + }) + + test('a root-path unset clears the whole journal', () => { + const journalMap: Journal = journal([ + 'a', + { + beforeShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + }, + ]) + + retire(journalMap, {type: 'unset', path: []}) + + expect(journalMap.size).toBe(0) + }) + + test('does nothing when the journal is empty (mirrors `invalidateVerifiedGroups`)', () => { + const journalMap: Journal = journal() + + retire(journalMap, {type: 'unset', path: [{_key: 'a'}]}) + + expect(journalMap.size).toBe(0) + }) + + test('an operation touching a key the journal does not hold leaves it untouched', () => { + const entry: RepairJournalEntry = { + beforeShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + afterShape: {_key: 'a', _type: 'block'} as PortableTextBlock, + } + const journalMap: Journal = journal(['a', entry]) + + retire(journalMap, { + type: 'unset', + path: [{_key: 'unrelated'}, 'children', {_key: 's0'}], + }) + + expect(journalMap).toEqual(journal(['a', entry])) + }) +}) + +describe('updateRepairJournal: FIFO cap', () => { + test('a 101st distinct entry evicts the oldest one', () => { + const journalMap: Journal = new Map() + + for (let index = 0; index < 100; index++) { + const key = `k${index}` + journalMap.set(key, { + beforeShape: {_key: key, _type: 'block'} as PortableTextBlock, + afterShape: {_key: key, _type: 'block'} as PortableTextBlock, + }) + } + + const beforeBlock: PortableTextBlock = { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock + const afterBlock: PortableTextBlock = { + _key: 'b0', + _type: 'block', + children: [{_key: 'kNew', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + } + + record( + journalMap, + {type: 'set', path: [{_key: 'b0'}, 'children', 0, '_key'], value: 'kNew'}, + [beforeBlock], + [afterBlock], + new Map([['b0', 0]]), + ) + + expect(journalMap.size).toBe(100) + expect(journalMap.has('k0')).toBe(false) + expect(journalMap.has('k1')).toBe(true) + expect(journalMap.get('b0')).toEqual({ + beforeShape: beforeBlock, + afterShape: afterBlock, + }) + }) +}) diff --git a/packages/editor/src/editor/subscriber.repair-journal.ts b/packages/editor/src/editor/subscriber.repair-journal.ts new file mode 100644 index 000000000..7df9e38d9 --- /dev/null +++ b/packages/editor/src/editor/subscriber.repair-journal.ts @@ -0,0 +1,156 @@ +import type {PortableTextBlock} from '@portabletext/schema' +import {hasRemoteFrame, isInNormalization} from '../engine/core/apply-context' +import {subscribeToOperations} from '../engine/core/operation-channel' +import type {EngineOperation} from '../engine/interfaces/operation' +import {deepEqualJson} from '../internal-utils/deep-equal-json' +import type {PortableTextEditorEngine} from '../types/editor-engine' +import {isKeyedSegment} from '../utils/util.is-keyed-segment' + +const REPAIR_JOURNAL_CAP = 100 + +type RepairJournal = PortableTextEditorEngine['repairJournal'] +export type RepairJournalEntry = + RepairJournal extends Map ? Entry : never +type JournalableOperation = Exclude + +/** + * Wires {@link updateRepairJournal} to the operation channel, wired like + * `subscribeUpdateValue` (see `create-editor-engine`). + */ +export function subscribeRepairJournal( + editor: PortableTextEditorEngine, +): () => void { + return subscribeToOperations(editor, (event) => { + if (event.operation.type === 'set.selection') { + return + } + + updateRepairJournal(editor.repairJournal, event.operation, { + isIntakeRepair: + isInNormalization(event.context) && hasRemoteFrame(event.context), + beforeValue: event.beforeValue, + afterValue: editor.snapshot.context.value, + blockIndexMap: editor.blockIndexMap, + }) + }) +} + +/** + * Records or retires intake-repair entries for one operation. An intake + * repair (normalization firing while a remote frame is on the stack: the + * exact signature of a repair fixing up content the engine just adopted, + * never a local edit's own normalization) either continues the chain + * already open on its owning top-level block or opens a new one. Any + * other operation retires the entry it touches: a local edit reaching the + * block, or a later remote write, both mean the journaled verdict no + * longer describes the block's state. + */ +export function updateRepairJournal( + journal: RepairJournal, + operation: JournalableOperation, + { + isIntakeRepair, + beforeValue, + afterValue, + blockIndexMap, + }: { + isIntakeRepair: boolean + beforeValue: ReadonlyArray + afterValue: ReadonlyArray + blockIndexMap: ReadonlyMap + }, +): void { + if (!isIntakeRepair) { + retireTouchedEntry(journal, operation, beforeValue) + return + } + + recordRepair(journal, operation, beforeValue, afterValue, blockIndexMap) +} + +function retireTouchedEntry( + journal: RepairJournal, + operation: JournalableOperation, + beforeValue: ReadonlyArray, +): void { + if (journal.size === 0) { + return + } + + if (operation.path.length === 0) { + journal.clear() + return + } + + const segment = operation.path[0]! + const key = isKeyedSegment(segment) + ? segment._key + : typeof segment === 'number' + ? operation.type === 'insert' + ? operation.node._key + : beforeValue[segment]?._key + : undefined + + if (key !== undefined) { + journal.delete(key) + } +} + +function recordRepair( + journal: RepairJournal, + operation: JournalableOperation, + beforeValue: ReadonlyArray, + afterValue: ReadonlyArray, + blockIndexMap: ReadonlyMap, +): void { + const segment = operation.path[0] + if (segment === undefined) { + return + } + + const index = + typeof segment === 'number' + ? segment + : isKeyedSegment(segment) + ? blockIndexMap.get(segment._key) + : undefined + + if (index === undefined) { + return + } + + const beforeBlock = beforeValue[index] + if (!beforeBlock) { + return + } + + const afterBlock = afterValue[index] + if (!afterBlock) { + return + } + + const chained = journal.get(beforeBlock._key) + + if (chained && deepEqualJson(chained.afterShape, beforeBlock)) { + if (afterBlock._key !== beforeBlock._key) { + journal.delete(beforeBlock._key) + } + journal.set(afterBlock._key, { + beforeShape: chained.beforeShape, + afterShape: afterBlock, + }) + return + } + + if (!journal.has(afterBlock._key) && journal.size >= REPAIR_JOURNAL_CAP) { + const oldestKey = journal.keys().next().value + if (oldestKey !== undefined) { + journal.delete(oldestKey) + } + } + + journal.set(afterBlock._key, { + beforeShape: beforeBlock, + afterShape: afterBlock, + }) +} diff --git a/packages/editor/src/editor/sync-machine.test.ts b/packages/editor/src/editor/sync-machine.test.ts index 02a6445d7..5bc79471f 100644 --- a/packages/editor/src/editor/sync-machine.test.ts +++ b/packages/editor/src/editor/sync-machine.test.ts @@ -1,15 +1,18 @@ import {compileSchema, defineSchema} from '@portabletext/schema' import {createTestKeyGenerator} from '@portabletext/test' import {describe, expect, test, vi} from 'vitest' -import {createActor} from 'xstate' +import {type AnyEventObject, createActor, fromCallback} from 'xstate' import {createBehaviorApiPlugin} from '../engine-plugins/engine-plugin.behavior-api' import {updateSelectionPlugin} from '../engine-plugins/engine-plugin.update-selection' import type {ApplyContextFrame} from '../engine/core/apply-context' import {subscribeToOperations} from '../engine/core/operation-channel' import {createEditor} from '../engine/create-editor' +import {withoutNormalizing} from '../engine/editor/without-normalizing' +import type {Node} from '../engine/interfaces/node' +import type {EngineOperation} from '../engine/interfaces/operation' import type {PortableTextEditorEngine} from '../types/editor-engine' import {editorMachine} from './editor-machine' -import {syncMachine} from './sync-machine' +import {syncMachine, updateBlock} from './sync-machine' function createTestEngine(keyGenerator: () => string) { const schema = compileSchema(defineSchema({})) @@ -17,6 +20,7 @@ function createTestEngine(keyGenerator: () => string) { e.containers = new Map() e.blockIndexMap = new Map() e.verifiedUniqueChildGroups = new Set() + e.repairJournal = new Map() e.snapshot = { blockIndexMap: e.blockIndexMap, context: { @@ -65,7 +69,6 @@ describe('sync machine', () => { initialValue: undefined, keyGenerator, schema, - readOnly: false, editorEngine: editor, }, }) @@ -90,4 +93,660 @@ describe('sync machine', () => { expect(remoteFrames).toEqual([{kind: 'remote', source: 'update-value'}]) }) + + test('`updateBlock` replaces children wholesale instead of building a `{_key: undefined}` path when a child lacks a usable key', () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const blockKey = keyGenerator() + const fooKey = keyGenerator() + + const oldEngineBlock: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + { + _type: 'span', + _key: undefined as unknown as string, + text: 'bar', + marks: [], + }, + ], + } + editor.snapshot.context.value = [oldEngineBlock] + + const appliedOps = updateBlockWithinRemoteFrame({ + editor, + context: { + keyGenerator, + previousValue: undefined, + schema, + }, + oldEngineBlock, + block: { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + {_type: 'span', text: 'baz', marks: []}, + ], + }, + index: 0, + }) + + expect(appliedOps).toEqual([ + { + type: 'set', + path: [{_key: blockKey}, 'markDefs'], + value: [], + inverse: { + type: 'set', + path: [{_key: blockKey}, 'markDefs'], + value: [], + }, + }, + { + type: 'set', + path: [{_key: blockKey}, 'children'], + value: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + {_type: 'span', text: 'baz', marks: []}, + ], + inverse: { + type: 'set', + path: [{_key: blockKey}, 'children'], + value: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + {_type: 'span', text: 'bar', marks: []}, + ], + }, + }, + { + type: 'set', + path: [{_key: blockKey}, 'children', 1, '_key'], + value: 'k2', + inverse: { + type: 'unset', + path: [{_key: blockKey}, 'children', 1, '_key'], + }, + }, + ]) + + expect(editor.snapshot.context.value).toEqual([ + { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + {_type: 'span', _key: 'k2', text: 'baz', marks: []}, + ], + }, + ]) + }) + + test('`updateBlock` replaces children wholesale, and normalization inserts the placeholder span, when the incoming block has no children', () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const blockKey = keyGenerator() + const fooKey = keyGenerator() + const barKey = keyGenerator() + + const oldEngineBlock: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + {_type: 'span', _key: barKey, text: 'bar', marks: []}, + ], + } + editor.snapshot.context.value = [oldEngineBlock] + + const appliedOps = updateBlockWithinRemoteFrame({ + editor, + context: { + keyGenerator, + previousValue: undefined, + schema, + }, + oldEngineBlock, + block: { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [], + }, + index: 0, + }) + + expect(appliedOps).toEqual([ + { + type: 'set', + path: [{_key: blockKey}, 'markDefs'], + value: [], + inverse: { + type: 'set', + path: [{_key: blockKey}, 'markDefs'], + value: [], + }, + }, + { + type: 'set', + path: [{_key: blockKey}, 'children'], + value: [], + inverse: { + type: 'set', + path: [{_key: blockKey}, 'children'], + value: [ + {_type: 'span', _key: fooKey, text: 'foo', marks: []}, + {_type: 'span', _key: barKey, text: 'bar', marks: []}, + ], + }, + }, + { + type: 'insert', + path: [{_key: blockKey}, 'children', 0], + node: {_type: 'span', _key: 'k3', text: '', marks: []}, + position: 'before', + }, + ]) + + expect(editor.snapshot.context.value).toEqual([ + { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k3', text: '', marks: []}], + }, + ]) + }) + + test('a repair-journal hit on a stale echo applies zero operations and keeps the entry', async () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const blockKey = keyGenerator() + + const beforeShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: '', marks: []} as unknown as Node], + } + const afterShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: '', marks: []}], + } + editor.snapshot.context.value = [afterShape] + editor.repairJournal.set(blockKey, {beforeShape, afterShape}) + + const appliedOps: Array = [] + subscribeToOperations(editor, (event) => { + if (event.operation.type !== 'set.selection') { + appliedOps.push(event.operation) + } + }) + + let doneSyncingCount = 0 + const actor = createActor(syncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: editor, + }, + }) + actor.on('done syncing value', () => { + doneSyncingCount++ + }) + actor.start() + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(1) + }) + + // The stale echo: `beforeShape` verbatim, the shape the engine held + // before this repair. + actor.send({type: 'update value', value: [beforeShape as any]}) + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(2) + }) + + expect(appliedOps).toEqual([]) + expect(editor.snapshot.context.value).toEqual([afterShape]) + expect(editor.repairJournal.get(blockKey)).toEqual({ + beforeShape, + afterShape, + }) + }) + + test('a repair-journal near-miss (the engine block has since moved on from the journaled `afterShape`) applies the normal op stream and retires the entry', async () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const blockKey = keyGenerator() + + const beforeShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: 'hello', marks: []} as unknown as Node], + } + // One field off the engine's actual current block (`text`): neither an + // ack (doesn't match the inbound block either) nor an echo (the + // engine's current block no longer matches it). + const staleAfterShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: 'mismatch', marks: []}], + } + const currentEngineBlock: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: 'hello', marks: []}], + } + editor.snapshot.context.value = [currentEngineBlock] + editor.repairJournal.set(blockKey, { + beforeShape, + afterShape: staleAfterShape, + }) + + const appliedOps: Array = [] + subscribeToOperations(editor, (event) => { + if (event.operation.type !== 'set.selection') { + appliedOps.push(event.operation) + } + }) + + let doneSyncingCount = 0 + const actor = createActor(syncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: editor, + }, + }) + actor.on('done syncing value', () => { + doneSyncingCount++ + }) + actor.start() + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(1) + }) + + actor.send({type: 'update value', value: [beforeShape as any]}) + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(2) + }) + + expect(appliedOps.length).toBeGreaterThan(0) + expect(editor.repairJournal.has(blockKey)).toBe(false) + }) + + test("a pass reports each block's echo independently in a set, not a single pass-wide flag", async () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const echoingBlockKey = keyGenerator() + const ackingBlockKey = keyGenerator() + + const echoingBeforeShape: Node = { + _type: 'block', + _key: echoingBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: '', marks: []} as unknown as Node], + } + const echoingAfterShape: Node = { + _type: 'block', + _key: echoingBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: '', marks: []}], + } + const ackingBeforeShape: Node = { + _type: 'block', + _key: ackingBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: '', marks: []} as unknown as Node], + } + const ackingAfterShape: Node = { + _type: 'block', + _key: ackingBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k3', text: '', marks: []}], + } + + editor.snapshot.context.value = [echoingAfterShape, ackingAfterShape] + editor.repairJournal.set(echoingBlockKey, { + beforeShape: echoingBeforeShape, + afterShape: echoingAfterShape, + }) + editor.repairJournal.set(ackingBlockKey, { + beforeShape: ackingBeforeShape, + afterShape: ackingAfterShape, + }) + + const inboundStateAppliedEvents: Array<{echoedBlockKeys: Set}> = [] + let doneSyncingCount = 0 + const actor = createActor(syncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: editor, + }, + }) + actor.on('inbound state applied', (event) => { + inboundStateAppliedEvents.push(event) + }) + actor.on('done syncing value', () => { + doneSyncingCount++ + }) + actor.start() + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(1) + }) + + // `echoingBlockKey`'s snapshot still carries its pre-repair shape (an + // echo); `ackingBlockKey`'s snapshot carries exactly the repaired + // shape (the host learned it). + actor.send({ + type: 'update value', + value: [echoingBeforeShape as any, ackingAfterShape as any], + }) + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(2) + }) + + expect(inboundStateAppliedEvents).toEqual([ + { + type: 'inbound state applied', + echoedBlockKeys: new Set([echoingBlockKey]), + }, + ]) + }) + + test('`inbound state applied` fires even on a pass where every block echoed', async () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const blockKey = keyGenerator() + + const beforeShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: '', marks: []} as unknown as Node], + } + const afterShape: Node = { + _type: 'block', + _key: blockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: '', marks: []}], + } + + editor.snapshot.context.value = [afterShape] + editor.repairJournal.set(blockKey, {beforeShape, afterShape}) + + const inboundStateAppliedEvents: Array<{echoedBlockKeys: Set}> = [] + let doneSyncingCount = 0 + const actor = createActor(syncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: editor, + }, + }) + actor.on('inbound state applied', (event) => { + inboundStateAppliedEvents.push(event) + }) + actor.on('done syncing value', () => { + doneSyncingCount++ + }) + actor.start() + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(1) + }) + + actor.send({type: 'update value', value: [beforeShape as any]}) + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(2) + }) + + expect(inboundStateAppliedEvents).toEqual([ + {type: 'inbound state applied', echoedBlockKeys: new Set([blockKey])}, + ]) + }) + + test('a pass that aborts on an invalid block never emits `inbound state applied`, leaving the unexamined block unjudged', async () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + const validBlockKey = keyGenerator() + const echoingBlockKey = keyGenerator() + + const validBlock: Node = { + _type: 'block', + _key: validBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k2', text: 'a', marks: []}], + } + const echoingBeforeShape: Node = { + _type: 'block', + _key: echoingBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', text: '', marks: []} as unknown as Node], + } + const echoingAfterShape: Node = { + _type: 'block', + _key: echoingBlockKey, + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k3', text: '', marks: []}], + } + + // `echoingBlockKey` sits second, already repaired by an earlier pass + // and still echoed by the host (unresolved journal entry). + editor.snapshot.context.value = [validBlock, echoingAfterShape] + editor.repairJournal.set(echoingBlockKey, { + beforeShape: echoingBeforeShape, + afterShape: echoingAfterShape, + }) + + const inboundStateAppliedEvents: Array = [] + const invalidValueEvents: Array = [] + let doneSyncingCount = 0 + const actor = createActor(syncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: editor, + }, + }) + actor.on('inbound state applied', (event) => { + inboundStateAppliedEvents.push(event) + }) + actor.on('invalid value', (event) => { + invalidValueEvents.push(event) + }) + actor.on('done syncing value', () => { + doneSyncingCount++ + }) + actor.start() + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(1) + }) + + // The first block now carries a human-decision defect (an unknown + // block type the schema never registered): the walk breaks there and + // never reaches `echoingBlockKey`, still carrying its pre-repair + // shape. + actor.send({ + type: 'update value', + value: [{_key: 'bad', _type: 'image'} as any, echoingBeforeShape as any], + }) + + await vi.waitFor(() => { + expect(doneSyncingCount).toBe(2) + }) + + expect(invalidValueEvents).toHaveLength(1) + expect(inboundStateAppliedEvents).toEqual([]) + }) + + test('`syncing` entry emits `inbound sync started`, including on the reenter transition a value arriving mid-pass triggers', () => { + const keyGenerator = createTestKeyGenerator() + const {editor, schema} = createTestEngine(keyGenerator) + + // A controllable stand-in for the real `sync value` actor: it never + // finishes on its own, so the test drives each pass's `done syncing` + // by hand and can send a new value in between. + const pendingSyncs: Array<{ + sendBack: (event: AnyEventObject) => void + value: unknown + }> = [] + const controlledSyncMachine = syncMachine.provide({ + actors: { + 'sync value': fromCallback(({sendBack, input}) => { + pendingSyncs.push({ + sendBack, + value: (input as {value: unknown}).value, + }) + }), + }, + }) + + let inboundSyncStartedCount = 0 + const actor = createActor(controlledSyncMachine, { + input: { + initialValue: undefined, + keyGenerator, + schema, + editorEngine: editor, + }, + }) + actor.on('inbound sync started', () => { + inboundSyncStartedCount++ + }) + actor.start() + + expect(inboundSyncStartedCount).toBe(0) + + const valueA = [ + { + _type: 'block', + _key: 'b1', + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 's1', text: 'a', marks: []}], + }, + ] + const valueB = [ + { + _type: 'block', + _key: 'b1', + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 's1', text: 'b', marks: []}], + }, + ] + + actor.send({type: 'update value', value: valueA as any}) + + // The first pass starts syncing `valueA` and never finishes on its own. + expect(inboundSyncStartedCount).toBe(1) + expect(pendingSyncs).toHaveLength(1) + + // A newer value arrives while that pass is still in progress. + actor.send({type: 'update value', value: valueB as any}) + + expect(inboundSyncStartedCount).toBe(1) + expect(pendingSyncs).toHaveLength(1) + + // The in-progress pass finishes syncing the stale `valueA`, but a + // newer value is now pending: the machine reenters `syncing` for a + // second pass, which fires `inbound sync started` again. + pendingSyncs[0]?.sendBack({ + type: 'done syncing', + value: valueA, + changed: true, + completed: true, + echoedBlockKeys: new Set(), + }) + + expect(inboundSyncStartedCount).toBe(2) + expect(pendingSyncs).toHaveLength(2) + + // The second pass finishes syncing the now-current value: nothing is + // pending, so the machine settles into `idle` without starting a + // third pass. + pendingSyncs[1]?.sendBack({ + type: 'done syncing', + value: valueB, + changed: true, + completed: true, + echoedBlockKeys: new Set(), + }) + + expect(inboundSyncStartedCount).toBe(2) + }) }) + +/** + * Mirrors how `syncBlock` always invokes `updateBlock`: inside a remote + * frame (suppresses cosmetic normalization unrelated to this fallback) and + * with normalization deferred until the wrapper exits. + */ +function updateBlockWithinRemoteFrame( + args: Omit[0], 'editorEngine'> & { + editor: PortableTextEditorEngine + }, +) { + const {editor, ...updateBlockArgs} = args + + editor.applyContext = [{kind: 'remote', source: 'update-value'}] + + const appliedOps: Array = [] + const originalApply = editor.apply.bind(editor) + editor.apply = (op: EngineOperation) => { + appliedOps.push(op) + return originalApply(op) + } + + withoutNormalizing(editor, () => { + updateBlock({...updateBlockArgs, editorEngine: editor}) + }) + + return appliedOps +} diff --git a/packages/editor/src/editor/sync-machine.ts b/packages/editor/src/editor/sync-machine.ts index 3cfd095ca..d328c56d1 100644 --- a/packages/editor/src/editor/sync-machine.ts +++ b/packages/editor/src/editor/sync-machine.ts @@ -1,4 +1,3 @@ -import {applyAll, type Patch} from '@portabletext/patches' import {isSpan, isTextBlock, type PortableTextBlock} from '@portabletext/schema' import type {ActorRefFrom} from 'xstate' import { @@ -6,6 +5,7 @@ import { assertEvent, assign, emit, + enqueueActions, fromCallback, not, raise, @@ -26,6 +26,7 @@ import type {Node} from '../engine/interfaces/node' import {applyNodeProperties} from '../internal-utils/apply-node-properties' import {applyDeselect, applySelect} from '../internal-utils/apply-selection' import {debug} from '../internal-utils/debug' +import {deepEqualJson} from '../internal-utils/deep-equal-json' import {deleteRange} from '../internal-utils/delete-range' import { isEqualBlocks, @@ -43,10 +44,6 @@ import {isKeyedSegment} from '../utils/util.is-keyed-segment' import type {EditorSchema} from './editor-schema' type SyncValueEvent = - | { - type: 'patch' - patch: Patch - } | { type: 'invalid value' resolution: InvalidValueResolution | null @@ -60,6 +57,13 @@ type SyncValueEvent = type: 'done syncing' value: Array | undefined changed: boolean + // Whether the pass walked every block. `false` when `syncBlock` hit + // a block a human has to resolve and the walk broke off early: the + // remaining blocks were never examined, so `echoedBlockKeys` below + // can't speak for them, and the pass must not emit `inbound state + // applied` (see 'emit inbound state applied' below). + completed: boolean + echoedBlockKeys: Set } const syncValueCallback: CallbackLogicFunction< @@ -69,7 +73,6 @@ const syncValueCallback: CallbackLogicFunction< context: { keyGenerator: () => string previousValue: Array | undefined - readOnly: boolean schema: EditorSchema } editorEngine: PortableTextEditorEngine @@ -111,7 +114,6 @@ export const syncMachine = setup({ initialValueSynced: boolean keyGenerator: () => string schema: EditorSchema - readOnly: boolean editorEngine: PortableTextEditorEngine pendingValue: Array | undefined previousValue: Array | undefined @@ -120,7 +122,6 @@ export const syncMachine = setup({ initialValue: Array | undefined keyGenerator: () => string schema: EditorSchema - readOnly: boolean editorEngine: PortableTextEditorEngine }, events: {} as @@ -128,30 +129,18 @@ export const syncMachine = setup({ type: 'update value' value: Array | undefined } - | { - type: 'update readOnly' - readOnly: boolean - } | SyncValueEvent, emitted: {} as - | PickFromUnion< - SyncValueEvent, - 'type', - 'invalid value' | 'patch' | 'value changed' - > + | PickFromUnion | {type: 'done syncing value'} - | {type: 'syncing value'}, + | {type: 'syncing value'} + | {type: 'inbound sync started'} + | {type: 'inbound state applied'; echoedBlockKeys: Set}, }, actions: { 'assign initial value synced': assign({ initialValueSynced: true, }), - 'assign readOnly': assign({ - readOnly: ({event}) => { - assertEvent(event, 'update readOnly') - return event.readOnly - }, - }), 'assign pending value': assign({ pendingValue: ({event}) => { assertEvent(event, 'update value') @@ -181,6 +170,48 @@ export const syncMachine = setup({ 'emit syncing value': emit({ type: 'syncing value', }), + // `emit`, not a plain action, so XState defers delivery behind `emit + // done syncing value`'s own synchronous fallout (the editor machine + // relaying this pass's patches to the mutation batcher), landing only + // once that fallout settles. A fresh editor's first sync holds those + // patches behind the editor machine's `setting up` state until that + // relay runs, so firing this any earlier would drop superseded + // repairs before they arrive and misattribute them to whichever pass + // drops them next. + // + // Fires on every settle that completed its walk, carrying the pass's + // echoed-block-keys set: a block in that set still echoes its + // pre-repair shape (the engine's repair journal matched it against a + // `beforeShape` while the engine still holds the `afterShape`), so + // the mutation batcher's `dropSupersededRepairs` keeps that block's + // held repair instead of dropping it as superseded when nothing has + // actually superseded it. Skipped entirely when the pass aborted on + // an invalid block: the walk never reached the remaining blocks, so + // it has no echoed-block-keys verdict for them, and the settle signal + // must not authorize `dropSupersededRepairs` to treat their held + // repairs as superseded when they were simply never examined. + 'emit inbound state applied': enqueueActions(({event, enqueue}) => { + assertEvent(event, 'done syncing') + if (!event.completed) { + return + } + enqueue.emit({ + type: 'inbound state applied' as const, + echoedBlockKeys: event.echoedBlockKeys, + }) + }), + // Bumps the mutation batcher's generation before this pass's own + // invoked `sync value` actor can mint anything: XState resolves a + // state's `entry` actions before spawning its `invoke`d actors (both + // run as part of entering the state, entry first), so this emission + // is guaranteed to land before the pass's first repair. A repair + // minted between passes (remote-patch fallout, outside any settling + // pass) still carries the generation from before this bump, which is + // what lets the next pass's `dropSupersededRepairs` tell it apart + // from that pass's own mints. + 'emit inbound sync started': emit({ + type: 'inbound sync started' as const, + }), }, guards: { 'initial value synced': ({context}) => context.initialValueSynced, @@ -257,7 +288,6 @@ export const syncMachine = setup({ initialValueSynced: false, keyGenerator: input.keyGenerator, schema: input.schema, - readOnly: input.readOnly, editorEngine: input.editorEngine, pendingValue: undefined, previousValue: undefined, @@ -267,11 +297,6 @@ export const syncMachine = setup({ return {type: 'update value', value: context.initialValue} }), ], - on: { - 'update readOnly': { - actions: ['assign readOnly'], - }, - }, initial: 'idle', states: { idle: { @@ -374,6 +399,7 @@ export const syncMachine = setup({ debug.syncValue('entry: syncing->syncing') }, 'emit syncing value', + 'emit inbound sync started', ], exit: [ () => { @@ -389,7 +415,6 @@ export const syncMachine = setup({ context: { keyGenerator: context.keyGenerator, previousValue: context.previousValue, - readOnly: context.readOnly, schema: context.schema, }, editorEngine: context.editorEngine, @@ -403,9 +428,6 @@ export const syncMachine = setup({ guard: 'is new value', actions: ['assign pending value'], }, - 'patch': { - actions: [emit(({event}) => event)], - }, 'invalid value': { actions: [emit(({event}) => event)], }, @@ -419,6 +441,7 @@ export const syncMachine = setup({ 'assign previous value', 'record synced value on engine', 'assign initial value synced', + 'emit inbound state applied', ], target: 'syncing', reenter: true, @@ -430,6 +453,7 @@ export const syncMachine = setup({ 'assign previous value', 'record synced value on engine', 'assign initial value synced', + 'emit inbound state applied', ], }, ], @@ -448,7 +472,6 @@ async function updateValue({ context: { keyGenerator: () => string previousValue: Array | undefined - readOnly: boolean schema: EditorSchema } sendBack: (event: SyncValueEvent) => void @@ -459,6 +482,18 @@ async function updateValue({ let doneSyncing = false let isChanged = false let isValid = true + const echoedBlockKeys = new Set() + + const finishSyncing = () => { + doneSyncing = true + sendBack({ + type: 'done syncing', + value, + changed: isChanged, + completed: isValid, + echoedBlockKeys, + }) + } const hadSelection = !!editorEngine.snapshot.context.selection // `streamBlocks` is true only for a fresh editor's first value sync @@ -500,7 +535,7 @@ async function updateValue({ ] of getStreamedBlocks({ value, })) { - const {blockChanged, blockValid} = syncBlock({ + const {blockChanged, blockValid, echoedBlockKey} = syncBlock({ context, sendBack, block: currentBlock, @@ -512,6 +547,9 @@ async function updateValue({ isChanged = blockChanged || isChanged isValid = isValid && blockValid + if (echoedBlockKey !== undefined) { + echoedBlockKeys.add(echoedBlockKey) + } if (!isValid) { break @@ -537,7 +575,7 @@ async function updateValue({ let index = 0 for (const block of value) { - const {blockChanged, blockValid} = syncBlock({ + const {blockChanged, blockValid, echoedBlockKey} = syncBlock({ context, sendBack, block, @@ -549,6 +587,9 @@ async function updateValue({ isChanged = blockChanged || isChanged isValid = isValid && blockValid + if (echoedBlockKey !== undefined) { + echoedBlockKeys.add(echoedBlockKey) + } if (!blockValid) { break @@ -562,9 +603,7 @@ async function updateValue({ if (!isValid) { debug.syncValue('Invalid value, returning') - doneSyncing = true - - sendBack({type: 'done syncing', value, changed: isChanged}) + finishSyncing() return } @@ -583,9 +622,7 @@ async function updateValue({ value, }) - doneSyncing = true - - sendBack({type: 'done syncing', value, changed: isChanged}) + finishSyncing() return } @@ -604,9 +641,7 @@ async function updateValue({ debug.syncValue('remote value and local value are equal, no need to sync') } - doneSyncing = true - - sendBack({type: 'done syncing', value, changed: isChanged}) + finishSyncing() } async function* getStreamedBlocks({value}: {value: Array}) { @@ -712,7 +747,6 @@ function syncBlock({ context: { keyGenerator: () => string previousValue: Array | undefined - readOnly: boolean schema: EditorSchema } sendBack: (event: SyncValueEvent) => void @@ -721,25 +755,24 @@ function syncBlock({ editorEngine: PortableTextEditorEngine value: Array remoteSource: RemoteSyncSource -}) { +}): { + blockChanged: boolean + blockValid: boolean + echoedBlockKey?: string +} { const oldEngineBlock = editorEngine.snapshot.context.value.at(index) const oldBlock = editorEngine.snapshot.context.value.at(index) if (!oldEngineBlock || !oldBlock) { - const validation = validateValue( - [block], - context.schema, - context.keyGenerator, - ) + const validation = validateValue([block], context.schema, index) debug.syncValue( 'Validating and inserting new block in the end of the value', block, ) - if (validation.valid || validation.resolution?.autoResolve) { - const repairedBlock = applyAutoResolution(validation, [block], block) - const engineBlock = toEngineBlock(repairedBlock, { + if (validation.valid) { + const engineBlock = toEngineBlock(block, { schemaTypes: context.schema, }) @@ -776,6 +809,36 @@ function syncBlock({ } } + const journalEntry = editorEngine.repairJournal.get(oldBlock._key) + + if (journalEntry) { + const inboundEngineBlock = toEngineBlock(block, { + schemaTypes: context.schema, + }) + + if (deepEqualJson(inboundEngineBlock, journalEntry.afterShape)) { + // The host has learned the repair: fall through to the ordinary + // diff, which will find nothing left to sync. + editorEngine.repairJournal.delete(oldBlock._key) + } else if ( + deepEqualJson(inboundEngineBlock, journalEntry.beforeShape) && + deepEqualJson(oldBlock, journalEntry.afterShape) + ) { + // The snapshot still echoes the pre-repair shape while the engine + // holds exactly what the repair produced: a stale echo, not a + // genuine edit. Leave the entry in place for the next echo. + return { + blockChanged: false, + blockValid: true, + echoedBlockKey: oldBlock._key, + } + } else { + // Neither an ack nor an echo: something else changed the block. + // The journaled verdict no longer describes it. + editorEngine.repairJournal.delete(oldBlock._key) + } + } + if (isEqualBlocks(context, block, oldBlock)) { return { blockChanged: false, @@ -791,42 +854,11 @@ function syncBlock({ } } const validationValue = [blockToValidate] - const validation = validateValue( - validationValue, - context.schema, - context.keyGenerator, - ) - - // Resolve validations that can be resolved automatically, without involving the user (but only if the value was changed) - if ( - !validation.valid && - validation.resolution?.autoResolve && - validation.resolution?.patches.length > 0 - ) { - // Only apply auto resolution if the value has been populated before and is different from the last one. - if ( - !context.readOnly && - context.previousValue && - context.previousValue !== value - ) { - console.warn( - `${validation.resolution.action} for block with _key '${blockToValidate._key}'. ${validation.resolution?.description}`, - ) - validation.resolution.patches.forEach((patch) => { - sendBack({type: 'patch', patch}) - }) - } - } - - if (validation.valid || validation.resolution?.autoResolve) { - const repairedBlock = applyAutoResolution( - validation, - validationValue, - block, - ) + const validation = validateValue(validationValue, context.schema, index) + if (validation.valid) { if (oldBlock._key === block._key && oldBlock._type === block._type) { - debug.syncValue('Updating block', oldBlock, repairedBlock) + debug.syncValue('Updating block', oldBlock, block) withRemoteChanges(editorEngine, remoteSource, () => { withoutNormalizing(editorEngine, () => { @@ -835,14 +867,14 @@ function syncBlock({ context, editorEngine, oldEngineBlock, - block: repairedBlock, + block, index, }) }) }) }) } else { - debug.syncValue('Replacing block', oldBlock, repairedBlock) + debug.syncValue('Replacing block', oldBlock, block) withRemoteChanges(editorEngine, remoteSource, () => { withoutNormalizing(editorEngine, () => { @@ -850,7 +882,7 @@ function syncBlock({ replaceBlock({ context, editorEngine, - block: repairedBlock, + block, index, }) }) @@ -876,26 +908,6 @@ function syncBlock({ } } -/** - * `validateValue` auto-resolutions must reach the engine as state, not - * only the document as patches. Applying them only outbound forks the - * repair: the document receives the resolution (e.g. a minted child - * `_key`) while the engine holds the un-repaired shape its addressing - * model cannot represent, and normalization then mints a different key - * for the same node. The resolution patches were built from - * `validationValue` itself, so they always apply; the fallback only - * covers the impossible empty result. - */ -function applyAutoResolution( - validation: ReturnType, - validationValue: Array, - block: PortableTextBlock, -): PortableTextBlock { - return !validation.valid && validation.resolution?.autoResolve - ? (applyAll(validationValue, validation.resolution.patches).at(0) ?? block) - : block -} - function replaceBlock({ context, editorEngine, @@ -905,7 +917,6 @@ function replaceBlock({ context: { keyGenerator: () => string previousValue: Array | undefined - readOnly: boolean schema: EditorSchema } editorEngine: PortableTextEditorEngine @@ -957,7 +968,7 @@ function replaceBlock({ } } -function updateBlock({ +export function updateBlock({ context, editorEngine, oldEngineBlock, @@ -967,7 +978,6 @@ function updateBlock({ context: { keyGenerator: () => string previousValue: Array | undefined - readOnly: boolean schema: EditorSchema } editorEngine: PortableTextEditorEngine @@ -1027,8 +1037,19 @@ function updateBlock({ oldKeySet.size === oldKeys.length && oldKeys.some((key, i) => key !== newKeys[i]) && newKeys.every((key) => oldKeySet.has(key)) + // Keyed reconciliation below builds `{_key: child._key}` path segments + // from `engineBlock.children`; a keyless child would produce + // `{_key: undefined}`, so route those cases through the wholesale set. + const hasKeylessChild = engineBlock.children.some( + (child) => typeof child._key !== 'string' || child._key === '', + ) - if (isPureReorder || (newKeys.length > 0 && !hasSharedKeys)) { + if ( + isPureReorder || + (newKeys.length > 0 && !hasSharedKeys) || + engineBlock.children.length === 0 || + hasKeylessChild + ) { debug.syncValue('Replacing children via set') applyNodeProperties(editorEngine, {children: engineBlock.children}, [ {_key: oldEngineBlock._key}, diff --git a/packages/editor/src/internal-utils/deep-equal-json.ts b/packages/editor/src/internal-utils/deep-equal-json.ts new file mode 100644 index 000000000..c8e2d0618 --- /dev/null +++ b/packages/editor/src/internal-utils/deep-equal-json.ts @@ -0,0 +1,53 @@ +/** + * Structural equality over plain JSON values: object keys compare + * order-insensitively, array elements compare order-sensitively by index. + * Engine blocks (and what `toEngineBlock` produces from them) are plain + * JSON, so this is exact for shape comparisons that need every field, not + * just the fields `isEqualBlocks` knows about. + */ +export function deepEqualJson(a: unknown, b: unknown): boolean { + if (a === b) { + return true + } + + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) { + return false + } + for (let index = 0; index < a.length; index++) { + if (!deepEqualJson(a[index], b[index])) { + return false + } + } + return true + } + + if ( + a !== null && + b !== null && + typeof a === 'object' && + typeof b === 'object' + ) { + const recordA = a as Record + const recordB = b as Record + const keysA = Object.keys(recordA) + const keysB = Object.keys(recordB) + + if (keysA.length !== keysB.length) { + return false + } + + for (const key of keysA) { + if ( + !Object.prototype.hasOwnProperty.call(recordB, key) || + !deepEqualJson(recordA[key], recordB[key]) + ) { + return false + } + } + + return true + } + + return false +} diff --git a/packages/editor/src/internal-utils/validateValue.test.ts b/packages/editor/src/internal-utils/validateValue.test.ts new file mode 100644 index 000000000..ddf0e0df4 --- /dev/null +++ b/packages/editor/src/internal-utils/validateValue.test.ts @@ -0,0 +1,76 @@ +import { + compileSchema, + defineSchema, + type PortableTextBlock, +} from '@portabletext/schema' +import {describe, expect, test} from 'vitest' +import {validateValue} from './validateValue' + +describe(validateValue.name, () => { + test('A keyless block anchors its resolution patch on its index, not a `{_key: undefined}` segment that would match the first keyless block', () => { + const schema = compileSchema(defineSchema({})) + const firstKeylessBlock = { + _type: 'block', + children: [{_type: 'span', text: 'foo', marks: []}], + } as unknown as PortableTextBlock + const secondKeylessBlock = { + _type: 'not-a-real-type', + children: [{_type: 'span', text: 'bar', marks: []}], + } as unknown as PortableTextBlock + + const validation = validateValue( + [firstKeylessBlock, secondKeylessBlock], + schema, + ) + + expect(validation).toEqual({ + valid: false, + resolution: { + patches: [{type: 'unset', path: [1]}], + description: "Block at index '1' has invalid _type 'not-a-real-type'", + action: 'Remove the block', + item: secondKeylessBlock, + i18n: { + description: + 'inputs.portable-text.invalid-value.disallowed-type.description', + action: 'inputs.portable-text.invalid-value.disallowed-type.action', + values: {key: undefined, typeName: 'not-a-real-type'}, + }, + }, + value: [firstKeylessBlock, secondKeylessBlock], + }) + }) + + test('A keyless child with an unknown `_type` anchors its description and i18n params on its index, not `undefined`', () => { + const schema = compileSchema(defineSchema({})) + const block = { + _type: 'block', + _key: 'b1', + children: [ + {_type: 'span', _key: 's1', text: 'foo', marks: []}, + {_type: 'not-a-real-type'}, + ], + } as unknown as PortableTextBlock + + const validation = validateValue([block], schema) + + expect(validation).toEqual({ + valid: false, + resolution: { + patches: [{type: 'unset', path: [{_key: 'b1'}, 'children', 1]}], + description: + "Child at index '1' in block with _key 'b1' has invalid '_type' property (not-a-real-type).", + action: 'Remove the object', + item: block, + i18n: { + description: + 'inputs.portable-text.invalid-value.disallowed-child-type.description', + action: + 'inputs.portable-text.invalid-value.disallowed-child-type.action', + values: {key: 'b1', childKey: 1, childType: 'not-a-real-type'}, + }, + }, + value: [block], + }) + }) +}) diff --git a/packages/editor/src/internal-utils/validateValue.ts b/packages/editor/src/internal-utils/validateValue.ts index a7f1d42c9..ff56a1459 100644 --- a/packages/editor/src/internal-utils/validateValue.ts +++ b/packages/editor/src/internal-utils/validateValue.ts @@ -1,11 +1,11 @@ -import {insert, set, setIfMissing, unset} from '@portabletext/patches' +import {set, unset} from '@portabletext/patches' import { - isSpan, isTextBlock, type PortableTextBlock, type PortableTextTextBlock, } from '@portabletext/schema' import type {EditorSchema} from '../editor/editor-schema' +import {hasUsableKey, nodeSegment} from '../paths/node-segment' import {getRootAcceptedTypes} from '../schema/get-root-accepted-types' import type {InvalidValueResolution} from '../types/editor' @@ -18,7 +18,7 @@ interface Validation { export function validateValue( value: PortableTextBlock[] | undefined, types: EditorSchema, - keyGenerator: () => string, + baseIndex = 0, ): Validation { let resolution: InvalidValueResolution | null = null let valid = true @@ -53,7 +53,10 @@ export function validateValue( } } if ( - value.some((blk: PortableTextBlock, index: number): boolean => { + value.some((blk: PortableTextBlock, localIndex: number): boolean => { + // `localIndex` is the position within the (possibly sliced) `value`; + // `baseIndex` recovers the block's real position in the document. + const index = baseIndex + localIndex // Is the block an object? if (typeof blk !== 'object' || blk === null) { resolution = { @@ -71,23 +74,6 @@ export function validateValue( } return true } - // Test that every block has a _key prop - if (!blk._key || typeof blk._key !== 'string') { - resolution = { - patches: [set({...blk, _key: keyGenerator()}, [index])], - description: `Block at index ${index} is missing required _key.`, - action: 'Set the block with a random _key value', - item: blk, - - i18n: { - description: - 'inputs.portable-text.invalid-value.missing-key.description', - action: 'inputs.portable-text.invalid-value.missing-key.action', - values: {index}, - }, - } - return true - } // Test that every block has valid _type if (!blk._type || !validBlockTypes.has(blk._type)) { // Special case where block type is set to default 'block', but the block type is named something else according to the schema. @@ -95,9 +81,11 @@ export function validateValue( const currentBlockTypeName = types.block.name resolution = { patches: [ - set({...blk, _type: currentBlockTypeName}, [{_key: blk._key}]), + set({...blk, _type: currentBlockTypeName}, [ + nodeSegment(blk, index), + ]), ], - description: `Block with _key '${blk._key}' has invalid type name '${blk._type}'. According to the schema, the block type name is '${currentBlockTypeName}'`, + description: `Block ${describeBlockLocation(blk, index)} has invalid type name '${blk._type}'. According to the schema, the block type name is '${currentBlockTypeName}'`, action: `Use type '${currentBlockTypeName}'`, item: blk, @@ -119,9 +107,9 @@ export function validateValue( ) { resolution = { patches: [ - set({...blk, _type: types.block.name}, [{_key: blk._key}]), + set({...blk, _type: types.block.name}, [nodeSegment(blk, index)]), ], - description: `Block with _key '${blk._key}' is missing a type name. According to the schema, the block type name is '${types.block.name}'`, + description: `Block ${describeBlockLocation(blk, index)} is missing a type name. According to the schema, the block type name is '${types.block.name}'`, action: `Use type '${types.block.name}'`, item: blk, @@ -138,8 +126,8 @@ export function validateValue( if (!blk._type) { resolution = { - patches: [unset([{_key: blk._key}])], - description: `Block with _key '${blk._key}' is missing an _type property`, + patches: [unset([nodeSegment(blk, index)])], + description: `Block ${describeBlockLocation(blk, index)} is missing an _type property`, action: 'Remove the block', item: blk, @@ -154,8 +142,8 @@ export function validateValue( } resolution = { - patches: [unset([{_key: blk._key}])], - description: `Block with _key '${blk._key}' has invalid _type '${blk._type}'`, + patches: [unset([nodeSegment(blk, index)])], + description: `Block ${describeBlockLocation(blk, index)} has invalid _type '${blk._type}'`, action: 'Remove the block', item: blk, @@ -175,8 +163,8 @@ export function validateValue( // Test that it has a valid children property (array) if (textBlock.children && !Array.isArray(textBlock.children)) { resolution = { - patches: [set({children: []}, [{_key: textBlock._key}])], - description: `Text block with _key '${textBlock._key}' has a invalid required property 'children'.`, + patches: [set({children: []}, [nodeSegment(textBlock, index)])], + description: `Text block ${describeBlockLocation(textBlock, index)} has a invalid required property 'children'.`, action: 'Reset the children property', item: textBlock, @@ -190,200 +178,121 @@ export function validateValue( } return true } - // Test that children is set and lengthy - if ( - textBlock.children === undefined || - (Array.isArray(textBlock.children) && textBlock.children.length === 0) - ) { - const newSpan = { - _type: types.span.name, - _key: keyGenerator(), - text: '', - marks: [], - } - resolution = { - autoResolve: true, - patches: [ - setIfMissing([], [{_key: blk._key}, 'children']), - insert([newSpan], 'after', [{_key: blk._key}, 'children', 0]), - ], - description: `Children for text block with _key '${blk._key}' is empty.`, - action: 'Insert an empty text', - item: blk, + // A missing or empty `children` array is mechanically fixable + // (the engine inserts an empty span); only run child-level checks + // when there's something to check. + if (Array.isArray(textBlock.children)) { + // Test every child + if ( + textBlock.children.some((child, cIndex: number) => { + if (typeof child !== 'object' || child === null) { + resolution = { + patches: [ + unset([nodeSegment(blk, index), 'children', cIndex]), + ], + description: `Child at index '${cIndex}' in block ${describeBlockLocation(blk, index)} is not an object.`, + action: 'Remove the item', + item: blk, - i18n: { - description: - 'inputs.portable-text.invalid-value.empty-children.description', - action: - 'inputs.portable-text.invalid-value.empty-children.action', - values: {key: blk._key}, - }, - } - return true - } - - const allUsedMarks = [ - ...new Set( - textBlock.children - .filter((child) => isSpan({schema: types}, child)) - .flatMap((cld) => cld.marks || []), - ), - ] - - // Test that all markDefs are in use (remove orphaned markDefs) - if (Array.isArray(blk.markDefs) && blk.markDefs.length > 0) { - const unusedMarkDefs: string[] = [ - ...new Set( - blk.markDefs - .map((def) => def._key) - .filter((key) => !allUsedMarks.includes(key)), - ), - ] - if (unusedMarkDefs.length > 0) { - resolution = { - autoResolve: true, - patches: unusedMarkDefs.map((markDefKey) => - unset([{_key: blk._key}, 'markDefs', {_key: markDefKey}]), - ), - description: `Block contains orphaned data (unused mark definitions): ${unusedMarkDefs.join( - ', ', - )}.`, - action: 'Remove unused mark definition item', - item: blk, - i18n: { - description: - 'inputs.portable-text.invalid-value.orphaned-mark-defs.description', - action: - 'inputs.portable-text.invalid-value.orphaned-mark-defs.action', - values: { - key: blk._key, - unusedMarkDefs: unusedMarkDefs.map((m) => m.toString()), - }, - }, - } - return true - } - } - - // Test every child - if ( - textBlock.children.some((child, cIndex: number) => { - if (typeof child !== 'object' || child === null) { - resolution = { - patches: [unset([{_key: blk._key}, 'children', cIndex])], - description: `Child at index '${cIndex}' in block with key '${blk._key}' is not an object.`, - action: 'Remove the item', - item: blk, - - i18n: { - description: - 'inputs.portable-text.invalid-value.non-object-child.description', - action: - 'inputs.portable-text.invalid-value.non-object-child.action', - values: {key: blk._key, index: cIndex}, - }, + i18n: { + description: + 'inputs.portable-text.invalid-value.non-object-child.description', + action: + 'inputs.portable-text.invalid-value.non-object-child.action', + values: {key: blk._key, index: cIndex}, + }, + } + return true } - return true - } - if (!child._key || typeof child._key !== 'string') { - const newChild = {...child, _key: keyGenerator()} - resolution = { - autoResolve: true, - patches: [ - set(newChild, [{_key: blk._key}, 'children', cIndex]), - ], - description: `Child at index ${cIndex} is missing required _key in block with _key ${blk._key}.`, - action: 'Set a new random _key on the object', - item: blk, + // A missing child `_key` is mechanically fixable; fall back to + // the child's index so a later check on the same child doesn't + // build a `{_key: undefined}` path segment. + const childRef = nodeSegment(child, cIndex) - i18n: { - description: - 'inputs.portable-text.invalid-value.missing-child-key.description', - action: - 'inputs.portable-text.invalid-value.missing-child-key.action', - values: {key: blk._key, index: cIndex}, - }, - } - return true - } - - // Verify that children have valid types - if (!child._type) { - resolution = { - patches: [ - unset([{_key: blk._key}, 'children', {_key: child._key}]), - ], - description: `Child with _key '${child._key}' in block with key '${blk._key}' is missing '_type' property.`, - action: 'Remove the object', - item: blk, + // Verify that children have valid types + if (!child._type) { + resolution = { + patches: [ + unset([nodeSegment(blk, index), 'children', childRef]), + ], + description: `Child ${describeChildLocation(child, cIndex)} in block ${describeBlockLocation(blk, index)} is missing '_type' property.`, + action: 'Remove the object', + item: blk, - i18n: { - description: - 'inputs.portable-text.invalid-value.missing-child-type.description', - action: - 'inputs.portable-text.invalid-value.missing-child-type.action', - values: {key: blk._key, childKey: child._key}, - }, + i18n: { + description: + 'inputs.portable-text.invalid-value.missing-child-type.description', + action: + 'inputs.portable-text.invalid-value.missing-child-type.action', + values: { + key: blk._key, + childKey: hasUsableKey(child._key) ? child._key : cIndex, + }, + }, + } + return true } - return true - } - if (!validChildTypes.includes(child._type)) { - resolution = { - patches: [ - unset([{_key: blk._key}, 'children', {_key: child._key}]), - ], - description: `Child with _key '${child._key}' in block with key '${blk._key}' has invalid '_type' property (${child._type}).`, - action: 'Remove the object', - item: blk, + if (!validChildTypes.includes(child._type)) { + resolution = { + patches: [ + unset([nodeSegment(blk, index), 'children', childRef]), + ], + description: `Child ${describeChildLocation(child, cIndex)} in block ${describeBlockLocation(blk, index)} has invalid '_type' property (${child._type}).`, + action: 'Remove the object', + item: blk, - i18n: { - description: - 'inputs.portable-text.invalid-value.disallowed-child-type.description', - action: - 'inputs.portable-text.invalid-value.disallowed-child-type.action', - values: { - key: blk._key, - childKey: child._key, - childType: child._type, + i18n: { + description: + 'inputs.portable-text.invalid-value.disallowed-child-type.description', + action: + 'inputs.portable-text.invalid-value.disallowed-child-type.action', + values: { + key: blk._key, + childKey: hasUsableKey(child._key) ? child._key : cIndex, + childType: child._type, + }, }, - }, + } + return true } - return true - } - // Verify that spans have .text property that is a string - if ( - child._type === types.span.name && - typeof child.text !== 'string' - ) { - resolution = { - patches: [ - set({...child, text: ''}, [ - {_key: blk._key}, - 'children', - {_key: child._key}, - ]), - ], - description: `Child with _key '${child._key}' in block with key '${blk._key}' has missing or invalid text property!`, - action: `Write an empty text property to the object`, - item: blk, + // Verify that spans have .text property that is a string + if ( + child._type === types.span.name && + typeof child.text !== 'string' + ) { + resolution = { + patches: [ + set({...child, text: ''}, [ + nodeSegment(blk, index), + 'children', + childRef, + ]), + ], + description: `Child ${describeChildLocation(child, cIndex)} in block ${describeBlockLocation(blk, index)} has missing or invalid text property!`, + action: `Write an empty text property to the object`, + item: blk, - i18n: { - description: - 'inputs.portable-text.invalid-value.invalid-span-text.description', - action: - 'inputs.portable-text.invalid-value.invalid-span-text.action', - values: {key: blk._key, childKey: child._key}, - }, + i18n: { + description: + 'inputs.portable-text.invalid-value.invalid-span-text.description', + action: + 'inputs.portable-text.invalid-value.invalid-span-text.action', + values: { + key: blk._key, + childKey: hasUsableKey(child._key) ? child._key : cIndex, + }, + }, + } + return true } - return true - } - return false - }) - ) { - valid = false + return false + }) + ) { + valid = false + } } } return false @@ -393,3 +302,20 @@ export function validateValue( } return {valid, resolution, value} } + +/** + * A block without a usable `_key` can only be pointed at by position; + * `index` is the block's position in the full document (`baseIndex` + * applied), not in the slice being validated. + */ +function describeBlockLocation(blk: PortableTextBlock, index: number): string { + return hasUsableKey(blk._key) + ? `with _key '${blk._key}'` + : `at index '${index}'` +} + +function describeChildLocation(child: {_key?: unknown}, index: number): string { + return hasUsableKey(child._key) + ? `with _key '${child._key}'` + : `at index '${index}'` +} diff --git a/packages/editor/src/test/vitest/test-editor.tsx b/packages/editor/src/test/vitest/test-editor.tsx index e3f8f4c3f..a06b9f223 100644 --- a/packages/editor/src/test/vitest/test-editor.tsx +++ b/packages/editor/src/test/vitest/test-editor.tsx @@ -22,6 +22,7 @@ import type {Context} from './step-context' type CreateTestEditorOptions = { initialValue?: Array keyGenerator?: () => string + readOnly?: boolean schemaDefinition?: SchemaDefinition children?: React.ReactNode editableProps?: PortableTextEditableProps @@ -45,6 +46,7 @@ export async function createTestEditor( keyGenerator, schemaDefinition: options.schemaDefinition ?? defineSchema({}), initialValue: options.initialValue, + readOnly: options.readOnly, }} > @@ -83,7 +85,18 @@ export async function createTestEditor( )) } - const locator = renderResult.locator.getByRole('textbox') + // A read-only editable carries no ARIA `textbox` role (see + // `editable.tsx`), so `getByRole` never resolves; the always-present + // `data-pt-editor` marker locates it instead. + const locator = options.readOnly + ? await vi.waitFor(() => { + const element = renderResult.container.querySelector('[data-pt-editor]') + if (element === null) { + throw new Error('Expected to find an element with `data-pt-editor`') + } + return page.elementLocator(element) + }) + : renderResult.locator.getByRole('textbox') await vi.waitFor(() => expect.element(locator).toBeInTheDocument()) @@ -94,6 +107,43 @@ export async function createTestEditor( } } +/** + * Relays a `mutation` event's patches to `target` immediately, and its + * `value` once the whole synchronous flush batch that produced the event + * has finished. A single `flush()` call can emit several `mutation` + * events back to back (one per pending bulk), all stamped with the same + * current-at-flush-time `value` (see `MutationEvent.value`); sending that + * value on the first of them would jump `target` straight past the + * others' own patches before they get relayed, so those patches would + * find nothing left to apply against. + */ +function relayMutationsTo(target: React.RefObject) { + let pendingValue: Array | undefined + let valueSyncScheduled = false + + return (event: EditorEmittedEvent) => { + if (event.type !== 'mutation') { + return + } + target.current?.send({ + type: 'patches', + patches: event.patches.map((patch) => ({ + ...patch, + origin: 'remote', + })), + snapshot: event.value, + }) + pendingValue = event.value + if (!valueSyncScheduled) { + valueSyncScheduled = true + queueMicrotask(() => { + valueSyncScheduled = false + target.current?.send({type: 'update value', value: pendingValue}) + }) + } + } +} + /** * @internal */ @@ -112,6 +162,8 @@ export async function createTestEditors( const keyGeneratorB = options.keyGenerator ?? createTestKeyGenerator('eb-') const onEditorEvent = vi.fn<(event: EditorEmittedEvent) => void>() const onEditorBEvent = vi.fn<(event: EditorEmittedEvent) => void>() + const relayToB = relayMutationsTo(editorBRef) + const relayToA = relayMutationsTo(editorRef) render( <> @@ -130,20 +182,7 @@ export async function createTestEditors( { onEditorEvent(event) - if (event.type === 'mutation') { - editorBRef.current?.send({ - type: 'patches', - patches: event.patches.map((patch) => ({ - ...patch, - origin: 'remote', - })), - snapshot: event.value, - }) - editorBRef.current?.send({ - type: 'update value', - value: event.value, - }) - } + relayToB(event) }} /> {options.children} @@ -163,20 +202,7 @@ export async function createTestEditors( { onEditorBEvent(event) - if (event.type === 'mutation') { - editorRef.current?.send({ - type: 'patches', - patches: event.patches.map((patch) => ({ - ...patch, - origin: 'remote', - })), - snapshot: event.value, - }) - editorRef.current?.send({ - type: 'update value', - value: event.value, - }) - } + relayToA(event) }} /> {options.children} diff --git a/packages/editor/src/types/editor-engine.ts b/packages/editor/src/types/editor-engine.ts index 368cdf2b9..844b8dcfb 100644 --- a/packages/editor/src/types/editor-engine.ts +++ b/packages/editor/src/types/editor-engine.ts @@ -4,6 +4,7 @@ import type {EditorSnapshot} from '../editor/editor-snapshot' import type {DecoratedRange} from '../editor/range-decorations-machine' import type {ApplyContextFrame} from '../engine/core/apply-context' import type {DOMEditor} from '../engine/dom/plugin/dom-editor' +import type {Node} from '../engine/interfaces/node' import type {EngineOperation} from '../engine/interfaces/operation' import type { AnnotationConfig, @@ -76,10 +77,57 @@ export interface PortableTextEditorEngine extends DOMEditor { * with the empty path. */ verifiedUniqueChildGroups: Set + /** + * A FIFO-capped (100 entries) journal of intake repairs, keyed by the + * owning top-level block's post-repair `_key`. `subscribeRepairJournal` + * populates it: an entry records the block's shape immediately before + * and after normalization repaired it while replaying remote content + * (intake repair, not a local edit fixed up in passing). A chained + * repair on the same block updates the existing entry's `afterShape` + * instead of adding a second one, so the entry always spans the whole + * repair session's before/after, not just its last step. + * + * `syncBlock` (`sync-machine.ts`) consults it before diffing an inbound + * block against the engine's block: an inbound block matching an + * entry's `beforeShape`, while the engine still holds exactly its + * `afterShape`, is the host's snapshot echoing content this repair has + * already superseded, not a genuine edit, and the sync is a no-op that + * leaves the entry in place. An inbound block matching `afterShape` + * acknowledges the repair and retires the entry. Anything else (a + * genuine edit, or an operation elsewhere touching the journaled block) + * also retires the entry, so a stale verdict can never outlive the + * state it was recorded against. + */ + repairJournal: Map remotePatches: Array undoStepId: string | undefined isDeferringMutations: boolean + /** + * Called by the sync machine when a value sync pass starts, before the + * pass's own invoked sync can mint anything. The mutation batcher + * installs this to bump the generation it tags newly created bulks + * with, so `notifyInboundStateApplied`'s cull can tell this pass's own + * mints (tagged with the generation current at their creation, which is + * this pass's) apart from a repair minted between passes (remote-patch + * fallout minted while no pass is running), which still carries the + * older generation from before this pass started. + */ + notifyInboundSyncStarted: (() => void) | null + /** + * Called by the sync machine once a value sync pass has walked every + * block and settled, whether or not it changed anything, including any + * fresh repair patches normalization emitted in response. A pass that + * stops early on an invalid block never calls this: it never examined + * the remaining blocks, so it can prove nothing about them. The + * argument carries the keys of the blocks the pass found still echoing + * their pre-repair shape (a snapshot the host hasn't learned a repair + * from yet). The mutation batcher installs this to drop held repair + * bulks whose block isn't in that set and isn't otherwise protected. + * Only value syncs call it: a remote patch batch is a delta and cannot + * prove a held repair superseded. + */ + notifyInboundStateApplied: ((echoedBlockKeys: Set) => void) | null /** * The last host value recorded by a value sync that changed the engine. A * pristine block equal to it is persisted content, not the local diff --git a/packages/editor/src/types/editor.ts b/packages/editor/src/types/editor.ts index a3182a267..311917e14 100644 --- a/packages/editor/src/types/editor.ts +++ b/packages/editor/src/types/editor.ts @@ -114,7 +114,6 @@ export type EditorSelection = { * The editor has invalid data in the value that can be resolved by the user * @public */ export type InvalidValueResolution = { - autoResolve?: boolean patches: Patch[] description: string action: string diff --git a/packages/editor/tests/collaborative-editing.test.tsx b/packages/editor/tests/collaborative-editing.test.tsx index dae396ef0..e2e93adb0 100644 --- a/packages/editor/tests/collaborative-editing.test.tsx +++ b/packages/editor/tests/collaborative-editing.test.tsx @@ -41,12 +41,16 @@ describe('Collaborative editing', () => { /** * This test mimics the following scenario: * 1. Editor A loads with initial value missing `marks` on a span - * 2. Editor A normalizes and adds marks: [], but defers the patch until the editor is dirty + * 2. The initial sync runs in a remote frame, and the `marks`-default + * normalization is gated off in that frame, so the span stays + * without `marks` * 3. Editor B (simulated) also normalizes, then makes "foo" bold * 4. Editor B emits: set marks to [], then set marks to ['strong'] * 5. Editor A receives these patches and applies them - "foo" becomes bold * 6. Editor A user starts typing - * 7. Editor A does not emit the deferred patch (would overwrite the bold) + * 7. By now `marks` is already the remote-set array, so the + * `marks`-default normalization has nothing left to add and never + * fires (it would otherwise have overwritten the bold) */ const keyGenerator = createTestKeyGenerator() const blockKey = keyGenerator() diff --git a/packages/editor/tests/container-normalization.test.tsx b/packages/editor/tests/container-normalization.test.tsx index 68221615c..84b872c74 100644 --- a/packages/editor/tests/container-normalization.test.tsx +++ b/packages/editor/tests/container-normalization.test.tsx @@ -212,8 +212,9 @@ describe('container normalization', () => { ]) }) - // Normalization patches are deferred during setup. Trigger the dirty - // state so deferred patches get emitted. + // Setup-deferred patches already flushed at ready; this edit is a + // causal sentinel, giving `vi.waitFor` something to catch once its own + // patch (and everything ahead of it) has landed. editor.send({ type: 'select', at: { diff --git a/packages/editor/tests/editor-value-adoption.test.tsx b/packages/editor/tests/editor-value-adoption.test.tsx index 469fc2956..709556235 100644 --- a/packages/editor/tests/editor-value-adoption.test.tsx +++ b/packages/editor/tests/editor-value-adoption.test.tsx @@ -67,6 +67,7 @@ describe('initialization', () => { value: 'normal', origin: 'local', }, + intakeRepair: false, }) expect(editor.getSnapshot().context.value).toStrictEqual([ { @@ -354,7 +355,7 @@ describe('initialization', () => { resolution: { action: 'Write an empty text property to the object', description: - "Child with _key 'def' in block with key 'abc' has missing or invalid text property!", + "Child with _key 'def' in block with _key 'abc' has missing or invalid text property!", i18n: { action: 'inputs.portable-text.invalid-value.invalid-span-text.action', diff --git a/packages/editor/tests/event.mutation.test.tsx b/packages/editor/tests/event.mutation.test.tsx index 8436a05c9..b685df307 100644 --- a/packages/editor/tests/event.mutation.test.tsx +++ b/packages/editor/tests/event.mutation.test.tsx @@ -1,22 +1,232 @@ +import {insert, setIfMissing} from '@portabletext/patches' import {compileSchema, defineSchema} from '@portabletext/schema' import {createTestKeyGenerator, toTextspec} from '@portabletext/test' import {makeDiff, makePatches, stringifyPatches} from '@sanity/diff-match-patch' -import {useState} from 'react' +import {createRef, useState} from 'react' import {describe, expect, test, vi} from 'vitest' import {render} from 'vitest-browser-react' import {page, userEvent, type Locator} from 'vitest/browser' import { EditorProvider, PortableTextEditable, + type Editor, type EditorEmittedEvent, type MutationEvent, type Patch, } from '../src' +import {defineBehavior, forward, raise} from '../src/behaviors' +import {BehaviorPlugin} from '../src/plugins/plugin.behavior' +import {EditorRefPlugin} from '../src/plugins/plugin.editor-ref' import {EventListenerPlugin} from '../src/plugins/plugin.event-listener' import {createTestEditor} from '../src/test/vitest' describe('event.mutation', () => { - test('Scenario: Deferring mutation events when read-only', async () => { + test('Scenario: a mutation batched before a read-only flip survives a host that rejects mutations while read-only', async () => { + let readOnly = false + const mutations: Array = [] + + const {editor, locator} = await createTestEditor({ + children: ( + { + if (event.type !== 'mutation') { + return + } + + if (readOnly) { + // Mirrors Studio's and Canvas' `onChange`: both throw when + // asked to patch a read-only document. + throw new Error('Attempted to patch a read-only document') + } + + mutations.push(event) + }} + /> + ), + }) + + await userEvent.type(locator, 'foo') + + readOnly = true + editor.send({type: 'update readOnly', readOnly: true}) + + // The batcher's typing debounce (250ms) and flush interval (500ms in + // test mode) both fire well within this window: waiting past it, still + // read-only, proves the held mutation survives an attempted flush + // rather than merely a flip that outran the cadence. No local edit can + // reach the engine while it's read-only, so there's no flush to anchor + // this wait on instead. + await new Promise((resolve) => setTimeout(resolve, 600)) + + readOnly = false + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + typedFooPatches(), + ]) + }) + }) + + test('Scenario: an edit batched before a read-only flip still blocks snapshot clobbering until delivered', async () => { + const mutations: Array = [] + + const {editor, locator} = await createTestEditor({ + children: ( + { + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + await userEvent.type(locator, 'foo') + + editor.send({type: 'update readOnly', readOnly: true}) + + editor.send({ + type: 'update value', + value: [ + { + _key: 'k0', + _type: 'block', + children: [{_key: 'k1', _type: 'span', text: 'bar', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + // The batcher's typing debounce (250ms) and flush interval (500ms in + // test mode) both fire well within this window: waiting past it, the + // typed text is still the editor's local value, proving the busy guard + // parked the incoming remote value instead of letting it clobber the + // unflushed edit. No local edit can reach the engine while it's + // read-only, so there's no flush to anchor this wait on instead. + await new Promise((resolve) => setTimeout(resolve, 600)) + + expect(toTextspec(editor.getSnapshot().context)).toEqual('B: foo|') + expect(mutations).toEqual([]) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + typedFooPatches(), + ]) + }) + + // Only once the edit is delivered does the sync machine leave `busy` + // and reconcile the parked remote value. + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'k0', + _type: 'block', + children: [{_key: 'k1', _type: 'span', text: 'bar', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + // The sync machine parks in `busy` and re-checks on a 1s timer, so + // this can land a beat over a second after the edit is delivered. + {timeout: 5000}, + ) + }) + + test('Scenario: a host that rejects read-only mutations discards the unmount handover', async () => { + const keyGenerator = createTestKeyGenerator() + const editorRef = createRef() + let readOnly = false + const deliveries: Array> = [] + const accepted: Array = [] + + const renderResult = await render( + + + { + if (event.type !== 'mutation') { + return + } + + deliveries.push(event.patches) + + if (readOnly) { + // Mirrors Studio's and Canvas' `onChange`: both throw when + // asked to patch a read-only document. + throw new Error('Attempted to patch a read-only document') + } + + accepted.push(event) + }} + /> + + , + ) + + const locator = page.getByRole('textbox') + await vi.waitFor(() => expect.element(locator).toBeInTheDocument()) + + await userEvent.click(locator) + await userEvent.type(locator, 'foo') + + editorRef.current!.send({type: 'update readOnly', readOnly: true}) + readOnly = true + + renderResult.unmount() + + // Pins the handover's limit: the mutation is delivered even to a + // rejecting host, and a host that throws loses the edit for good + // (there is no later tick to retry on). + expect(deliveries).toEqual([typedFooPatches()]) + expect(accepted).toEqual([]) + }) + + test('Scenario: pending mutations are handed over on unmount even while read-only', async () => { + const keyGenerator = createTestKeyGenerator() + const editorRef = createRef() + const mutations: Array = [] + + const renderResult = await render( + + + { + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + + , + ) + + const locator = page.getByRole('textbox') + await vi.waitFor(() => expect.element(locator).toBeInTheDocument()) + + await userEvent.click(locator) + await userEvent.type(locator, 'foo') + + editorRef.current!.send({type: 'update readOnly', readOnly: true}) + + renderResult.unmount() + + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + typedFooPatches(), + ]) + }) + + test('Scenario: mutations flush once the editor becomes editable again, even for edits made while still read-only', async () => { const onEvent = vi.fn<(event: EditorEmittedEvent) => void>() let resolveFooMutation: () => void @@ -52,45 +262,207 @@ describe('event.mutation', () => { editor.send({type: 'update readOnly', readOnly: true}) - await new Promise((resolve) => setTimeout(resolve, 250)) + const foobarMutation = { + type: 'mutation', + patches: [ + { + type: 'diffMatchPatch', + path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('foo', 'foobar'))), + origin: 'local', + }, + ], + value: [ + { + _type: 'block', + _key: 'k0', + children: [{_type: 'span', _key: 'k1', text: 'foobar', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + } - expect(onEvent).not.toHaveBeenCalledWith( - expect.objectContaining({ - type: 'mutation', - value: [ - { - _type: 'block', - _key: 'k0', - children: [{_type: 'span', _key: 'k1', text: 'foobar', marks: []}], - markDefs: [], - style: 'normal', - }, - ], - }), - ) + // The batcher's typing debounce (250ms) and flush interval (500ms in + // test mode) both fire well within this window: waiting past it, still + // read-only, proves the "bar" mutation is held, not just late. No + // local edit can reach the engine while it's read-only, so there's no + // flush to anchor this wait on instead. + await new Promise((resolve) => setTimeout(resolve, 600)) + + expect(onEvent).not.toHaveBeenCalledWith(foobarMutation) editor.send({type: 'update readOnly', readOnly: false}) - await new Promise((resolve) => setTimeout(resolve, 250)) + await vi.waitFor(() => { + expect(onEvent).toHaveBeenCalledWith(foobarMutation) + }) + }) + + test('Scenario: a mutating behavior on `select` while read-only survives a value-sync pass and flushes once editable', async () => { + const keyGenerator = createTestKeyGenerator() + const keepBlockKey = keyGenerator() + const keepSpanKey = keyGenerator() + const goneBlockKey = keyGenerator() + const goneSpanKey = keyGenerator() + const mutations: Array = [] + + const {editor} = await createTestEditor({ + keyGenerator, + readOnly: true, + initialValue: [ + { + _key: keepBlockKey, + _type: 'block', + children: [ + {_key: keepSpanKey, _type: 'span', text: 'keep', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: goneBlockKey, + _type: 'block', + children: [ + {_key: goneSpanKey, _type: 'span', text: 'gone', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + children: ( + <> + { + const anchorSegment = event.at?.anchor.path[0] + const targetsKeepBlock = + typeof anchorSegment === 'object' && + anchorSegment !== null && + '_key' in anchorSegment && + anchorSegment._key === keepBlockKey + + // `select` is one of the handful of behavior events + // the edit-mode machine still admits while read-only + // (see the "read only" state's `behavior event` + // guard), so this action runs, and mutates, even + // though the editor never left read-only. + return targetsKeepBlock + ? [ + forward(event), + raise({ + type: 'delete.block', + at: [{_key: goneBlockKey}], + }), + ] + : [forward(event)] + }, + ], + }), + ]} + /> + { + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + + ), + }) await vi.waitFor(() => { - expect(onEvent).toHaveBeenCalledWith( - expect.objectContaining({ - type: 'mutation', - value: [ - { - _type: 'block', - _key: 'k0', - children: [ - {_type: 'span', _key: 'k1', text: 'foobar', marks: []}, - ], - markDefs: [], - style: 'normal', - }, + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: keepBlockKey, + _type: 'block', + children: [ + {_key: keepSpanKey, _type: 'span', text: 'keep', marks: []}, ], - }), - ) + markDefs: [], + style: 'normal', + }, + { + _key: goneBlockKey, + _type: 'block', + children: [ + {_key: goneSpanKey, _type: 'span', text: 'gone', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + editor.send({ + type: 'select', + at: { + anchor: { + path: [{_key: keepBlockKey}, 'children', {_key: keepSpanKey}], + offset: 0, + }, + focus: { + path: [{_key: keepBlockKey}, 'children', {_key: keepSpanKey}], + offset: 0, + }, + }, + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: keepBlockKey, + _type: 'block', + children: [ + {_key: keepSpanKey, _type: 'span', text: 'keep', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) }) + + // A genuinely new remote value, still read-only: settling this value + // sync pass is what previously culled the held delete as though it + // were a superseded repair. The held delete's own patch now marks + // `isDeferringMutations`, so the sync machine parks this value in + // `busy` rather than syncing it immediately; it only applies once the + // held mutation flushes below. + editor.send({ + type: 'update value', + value: [ + { + _key: keepBlockKey, + _type: 'block', + children: [ + {_key: keepSpanKey, _type: 'span', text: 'kept', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(mutations.flatMap((mutation) => mutation.patches)).toEqual([ + {type: 'unset', path: [{_key: goneBlockKey}], origin: 'local'}, + ]) + }) + + await vi.waitFor( + () => { + expect(toTextspec(editor.getSnapshot().context)).toEqual('B: kept|') + }, + // The sync machine parks in `busy` while the held mutation above is + // still in flight and re-checks on a 1s timer. + {timeout: 5000}, + ) }) test('Scenario: Batching typing mutations', async () => { @@ -263,6 +635,46 @@ describe('event.mutation', () => { }) }) +function typedFooPatches(): Array { + return [ + {...setIfMissing([], []), origin: 'local'}, + { + ...insert( + [ + { + _key: 'k0', + _type: 'block', + children: [{_key: 'k1', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + 'before', + [0], + ), + origin: 'local', + }, + { + origin: 'local', + type: 'diffMatchPatch', + path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('', 'f'))), + }, + { + origin: 'local', + type: 'diffMatchPatch', + path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('f', 'fo'))), + }, + { + origin: 'local', + type: 'diffMatchPatch', + path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('fo', 'foo'))), + }, + ] +} + function insertTextSync(locator: Locator, text: string) { const element = locator.element() for (const character of text) { diff --git a/packages/editor/tests/event.operation.test.tsx b/packages/editor/tests/event.operation.test.tsx index 0d28a7026..e93062121 100644 --- a/packages/editor/tests/event.operation.test.tsx +++ b/packages/editor/tests/event.operation.test.tsx @@ -74,7 +74,7 @@ describe('event.operation', () => { }) }) - test('Scenario: Operations from value sync are observed while patches are gated', async () => { + test('Scenario: Operations from value sync are observed alongside patches', async () => { const {editor} = await createTestEditor() const operations = collectOperations(editor) const patches: Array = [] @@ -101,8 +101,8 @@ describe('event.operation', () => { ]) }) - // `patch`/`mutation` are gated while the editor is pristine; the - // operation stream is not. + // The synced block is already valid, so there is nothing to repair + // and nothing for `patch` to carry. expect(patches).toEqual([]) }) @@ -110,9 +110,9 @@ describe('event.operation', () => { const {editor} = await createTestEditor() const operations = collectOperations(editor) - // A text block with no children is auto-resolved by `validateValue` at - // sync ingress: the placeholder span is part of the inserted node - // itself, not a separate normalization fix operation. + // Validation passes a text block with no children through untouched; + // engine normalization repairs it afterward, inserting the placeholder + // span as its own operation, adjacent to the block's own insert. editor.send({type: 'update value', value: [emptyBlock('b1')]}) await vi.waitFor(() => { @@ -125,10 +125,13 @@ describe('event.operation', () => { type: 'insert', path: [0], position: 'before', - node: { - ...emptyBlock('b1'), - children: [{_type: 'span', _key: 'k2', text: '', marks: []}], - }, + node: emptyBlock('b1'), + }, + { + type: 'insert', + path: [{_key: 'b1'}, 'children', 0], + position: 'before', + node: {_type: 'span', _key: 'k2', text: '', marks: []}, }, ]) }) diff --git a/packages/editor/tests/event.patches.sidecar-arrays.test.tsx b/packages/editor/tests/event.patches.sidecar-arrays.test.tsx index 63e0f6834..52840d33c 100644 --- a/packages/editor/tests/event.patches.sidecar-arrays.test.tsx +++ b/packages/editor/tests/event.patches.sidecar-arrays.test.tsx @@ -824,10 +824,9 @@ describe('event.patches sidecar arrays: multi-element and keyed tails', () => { children: , }) - // The mark insert is included so the new markDef is referenced. An - // unreferenced markDef is orphaned data the editor strips (both the - // unused-markDefs normalization rule and `validateValue`'s - // auto-resolution remove it). + // The mark insert is included so the new markDef is referenced: an + // unreferenced markDef is orphaned data the engine's normalization + // strips as unused. editor.send({ type: 'patches', patches: [ diff --git a/packages/editor/tests/event.patches.test.tsx b/packages/editor/tests/event.patches.test.tsx index 6f251f267..76586ac2b 100644 --- a/packages/editor/tests/event.patches.test.tsx +++ b/packages/editor/tests/event.patches.test.tsx @@ -11,7 +11,7 @@ import {createTestKeyGenerator, toTextspec} from '@portabletext/test' import {makeDiff, makePatches, stringifyPatches} from '@sanity/diff-match-patch' import {describe, expect, test, vi} from 'vitest' import {userEvent} from 'vitest/browser' -import {defineSchema, type EditorEmittedEvent} from '../src' +import {defineSchema, type EditorEmittedEvent, type MutationEvent} from '../src' import {raise} from '../src/behaviors/behavior.types.action' import {defineBehavior} from '../src/behaviors/behavior.types.behavior' import {BehaviorPlugin} from '../src/plugins/plugin.behavior' @@ -181,6 +181,7 @@ describe('event.patches', () => { path: [], value: [], }, + intakeRepair: false, }) expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', @@ -199,6 +200,7 @@ describe('event.patches', () => { }, ], }, + intakeRepair: false, }) expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', @@ -208,6 +210,7 @@ describe('event.patches', () => { path: [{_key: 'ea-k0'}, 'children', {_key: 'ea-k1'}, 'text'], value: '@@ -0,0 +1 @@\n+f\n', }, + intakeRepair: false, }) }) @@ -248,6 +251,7 @@ describe('event.patches', () => { path: [], value: [], }, + intakeRepair: false, }) expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', @@ -266,6 +270,7 @@ describe('event.patches', () => { }, ], }, + intakeRepair: false, }) expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', @@ -284,6 +289,7 @@ describe('event.patches', () => { }, ], }, + intakeRepair: false, }) }) @@ -338,6 +344,7 @@ describe('event.patches', () => { path: [], value: [], }, + intakeRepair: false, }) expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', @@ -356,6 +363,7 @@ describe('event.patches', () => { }, ], }, + intakeRepair: false, }) expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', @@ -374,6 +382,7 @@ describe('event.patches', () => { }, ], }, + intakeRepair: false, }) }) @@ -1856,6 +1865,73 @@ describe('event.patches', () => { }) }) + test('Scenario: a pristine editor emits a remote-fallout repair immediately, before any local edit', async () => { + const patches: Array = [] + const mutations: Array = [] + const keyGenerator = createTestKeyGenerator() + const blockKey = keyGenerator() + const spanKey = keyGenerator() + const {editor} = await createTestEditor({ + keyGenerator, + initialValue: [ + { + _key: blockKey, + _type: 'block', + children: [{_key: spanKey, _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + editor.send({ + type: 'patches', + patches: [ + { + type: 'unset', + origin: 'remote', + path: [{_key: blockKey}, 'children', {_key: spanKey}, '_key'], + }, + ], + snapshot: undefined, + }) + + const repairPatch = { + type: 'set', + path: [{_key: blockKey}, 'children', 0, '_key'], + value: 'k4', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: blockKey, + _type: 'block', + children: [{_key: 'k4', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + }) + test('Scenario: `set`ing `style` on text block', async () => { const keyGenerator = createTestKeyGenerator() const blockKey = keyGenerator() @@ -5535,14 +5611,20 @@ describe('event.patches', () => { ), }) - // Unlike the single-flush clear above, a character-by-character clear - // spreads across flushes, so the mirrored echoes race the ongoing - // edits: a stale echo differs from the engine by then, syncs as a - // genuine write, and records the placeholder as the last synced - // value. The editor's own `unset([])` emission must override that: - // its stream destroyed the field, so the retype must rebuild it. + // A character-by-character clear spreads across flushes, and each + // mutation's `value` reflects the engine at that flush, not at some + // earlier moment, so a mirror sent as its own mutation lands can + // never itself be stale: a real host's round trip is what actually + // makes an echo land describing content the editor has since moved + // past. Capturing the 'foo' flush's value and delivering it only + // after the clear's own flush has landed reproduces that: the echo + // is guaranteed stale by the time it arrives, syncs as a genuine + // write, and records the placeholder as the last synced value. The + // editor's own `unset([])` emission must override that: its stream + // destroyed the field, so the retype must rebuild it. + const mutations: Array = [] editor.on('mutation', (event) => { - editor.send({type: 'update value', value: event.value}) + mutations.push(event) }) let remoteOperations = 0 editor.on('operation', (event) => { @@ -5563,6 +5645,21 @@ describe('event.patches', () => { }) }) + // Waits for the 'foo' mutation's own flush (not just its patch relay) + // so the clear below flushes separately, and captures the value that + // flush carries: the pre-clear content a host's echo would mirror + // back. + await vi.waitFor(() => { + expect(mutations.at(-1)?.patches.at(-1)).toEqual({ + type: 'diffMatchPatch', + path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('fo', 'foo'))), + origin: 'local', + }) + }) + const preClearValue = mutations.at(-1)?.value + const mutationCountBeforeClear = mutations.length + await userEvent.keyboard('{Backspace}{Backspace}{Backspace}') await vi.waitFor(() => { @@ -5573,10 +5670,25 @@ describe('event.patches', () => { }) }) - // The echo cascade settles when a stale echo has written into the - // engine (a remote-origin operation) and the last write restored the - // placeholder. Only then has the poisoned value been recorded, which - // is the state the retype below must survive. + // Waits for the clear's own flush to land before delivering either + // echo: only then is the captured 'foo' value guaranteed stale + // (superseded by the clear), and only then does the clear's own + // settled value exist to mirror back. + await vi.waitFor(() => { + expect(mutations.length).toBeGreaterThan(mutationCountBeforeClear) + }) + const postClearValue = mutations.at(-1)?.value + + // Delivers the stale 'foo' echo first (it diverges from the current, + // cleared engine state, so it syncs as a genuine remote write), then + // the clear's own echo (which overwrites that divergence back down to + // the placeholder), reproducing a host's round trip landing after a + // more recent one settled: the placeholder ends up recorded as the + // last synced value, which is the poisoned state the retype below + // must survive. + editor.send({type: 'update value', value: preClearValue}) + editor.send({type: 'update value', value: postClearValue}) + await vi.waitFor(() => { expect(remoteOperations).toBeGreaterThan(0) expect(editor.getSnapshot().context.value).toEqual([ @@ -5703,13 +5815,15 @@ describe('event.patches', () => { }) // Build the same poisoned recording as 'Retyping after a - // character-by-character clear when the host mirrors values': a - // stale mirrored echo lands after the editor's own `unset([])` and - // records the placeholder as the last synced value. A remote root - // `insert` must override that recording the same way the local - // retype does. + // character-by-character clear when the host mirrors values' (see its + // capture-and-deliver comments for why both the pre-clear capture and + // the flush-separating wait matter here too): a stale mirrored echo + // lands after the editor's own `unset([])` and records the + // placeholder as the last synced value. A remote root `insert` must + // override that recording the same way the local retype does. + const mutations: Array = [] editor.on('mutation', (event) => { - editor.send({type: 'update value', value: event.value}) + mutations.push(event) }) let remoteOperations = 0 editor.on('operation', (event) => { @@ -5730,6 +5844,21 @@ describe('event.patches', () => { }) }) + // Waits for the 'foo' mutation's own flush (not just its patch relay) + // so the clear below flushes separately, and captures the value that + // flush carries: the pre-clear content a host's echo would mirror + // back. + await vi.waitFor(() => { + expect(mutations.at(-1)?.patches.at(-1)).toEqual({ + type: 'diffMatchPatch', + path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('fo', 'foo'))), + origin: 'local', + }) + }) + const preClearValue = mutations.at(-1)?.value + const mutationCountBeforeClear = mutations.length + await userEvent.keyboard('{Backspace}{Backspace}{Backspace}') await vi.waitFor(() => { @@ -5740,6 +5869,25 @@ describe('event.patches', () => { }) }) + // Waits for the clear's own flush to land before delivering either + // echo: only then is the captured 'foo' value guaranteed stale + // (superseded by the clear), and only then does the clear's own + // settled value exist to mirror back. + await vi.waitFor(() => { + expect(mutations.length).toBeGreaterThan(mutationCountBeforeClear) + }) + const postClearValue = mutations.at(-1)?.value + + // Delivers the stale 'foo' echo first (it diverges from the current, + // cleared engine state, so it syncs as a genuine remote write), then + // the clear's own echo (which overwrites that divergence back down to + // the placeholder), reproducing a host's round trip landing after a + // more recent one settled: the placeholder ends up recorded as the + // last synced value, which is the poisoned state the remote insert + // below must survive. + editor.send({type: 'update value', value: preClearValue}) + editor.send({type: 'update value', value: postClearValue}) + await vi.waitFor(() => { expect(remoteOperations).toBeGreaterThan(0) expect(editor.getSnapshot().context.value).toEqual([ diff --git a/packages/editor/tests/event.update-value.container.test.tsx b/packages/editor/tests/event.update-value.container.test.tsx index 591fd1b58..33d4ecb52 100644 --- a/packages/editor/tests/event.update-value.container.test.tsx +++ b/packages/editor/tests/event.update-value.container.test.tsx @@ -1,7 +1,9 @@ import {defineSchema} from '@portabletext/schema' import {createTestKeyGenerator, toTextspec} from '@portabletext/test' import {describe, expect, test, vi} from 'vitest' +import type {MutationEvent, Patch} from '../src' import {safeParse, safeStringify} from '../src/internal-utils/safe-json' +import {EventListenerPlugin} from '../src/plugins/plugin.event-listener' import {NodePlugin} from '../src/plugins/plugin.node' import {defineContainer} from '../src/renderers/renderer.types' import {createTestEditor} from '../src/test/vitest' @@ -56,6 +58,63 @@ const codeBlockContainer = [ }), ] +const tableSchemaDefinition = defineSchema({ + blockObjects: [ + { + name: 'table', + fields: [ + { + name: 'rows', + type: 'array', + of: [ + { + type: 'object', + name: 'row', + fields: [ + { + name: 'cells', + type: 'array', + of: [ + { + type: 'object', + name: 'cell', + fields: [ + { + name: 'content', + type: 'array', + of: [{type: 'block'}], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ], +}) + +const tableContainers = [ + defineContainer({ + type: 'table', + arrayField: 'rows', + render: ({children}) => <>{children}, + }), + defineContainer({ + type: 'row', + arrayField: 'cells', + render: ({children}) => <>{children}, + }), + defineContainer({ + type: 'cell', + arrayField: 'content', + render: ({children}) => <>{children}, + }), +] + describe('event.update value with containers', () => { test('Scenario: Update text inside a container line', async () => { const keyGenerator = createTestKeyGenerator() @@ -1115,3 +1174,656 @@ describe('event.update value with containers', () => { }) }) }) + +describe('event.update value with containers: intake repairs', () => { + test('Scenario: a loaded container with a missing child array materializes its default child in one repair mutation', async () => { + const keyGenerator = createTestKeyGenerator() + const calloutKey = keyGenerator() + const patches: Array = [] + const mutations: Array = [] + + const {editor} = await createTestEditor({ + keyGenerator, + schemaDefinition, + children: ( + <> + + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + + ), + initialValue: [ + { + _type: 'callout', + _key: calloutKey, + } as never, + ], + }) + + const repairPatch = { + type: 'set', + path: [{_key: calloutKey}, 'content'], + value: [ + { + _type: 'block', + _key: 'k3', + style: 'normal', + markDefs: [], + children: [{_type: 'span', _key: 'k4', text: '', marks: []}], + }, + ], + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _type: 'callout', + _key: calloutKey, + content: repairPatch.value, + }, + ]) + }) + + // The host echoes back the pre-repair snapshot: `content` is still + // missing. An unrelated new sibling block makes this a genuine new + // value, so the machine reconciles it instead of no-opping, giving a + // deterministic point to prove the echo alone minted nothing further. + editor.send({ + type: 'update value', + value: [ + { + _type: 'callout', + _key: calloutKey, + } as never, + { + _key: 'sibling', + _type: 'block', + children: [ + {_key: 'siblingSpan', _type: 'span', text: 'sibling', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _type: 'callout', + _key: calloutKey, + content: repairPatch.value, + }, + { + _key: 'sibling', + _type: 'block', + children: [ + {_key: 'siblingSpan', _type: 'span', text: 'sibling', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }, + {timeout: 5000}, + ) + + // No second repair: the echo was recognized, not re-repaired. + expect(patches).toEqual([repairPatch]) + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a loaded container with a deep missing child key is repaired once, echo swallowed at block granularity', async () => { + const keyGenerator = createTestKeyGenerator() + const tableKey = keyGenerator() + const rowKey = keyGenerator() + const cellKey = keyGenerator() + const blockKey = keyGenerator() + const patches: Array = [] + const mutations: Array = [] + + const {editor} = await createTestEditor({ + keyGenerator, + schemaDefinition: tableSchemaDefinition, + children: ( + <> + + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + + ), + initialValue: [ + deepTable({tableKey, rowKey, cellKey, blockKey, text: 'deep'}), + ], + }) + + const repairPatch = deepSpanKeyRepairPatch({ + tableKey, + rowKey, + cellKey, + blockKey, + mintedKey: 'k6', + }) + + const repairedTable = deepTable({ + tableKey, + rowKey, + cellKey, + blockKey, + text: 'deep', + spanKey: 'k6', + }) + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([repairedTable]) + }) + + // The host echoes back the pre-repair snapshot: the deep span is + // still keyless. An unrelated new sibling block makes this a genuine + // new value, so the machine reconciles it instead of no-opping. + editor.send({ + type: 'update value', + value: [ + deepTable({tableKey, rowKey, cellKey, blockKey, text: 'deep'}), + { + _key: 'sibling', + _type: 'block', + children: [ + {_key: 'siblingSpan', _type: 'span', text: 'sibling', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + repairedTable, + { + _key: 'sibling', + _type: 'block', + children: [ + {_key: 'siblingSpan', _type: 'span', text: 'sibling', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }, + {timeout: 5000}, + ) + + // No second repair: the echo was recognized at the top-level + // container's own granularity, not re-repaired. + expect(patches).toEqual([repairPatch]) + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test("Scenario: two sibling containers with deep defects, one host-repaired and one still echoing, flush only the still-echoing container's repair", async () => { + const keyGenerator = createTestKeyGenerator() + const tableAKey = keyGenerator() + const rowAKey = keyGenerator() + const cellAKey = keyGenerator() + const blockAKey = keyGenerator() + const tableBKey = keyGenerator() + const rowBKey = keyGenerator() + const cellBKey = keyGenerator() + const blockBKey = keyGenerator() + const patches: Array = [] + const mutations: Array = [] + + const {editor} = await createTestEditor({ + keyGenerator, + schemaDefinition: tableSchemaDefinition, + readOnly: true, + children: ( + <> + + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + + ), + initialValue: [ + deepTable({ + tableKey: tableAKey, + rowKey: rowAKey, + cellKey: cellAKey, + blockKey: blockAKey, + text: 'alpha', + }), + deepTable({ + tableKey: tableBKey, + rowKey: rowBKey, + cellKey: cellBKey, + blockKey: blockBKey, + text: 'beta', + }), + ], + }) + + const repairPatchA = deepSpanKeyRepairPatch({ + tableKey: tableAKey, + rowKey: rowAKey, + cellKey: cellAKey, + blockKey: blockAKey, + mintedKey: 'k10', + }) + const repairPatchB = deepSpanKeyRepairPatch({ + tableKey: tableBKey, + rowKey: rowBKey, + cellKey: cellBKey, + blockKey: blockBKey, + mintedKey: 'k11', + }) + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatchA, repairPatchB]) + expect(editor.getSnapshot().context.value).toEqual([ + deepTable({ + tableKey: tableAKey, + rowKey: rowAKey, + cellKey: cellAKey, + blockKey: blockAKey, + text: 'alpha', + spanKey: 'k10', + }), + deepTable({ + tableKey: tableBKey, + rowKey: rowBKey, + cellKey: cellBKey, + blockKey: blockBKey, + text: 'beta', + spanKey: 'k11', + }), + ]) + }) + + // The host (e.g. Sanity Studio) persisted its own key for table A's + // deep span, but never picked up table B's repair: table B still + // echoes its pre-repair keyless shape. A control block makes this a + // new value, so the machine reconciles it instead of no-opping. + editor.send({ + type: 'update value', + value: [ + deepTable({ + tableKey: tableAKey, + rowKey: rowAKey, + cellKey: cellAKey, + blockKey: blockAKey, + text: 'alpha', + spanKey: 'hostKey', + }), + deepTable({ + tableKey: tableBKey, + rowKey: rowBKey, + cellKey: cellBKey, + blockKey: blockBKey, + text: 'beta', + }), + { + _key: 'control', + _type: 'block', + children: [ + {_key: 'controlSpan', _type: 'span', text: 'control', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + const settledValue = [ + deepTable({ + tableKey: tableAKey, + rowKey: rowAKey, + cellKey: cellAKey, + blockKey: blockAKey, + text: 'alpha', + spanKey: 'hostKey', + }), + deepTable({ + tableKey: tableBKey, + rowKey: rowBKey, + cellKey: cellBKey, + blockKey: blockBKey, + text: 'beta', + spanKey: 'k11', + }), + { + _key: 'control', + _type: 'block', + children: [ + {_key: 'controlSpan', _type: 'span', text: 'control', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ] + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual(settledValue) + }) + + // No re-repair of either table: table A's repair was superseded by + // the host's own key, not re-applied, and table B's echo was + // recognized, not re-repaired. + expect(patches).toEqual([repairPatchA, repairPatchB]) + + editor.send({type: 'update readOnly', readOnly: false}) + + // Only table B's repair flushes: table A's repair bulk was dropped as + // superseded once the host's own key for table A landed. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatchB], + ]) + }) + + expect(editor.getSnapshot().context.value).toEqual(settledValue) + }) + + test('Scenario: a stale echo of a keyless container with a deep typeless block delivers every repair patch, none dropped', async () => { + const keyGenerator = createTestKeyGenerator() + const rowKey = keyGenerator() + const cellKey = keyGenerator() + const patches: Array = [] + const mutations: Array = [] + + // The table itself arrives without a `_key`, and the block nested + // three levels down (table > row > cell > block) arrives without a + // `_type` on top of that. `normalizeNode`'s missing-`_type` arm (the + // second per-node arm) fires before its missing-`_key` arm (the + // fourth), so the deep block gets its `_type` set, then its own + // `_key` minted, while the table is still keyless: both of those + // repair patches address the deep block through the table's own + // still-numeric root index, so the mutation batcher can't resolve + // either one to the table's block key yet. Only the table's own + // `_key` mint, third and last, resolves once applied. That gives the + // unresolved-then-resolved repair pair `dropSupersededRepairs` must + // never let the cull silently swallow. + const rawTable = { + _type: 'table', + rows: [ + { + _type: 'row', + _key: rowKey, + cells: [ + { + _type: 'cell', + _key: cellKey, + content: [ + { + children: [{_type: 'span', text: 'deep', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }, + ], + }, + ], + } as never + + const {editor} = await createTestEditor({ + keyGenerator, + schemaDefinition: tableSchemaDefinition, + readOnly: true, + children: ( + <> + + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + + ), + initialValue: [rawTable], + }) + + const deepTypeRepairPatch = { + type: 'set', + path: [ + 0, + 'rows', + {_key: rowKey}, + 'cells', + {_key: cellKey}, + 'content', + 0, + '_type', + ], + value: 'block', + origin: 'local', + } + const deepKeyRepairPatch = { + type: 'set', + path: [ + 0, + 'rows', + {_key: rowKey}, + 'cells', + {_key: cellKey}, + 'content', + 0, + '_key', + ], + value: 'k4', + origin: 'local', + } + const tableKeyRepairPatch = { + type: 'set', + path: [0, '_key'], + value: 'k5', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([ + deepTypeRepairPatch, + deepKeyRepairPatch, + tableKeyRepairPatch, + ]) + }) + + const repairedTable = { + _type: 'table', + _key: 'k5', + rows: [ + { + _type: 'row', + _key: rowKey, + cells: [ + { + _type: 'cell', + _key: cellKey, + content: [ + { + _type: 'block', + _key: 'k4', + children: [{_type: 'span', text: 'deep', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }, + ], + }, + ], + } + + expect(editor.getSnapshot().context.value).toEqual([repairedTable]) + + // The host echoes back the pre-repair snapshot verbatim: the table is + // still keyless and the deep block still lacks a `_type`. A control + // block makes this a genuine new value, not a no-op. + editor.send({ + type: 'update value', + value: [ + rawTable, + { + _key: 'control', + _type: 'block', + children: [ + {_key: 'controlSpan', _type: 'span', text: 'control', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + repairedTable, + { + _key: 'control', + _type: 'block', + children: [ + {_key: 'controlSpan', _type: 'span', text: 'control', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The echo was recognized: no fourth repair patch. + expect(patches).toEqual([ + deepTypeRepairPatch, + deepKeyRepairPatch, + tableKeyRepairPatch, + ]) + + editor.send({type: 'update readOnly', readOnly: false}) + + // Every repair patch flushes once editable, none dropped by the + // settle the stale echo triggered. + await vi.waitFor(() => { + expect(mutations.flatMap((mutation) => mutation.patches)).toEqual([ + deepTypeRepairPatch, + deepKeyRepairPatch, + tableKeyRepairPatch, + ]) + }) + }) +}) + +function deepTable(args: { + tableKey: string + rowKey: string + cellKey: string + blockKey: string + text: string + spanKey?: string +}) { + return { + _type: 'table', + _key: args.tableKey, + rows: [ + { + _type: 'row', + _key: args.rowKey, + cells: [ + { + _type: 'cell', + _key: args.cellKey, + content: [ + { + _type: 'block', + _key: args.blockKey, + children: [ + args.spanKey === undefined + ? ({_type: 'span', text: args.text, marks: []} as never) + : { + _type: 'span', + _key: args.spanKey, + text: args.text, + marks: [], + }, + ], + markDefs: [], + style: 'normal', + }, + ], + }, + ], + }, + ], + } +} + +function deepSpanKeyRepairPatch(args: { + tableKey: string + rowKey: string + cellKey: string + blockKey: string + mintedKey: string +}) { + return { + type: 'set', + path: [ + {_key: args.tableKey}, + 'rows', + {_key: args.rowKey}, + 'cells', + {_key: args.cellKey}, + 'content', + {_key: args.blockKey}, + 'children', + 0, + '_key', + ], + value: args.mintedKey, + origin: 'local', + } +} diff --git a/packages/editor/tests/event.update-value.test.tsx b/packages/editor/tests/event.update-value.test.tsx index ef376cb41..223490522 100644 --- a/packages/editor/tests/event.update-value.test.tsx +++ b/packages/editor/tests/event.update-value.test.tsx @@ -1,3 +1,5 @@ +import {applyAll} from '@portabletext/patches' +import type {PortableTextBlock} from '@portabletext/schema' import {createTestKeyGenerator, toTextspec} from '@portabletext/test' import {makeDiff, makePatches, stringifyPatches} from '@sanity/diff-match-patch' import {describe, expect, test, vi} from 'vitest' @@ -670,6 +672,7 @@ describe('event.update value', () => { path: [{_key: 'k2'}, 'children', {_key: 'k3'}, 'text'], value: stringifyPatches(makePatches(makeDiff('foo', 'foo!'))), }, + intakeRepair: false, }, { type: 'patch', @@ -679,6 +682,7 @@ describe('event.update value', () => { path: [{_key: 'k2'}, 'markDefs'], value: [], }, + intakeRepair: false, }, { type: 'patch', @@ -688,6 +692,7 @@ describe('event.update value', () => { path: [{_key: 'k2'}, 'style'], value: 'normal', }, + intakeRepair: false, }, { type: 'selection', @@ -2053,13 +2058,12 @@ describe('event.update value: adjacent same-mark spans', () => { }) }) -describe('event.update value: auto-resolved invalid blocks', () => { - // Regression: `validateValue` auto-resolutions (e.g. minting a missing - // child `_key`) were emitted as outbound patches while the *raw* block - // proceeded into the engine. The engine ended up holding the un-repaired - // shape (a keyless child), diverging from the document that received the - // minted key, and the next sync against that invalid engine state killed - // the sync silently. +describe('event.update value: mechanically repaired invalid blocks', () => { + // Engine normalization mints a missing child `_key` on intake and + // applies it to the engine's own document, not just to the outbound + // patch: the emitted repair and the block the engine holds carry the + // same minted key, so a later sync against this block finds a valid + // shape instead of diverging from it. const keylessChildBlock = { _key: 'b0', _type: 'block', @@ -2096,19 +2100,15 @@ describe('event.update value: auto-resolved invalid blocks', () => { // A changed block arrives whose span lost its `_key`. editor.send({type: 'update value', value: [keylessChildBlock]}) - // The auto-resolution is emitted as a patch AND applied to the block - // the engine receives: one key, minted once, on both sides. + // Engine normalization mints the key once, on both the outbound + // patch and the block it applies internally. await vi.waitFor(() => { expect(patches).toEqual([ { type: 'set', - path: [{_key: 'b0'}, 'children', 0], - value: { - _type: 'span', - _key: 'k2', - text: 'hello changed', - marks: [], - }, + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', }, ]) expect(editor.getSnapshot().context.value).toEqual([ @@ -2159,7 +2159,7 @@ describe('event.update value: auto-resolved invalid blocks', () => { ) }) - test('Scenario: a mid-session update with an unused markDef is repaired on both sides', async () => { + test('Scenario: a mid-session update with an orphaned markDef alongside a referenced one is repaired on both sides', async () => { const patches: Array = [] const {editor} = await createTestEditor({ keyGenerator: createTestKeyGenerator(), @@ -2186,7 +2186,8 @@ describe('event.update value: auto-resolved invalid blocks', () => { ), }) - // A changed block arrives carrying a markDef no span references. + // A changed block arrives carrying one markDef its span still + // references and one markDef no span references. editor.send({ type: 'update value', value: [ @@ -2194,66 +2195,101 @@ describe('event.update value: auto-resolved invalid blocks', () => { _key: 'b0', _type: 'block', children: [ - {_key: 's0', _type: 'span', text: 'hello changed', marks: []}, + { + _key: 's0', + _type: 'span', + text: 'hello changed', + marks: ['m0'], + }, + ], + markDefs: [ + {_key: 'm0', _type: 'link', href: 'https://example.com/kept'}, + {_key: 'm1', _type: 'link', href: 'https://example.com/orphan'}, ], - markDefs: [{_key: 'm1', _type: 'link', href: 'https://example.com'}], style: 'normal', }, ], }) - const unsetPatch = { - type: 'unset', - path: [{_key: 'b0'}, 'markDefs', {_key: 'm1'}], - } - - // The auto-resolution is emitted as a patch AND applied to the block - // the engine receives: the def is gone on both sides. + // Intake passes the raw block through untouched: the orphan survives + // until something else marks the block dirty. await vi.waitFor(() => { - expect(patches).toEqual([unsetPatch]) + expect(patches).toEqual([]) expect(editor.getSnapshot().context.value).toEqual([ { _key: 'b0', _type: 'block', children: [ - {_key: 's0', _type: 'span', text: 'hello changed', marks: []}, + { + _key: 's0', + _type: 'span', + text: 'hello changed', + marks: ['m0'], + }, + ], + markDefs: [ + {_key: 'm0', _type: 'link', href: 'https://example.com/kept'}, + {_key: 'm1', _type: 'link', href: 'https://example.com/orphan'}, ], - markDefs: [], style: 'normal', }, ]) }) - // Causal sentinel: one local edit, then assert the full emission - // history. Before the fix the engine received the block with the def - // still present, its own normalizer pruned it and parked a whole-array - // `set` on the pristine editor, and that parked patch flushed here, - // ahead of the sentinel's. + // The first local edit marks the block dirty: normalization removes + // the orphan in the same pass, keeping the referenced def, and emits + // the filtered `markDefs` array's wholesale `set` alongside the edit's + // own patch. editor.send({ type: 'select', at: { - anchor: {path: [{_key: 'b0'}, 'children', {_key: 's0'}], offset: 13}, - focus: {path: [{_key: 'b0'}, 'children', {_key: 's0'}], offset: 13}, + anchor: {path: [{_key: 'b0'}, 'children', {_key: 's0'}], offset: 5}, + focus: {path: [{_key: 'b0'}, 'children', {_key: 's0'}], offset: 5}, }, }) editor.send({type: 'insert.text', text: '!'}) await vi.waitFor(() => { expect(patches).toEqual([ - unsetPatch, { type: 'diffMatchPatch', path: [{_key: 'b0'}, 'children', {_key: 's0'}, 'text'], value: stringifyPatches( - makePatches(makeDiff('hello changed', 'hello changed!')), + makePatches(makeDiff('hello changed', 'hello! changed')), ), origin: 'local', }, + { + type: 'set', + path: [{_key: 'b0'}, 'markDefs'], + value: [ + {_key: 'm0', _type: 'link', href: 'https://example.com/kept'}, + ], + origin: 'local', + }, + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + { + _key: 's0', + _type: 'span', + text: 'hello! changed', + marks: ['m0'], + }, + ], + markDefs: [ + {_key: 'm0', _type: 'link', href: 'https://example.com/kept'}, + ], + style: 'normal', + }, ]) }) }) - test('Scenario: a startup value with a keyless child is repaired in the engine without emitting patches', async () => { + test('Scenario: a startup value with a keyless child emits the repair patch immediately, before any local edit', async () => { const patches: Array = [] const mutations: Array = [] const {editor} = await createTestEditor({ @@ -2282,7 +2318,18 @@ describe('event.update value: auto-resolved invalid blocks', () => { ), }) + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) expect(editor.getSnapshot().context.value).toEqual([ { _key: 'b0', @@ -2294,11 +2341,6 @@ describe('event.update value: auto-resolved invalid blocks', () => { ]) }) - // No mutation leaves a pristine editor on open. "Nothing was emitted" - // can't be awaited directly, so prove it with a causal sentinel: make - // one local edit and assert its patches are the only ones ever - // collected. Event ordering guarantees a would-be repair emission had - // flushed before the sentinel's. editor.send({ type: 'select', at: { @@ -2315,14 +2357,2518 @@ describe('event.update value: auto-resolved invalid blocks', () => { origin: 'local', } + // The repair already published on open: the local edit's flush carries + // only its own patch. await vi.waitFor(() => { - expect(patches).toEqual([sentinelPatch]) + expect(patches).toEqual([repairPatch, sentinelPatch]) expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], [sentinelPatch], ]) }) }) + test('Scenario: a host that repaired the same defect with its own key supersedes the held startup repair before it flushes', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + // The engine's own intake repair publishes immediately, same as when + // no newer value ever arrives. + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host (e.g. Sanity Studio, repairing the same missing-key defect + // on draft creation) already persisted its own key for the same span, + // and this value lands well before the batcher's flush interval + // (500ms in test mode) elapses. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'hello', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'hello', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // A genuinely new local edit flushes on its own cadence (typing + // debounce, then the flush interval); waiting for that flush lands + // well past the point the superseded repair would have flushed too, + // so asserting `mutations` holds only this one proves the engine's + // own `k2` never reached the host. + editor.send({ + type: 'select', + at: { + anchor: { + path: [{_key: 'b0'}, 'children', {_key: 'hostKey'}], + offset: 5, + }, + focus: { + path: [{_key: 'b0'}, 'children', {_key: 'hostKey'}], + offset: 5, + }, + }, + }) + editor.send({type: 'insert.text', text: '!'}) + + await vi.waitFor(() => { + expect(mutations).toEqual([ + { + type: 'mutation', + patches: [ + { + type: 'diffMatchPatch', + path: [{_key: 'b0'}, 'children', {_key: 'hostKey'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('hello', 'hello!'))), + origin: 'local', + }, + ], + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'hello!', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }, + ]) + }) + }) + + test('Scenario: a read-only startup value with a keyless child emits the repair patch immediately but holds the repair mutation until editable', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + expect(editor.getSnapshot().context.readOnly).toBe(true) + }) + + // The batcher's flush interval (500ms in test mode) fires well within + // this window: waiting past it, still read-only, proves the repair + // mutation is held, not just not-yet-flushed. No local edit can reach + // the engine while it's read-only, so there's no flush to anchor this + // wait on instead; the assertion is exactly that the interval keeps + // firing and keeps doing nothing. + await new Promise((resolve) => setTimeout(resolve, 600)) + + expect(mutations).toEqual([]) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a read-only editor holding an intake repair still applies later value updates', async () => { + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // A genuinely different remote value, still read-only: the intake + // repair holds a mutation the editor can never flush, but that + // mutation carries no unflushed user work, so it must not block this + // update from landing. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'goodbye', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'goodbye', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + }) + + test('Scenario: a remote patch batch touching unrelated content does not drop a held repair mutation', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + }) + + // A collaborator edits the other block. The engine applies the change, + // but nothing about it supersedes the held repair: the store still + // lacks the minted key, and no normalization pass re-mints it (the + // engine's own tree is already repaired), so the held mutation is the + // only carrier of the fix. + editor.send({ + type: 'patches', + patches: [ + { + type: 'set', + path: [{_key: 'b1'}, 'children', {_key: 's1'}, 'text'], + value: 'there', + origin: 'remote', + }, + ], + snapshot: undefined, + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'there', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a remote patch batch that changes nothing does not drop a held repair mutation', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + }) + + // A remote batch the engine ignores: `setIfMissing` on a text that + // already exists applies nothing, so nothing supersedes the held + // repair and no normalization pass could have re-minted it. + editor.send({ + type: 'patches', + patches: [ + { + type: 'setIfMissing', + path: [{_key: 'b0'}, 'children', {_key: 'k2'}, 'text'], + value: 'hello', + origin: 'remote', + }, + ], + snapshot: undefined, + }) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: an update value snapshot arriving with a different key for the same defect drops a repair minted by a remote patch batch', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + // A remote patch batch drops the child's `_key`, minted fresh by + // normalization firing inside this `patches` frame: no value-sync pass + // is in progress when this repair is minted. + editor.send({ + type: 'patches', + patches: [ + { + type: 'set', + path: [{_key: 'b0'}, 'children'], + value: [{_type: 'span', text: 'hello', marks: []}], + origin: 'remote', + }, + ], + snapshot: undefined, + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + }) + + // Before the batcher's flush interval (500ms in test mode) elapses, a + // full snapshot arrives that already fixed the same defect, under a + // key a different client minted. This is a genuine edit as far as the + // sync is concerned (the inbound child matches neither the repair + // journal's `beforeShape` nor its `afterShape`), so the sync overwrites + // the locally minted key with the inbound one; the held repair bulk, + // minted between passes, must not also flush and reapply the local key + // over it. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'host-key', _type: 'span', text: 'hello', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'host-key', _type: 'span', text: 'hello', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // A genuinely new local edit flushes on its own cadence (typing + // debounce, then the flush interval); waiting for that flush lands + // well past the point the stale, locally minted key would have + // flushed too, so asserting `mutations` holds only this one proves it + // never did. + editor.send({ + type: 'select', + at: { + anchor: { + path: [{_key: 'b0'}, 'children', {_key: 'host-key'}], + offset: 5, + }, + focus: { + path: [{_key: 'b0'}, 'children', {_key: 'host-key'}], + offset: 5, + }, + }, + }) + editor.send({type: 'insert.text', text: '!'}) + + await vi.waitFor(() => { + expect(mutations).toEqual([ + { + type: 'mutation', + patches: [ + { + type: 'diffMatchPatch', + path: [{_key: 'b0'}, 'children', {_key: 'host-key'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('hello', 'hello!'))), + origin: 'local', + }, + ], + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'host-key', _type: 'span', text: 'hello!', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }, + ]) + }) + + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'host-key', _type: 'span', text: 'hello!', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + test('Scenario: a held repair superseded by a corrected snapshot does not flush', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // A corrected echo carrying the repair the engine already applied: the + // host learned the key before this snapshot was produced, so the held + // repair bulk addresses a key the document has already learned and + // must not replay. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + editor.send({type: 'update readOnly', readOnly: false}) + + // A genuinely new local edit, now that the editor is editable, flushes + // on its own cadence (typing debounce, then the flush interval); + // waiting for that flush lands well past the point the stale repair + // would have flushed too, so asserting `mutations` holds only this + // one proves it was dropped, not just not-yet-flushed. + editor.send({ + type: 'select', + at: { + anchor: {path: [{_key: 'b0'}, 'children', {_key: 'k2'}], offset: 5}, + focus: {path: [{_key: 'b0'}, 'children', {_key: 'k2'}], offset: 5}, + }, + }) + editor.send({type: 'insert.text', text: '!'}) + + await vi.waitFor(() => { + expect(mutations).toEqual([ + { + type: 'mutation', + patches: [ + { + type: 'diffMatchPatch', + path: [{_key: 'b0'}, 'children', {_key: 'k2'}, 'text'], + value: stringifyPatches(makePatches(makeDiff('hello', 'hello!'))), + origin: 'local', + }, + ], + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'k2', _type: 'span', text: 'hello!', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }, + ]) + }) + }) + + test('Scenario: a stale echo of an already-repaired block, alongside a genuinely new block, does not re-repair and does not drop the held repair', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host echoes back a snapshot that never picked up the repair: `b0` + // still carries its pre-repair keyless span. A fresh array (not the one + // the editor last sent) with an unrelated second block makes this a new + // value, so the machine reconciles it instead of no-opping. The repair + // journal recognizes `b0`'s echo (its shape matches the block's + // pre-repair journal entry, and the engine still holds exactly the + // post-repair shape) and leaves it alone; `b1` is genuinely new content. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // No second repair: the echo was recognized, not re-repaired. + expect(patches).toEqual([repairPatch]) + + editor.send({type: 'update readOnly', readOnly: false}) + + // `b0`'s key is in the pass's echoed-block-keys set both times, so + // `dropSupersededRepairs` keeps its held repair instead of dropping it + // as superseded by a snapshot that, in truth, never picked it up. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test("Scenario: two independently broken blocks, one host-repaired and one still echoing, flush only the still-echoing block's repair once editable", async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatchA = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + const repairPatchB = { + type: 'set', + path: [{_key: 'b1'}, 'children', 0, '_key'], + value: 'k3', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatchA, repairPatchB]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 'k3', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host (e.g. Sanity Studio) persisted its own key for `b1`'s span, + // but never picked up `b0`'s repair: `b0` still echoes its pre-repair + // keyless shape. Sent twice, with a different control block each time + // so the machine treats every send as a new value instead of + // no-opping: `b0`'s repair must survive both settles on its own echo + // match, not on the one-settle current-generation exemption. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'control-a', + _type: 'block', + children: [{_key: 'ca0', _type: 'span', text: 'a', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'control-a', + _type: 'block', + children: [{_key: 'ca0', _type: 'span', text: 'a', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'control-b', + _type: 'block', + children: [{_key: 'cb0', _type: 'span', text: 'b', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [ + {_key: 'hostKey', _type: 'span', text: 'world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'control-b', + _type: 'block', + children: [{_key: 'cb0', _type: 'span', text: 'b', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // No re-repair of either block: `b0`'s echo was recognized, and `b1`'s + // repair was superseded by the host's own key, not re-applied. + expect(patches).toEqual([repairPatchA, repairPatchB]) + + editor.send({type: 'update readOnly', readOnly: false}) + + // Only `b0`'s repair flushes: `b1`'s repair bulk was dropped as + // superseded once the host's own key for `b1` landed, and never came + // back, even though `b0` kept echoing in the same passes. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatchA], + ]) + }) + + // The final value keeps the host's own key for `b1`, not the engine's + // superseded mint. + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 'hostKey', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'control-b', + _type: 'block', + children: [{_key: 'cb0', _type: 'span', text: 'b', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + test('Scenario: an invalid first block aborts the sync before a still-echoing repair on a later block, which is not dropped', async () => { + const patches: Array = [] + const mutations: Array = [] + const invalidValueEvents: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'bValid', + _type: 'block', + children: [{_key: 'sValid', _type: 'span', text: 'first', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + if (event.type === 'invalid value') { + invalidValueEvents.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b1'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'bValid', + _type: 'block', + children: [{_key: 'sValid', _type: 'span', text: 'first', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // `bValid` now carries a human-decision defect the schema never + // registered (`image` isn't a registered block object): the walk + // breaks there before ever reaching `b1`, whose incoming value still + // echoes its pre-repair keyless shape. + editor.send({ + type: 'update value', + value: [ + {_key: 'bValid', _type: 'image'}, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(invalidValueEvents).toHaveLength(1) + }) + + // No re-repair, and no drop: the aborted pass never reached `b1`, so + // it must not have authorized dropping its held repair either. + expect(patches).toEqual([repairPatch]) + + editor.send({type: 'update readOnly', readOnly: false}) + + // `b1`'s repair still flushes once editable: the aborted pass never + // examined it, so it was never superseded. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test("Scenario: a genuine replacement of the first block does not disturb the second block's still-echoing repair", async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({blockObjects: [{name: 'image'}]}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b1'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // A genuine replacement of `b0` (different `_key` and `_type`) reaches + // the sync machine's `replaceBlock`, which unsets the old block and + // inserts the replacement at the same index: `b1` shifts into that + // index for the span of the unset. The inbound value still echoes + // `b1`'s pre-repair, keyless shape, exactly as a host that never + // picked up the repair would send it. + editor.send({ + type: 'update value', + value: [ + {_key: 'i0', _type: 'image'}, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + {_key: 'i0', _type: 'image'}, + { + _key: 'b1', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // No second repair for `b1`: the echo was recognized, not treated as + // a genuine edit that would mint a fresh key. + expect(patches).toEqual([repairPatch]) + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a value queued during the streamed initial sync re-mints once the reentrant pass settles', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + readOnly: true, + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + // The initial sync streams its blocks, awaiting a tick before touching + // the first one: sending a second still-broken value here, in the same + // synchronous stretch as editor creation, reliably lands before that + // tick fires and queues as the pending value the initial pass reenters + // with once it settles. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello2', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + const firstRepairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + const secondRepairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k3', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([firstRepairPatch, secondRepairPatch]) + }) + + editor.send({type: 'update readOnly', readOnly: false}) + + // The reentrant pass's own `dropSupersededRepairs` sees the first + // pass's repair as already superseded (the block it targeted no longer + // matches this pass's value) and drops it, same as a non-reentrant + // supersession; only the fresh re-mint for the value that's actually + // current flushes. + await vi.waitFor(() => { + expect(mutations).toEqual([ + { + type: 'mutation', + patches: [secondRepairPatch], + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 'k3', _type: 'span', text: 'hello2', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + }, + ]) + }) + }) + + test('Scenario: a still-keyless echo of an already-repaired block does not re-mint', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host echoes back a snapshot that never picked up the repair: `b0` + // still carries its pre-repair keyless span. A fresh array (not the + // one the editor last sent) with an unrelated second block makes this + // a new value, so the machine reconciles it instead of no-opping. The + // repair journal recognizes `b0`'s echo and leaves it alone; `b1` is + // genuinely new content. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor(() => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // No re-mint, and no second mutation: the echo was recognized, not + // treated as a fresh edit to repair. + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + + test('Scenario: a stale echo of an empty-children repair does not insert another placeholder span', async () => { + const patches: Array = [] + const mutations: Array = [] + const initialValue = [ + { + _key: 'b0', + _type: 'block', + children: [], + markDefs: [], + style: 'normal', + }, + ] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue, + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatches = [ + { + type: 'setIfMissing', + path: [{_key: 'b0'}, 'children'], + value: [], + origin: 'local', + }, + { + type: 'insert', + path: [{_key: 'b0'}, 'children', 0], + position: 'before', + items: [{_key: 'k2', _type: 'span', text: '', marks: []}], + origin: 'local', + }, + ] + + await vi.waitFor(() => { + expect(patches).toEqual(repairPatches) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host echoes back a snapshot that never picked up the repair: + // `children` is still `[]`. A fresh array with an unrelated second + // block makes this a new value, so the machine reconciles it instead + // of no-opping. Without the journal, the engine would wholesale-wipe + // `b0`'s children back to `[]` (unpatched) and normalization would + // insert a second placeholder span at position 0 on top of the one + // already persisted, growing the stored content by one span per echo. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + {timeout: 5000}, + ) + + // No further patches, and applying every patch ever emitted to the + // original value yields exactly one placeholder span, not two. + expect(patches).toEqual(repairPatches) + // The repair patch is an intake repair, so it never made the editor + // busy for the sync above: the value already reconciled by the time + // this runs, while the repair mutation itself, unaffected by that + // sync, still flushes on its own schedule. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + repairPatches, + ]) + }) + expect(applyAll(initialValue, patches)).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + test('Scenario: a stale echo of a keyless-block repair does not re-repair', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'k2', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host echoes back a snapshot that never picked up the repair: + // the block is still keyless. A fresh array with an unrelated second + // block makes this a new value, so the machine reconciles it instead + // of no-opping. + editor.send({ + type: 'update value', + value: [ + { + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'k2', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + {timeout: 5000}, + ) + + expect(patches).toEqual([repairPatch]) + // The repair patch is an intake repair, so it never made the editor + // busy for the sync above: the value already reconciled by the time + // this runs, while the repair mutation itself, unaffected by that + // sync, still flushes on its own schedule. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a stale echo of a duplicate top-level block key repair does not re-repair', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b0', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [1, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'k2', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host echoes back a snapshot that never picked up the repair: the + // second block still carries the duplicate `b0` key. A third, + // unrelated block makes this a new value, so the machine reconciles it + // instead of no-opping. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b0', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock, + { + _key: 'b2', + _type: 'block', + children: [{_key: 's2', _type: 'span', text: 'again', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'k2', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b2', + _type: 'block', + children: [{_key: 's2', _type: 'span', text: 'again', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + {timeout: 5000}, + ) + + expect(patches).toEqual([repairPatch]) + // The repair patch is an intake repair, so it never made the editor + // busy for the sync above: the value already reconciled by the time + // this runs, while the repair mutation itself, unaffected by that + // sync, still flushes on its own schedule. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a stale echo of a duplicate child key repair does not re-repair', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 's0', _type: 'span', text: 'hello', marks: []}, + {_key: 's0', _type: 'span', text: ' world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 1, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 's0', _type: 'span', text: 'hello', marks: []}, + {_key: 'k2', _type: 'span', text: ' world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The host echoes back a snapshot that never picked up the repair: + // `b0`'s children still carry the duplicate `s0` key. An unrelated + // second block makes this a new value, so the machine reconciles it + // instead of no-opping. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 's0', _type: 'span', text: 'hello', marks: []}, + {_key: 's0', _type: 'span', text: ' world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'again', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + await vi.waitFor( + () => { + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 's0', _type: 'span', text: 'hello', marks: []}, + {_key: 'k2', _type: 'span', text: ' world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'again', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + {timeout: 5000}, + ) + + expect(patches).toEqual([repairPatch]) + // The repair patch is an intake repair, so it never made the editor + // busy for the sync above: the value already reconciled by the time + // this runs, while the repair mutation itself, unaffected by that + // sync, still flushes on its own schedule. + await vi.waitFor(() => { + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + }) + }) + + test('Scenario: a stale echo with different text on the repaired span is a genuine edit and re-repairs with a fresh key', async () => { + const patches: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + }} + /> + ), + }) + + const firstRepairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([firstRepairPatch]) + }) + + // Still keyless, but the text itself is different: this is not the + // journaled pre-repair shape, so it is a genuine edit, not an echo. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello2', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + const secondRepairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k3', + origin: 'local', + } + + await vi.waitFor( + () => { + expect(patches).toEqual([firstRepairPatch, secondRepairPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k3', _type: 'span', text: 'hello2', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + // The sync machine parks in `busy` while its own emitted mutation + // flushes and re-checks on a 1s timer. + {timeout: 5000}, + ) + }) + + test('Scenario: an acknowledged repair, later genuinely re-emptied, repairs anew with a fresh key', async () => { + const patches: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + }} + /> + ), + }) + + const firstRepairPatches = [ + { + type: 'setIfMissing', + path: [{_key: 'b0'}, 'children'], + value: [], + origin: 'local', + }, + { + type: 'insert', + path: [{_key: 'b0'}, 'children', 0], + position: 'before', + items: [{_key: 'k2', _type: 'span', text: '', marks: []}], + origin: 'local', + }, + ] + + await vi.waitFor(() => { + expect(patches).toEqual(firstRepairPatches) + }) + + // The host acknowledges the repair: its snapshot now matches exactly + // what the engine holds. This retires the journal entry. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + // Give the ACK pass time to fully settle: the sync machine parks in + // `busy` while its own emitted patches are still deferred by the + // mutation batcher, and only accepts a new value once it re-checks + // and finds itself idle. The ACK produces no patch either way, so + // waiting on `patches` alone would not prove it settled; sending the + // next value too early would evaluate it against the stale + // pre-ACK `previousValue` and drop it as "not new". `busy` re-checks + // on its own internal 1s timer with no externally observable signal + // (even the first repair's mutation flushing doesn't trigger an + // immediate re-check), so there's no event to anchor this wait on + // instead of a fixed delay comfortably past that timer. + await new Promise((resolve) => setTimeout(resolve, 1600)) + + // A later, genuine clear (not a stale echo of the original defect): + // with the journal entry retired, this must repair again, minting a + // fresh key rather than being mistaken for an echo of the first repair. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [], + markDefs: [], + style: 'normal', + }, + ], + }) + + const secondRepairPatches = [ + { + type: 'setIfMissing', + path: [{_key: 'b0'}, 'children'], + value: [], + origin: 'local', + }, + { + type: 'insert', + path: [{_key: 'b0'}, 'children', 0], + position: 'before', + items: [{_key: 'k3', _type: 'span', text: '', marks: []}], + origin: 'local', + }, + ] + + await vi.waitFor( + () => { + expect(patches).toEqual([...firstRepairPatches, ...secondRepairPatches]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k3', _type: 'span', text: '', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + // The sync machine parks in `busy` while its own emitted mutation + // flushes and re-checks on a 1s timer. + {timeout: 5000}, + ) + }) + + test('Scenario: a stale echo arriving after a local edit is a normal sync, not an echo (the local edit already retired the journal entry)', async () => { + const patches: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + }) + + // A local edit on the repaired span: this touches `b0`, retiring its + // journal entry, same as any other operation reaching the block. + editor.send({ + type: 'select', + at: { + anchor: {path: [{_key: 'b0'}, 'children', {_key: 'k2'}], offset: 5}, + focus: {path: [{_key: 'b0'}, 'children', {_key: 'k2'}], offset: 5}, + }, + }) + editor.send({type: 'insert.text', text: '!'}) + + const localEditPatch = { + type: 'diffMatchPatch', + path: [{_key: 'b0'}, 'children', {_key: 'k2'}, 'text'], + value: '@@ -1,5 +1,6 @@\n hello\n+!\n', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch, localEditPatch]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'hello!', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + // The pre-repair echo arrives after the local edit, alongside an + // unrelated new block so the machine reconciles instead of no-opping. + // With no journal entry left to consult (the local edit above retired + // it), this is an ordinary sync: the incoming (still keyless) shape + // wins, same as it would without a repair journal at all, and the + // local edit's text is overwritten. This is pre-existing, orthogonal + // behavior: the journal does not protect a block once a local edit + // has touched it. + editor.send({ + type: 'update value', + value: [ + { + _key: 'b0', + _type: 'block', + children: [{_type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + }) + + const secondRepairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 0, '_key'], + value: 'k3', + origin: 'local', + } + + await vi.waitFor( + () => { + expect(patches).toEqual([ + repairPatch, + localEditPatch, + secondRepairPatch, + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 'k3', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }, + // The sync machine parks in `busy` while its own emitted mutation + // flushes and re-checks on a 1s timer. + {timeout: 5000}, + ) + }) + + test('Scenario: a startup value with a duplicate sibling _key emits the re-mint patch immediately, before any local edit', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 's0', _type: 'span', text: 'hello', marks: []}, + {_key: 's0', _type: 'span', text: ' world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b0'}, 'children', 1, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [ + {_key: 's0', _type: 'span', text: 'hello', marks: []}, + {_key: 'k2', _type: 'span', text: ' world', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + }) + + test('Scenario: a startup value with a keyless child in the second block emits the repair patch addressed to that block', async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairPatch = { + type: 'set', + path: [{_key: 'b1'}, 'children', 0, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + const initialValue = [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _key: 'b1', + _type: 'block', + children: [{_type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ] + + expect(applyAll(initialValue, patches)).toEqual([ + initialValue[0], + { + _key: 'b1', + _type: 'block', + children: [{_key: 'k2', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + }) + + test("Scenario: a startup value with a keyless second block emits the repair patch addressed by that block's index, not the first block's", async () => { + const patches: Array = [] + const mutations: Array = [] + const {editor} = await createTestEditor({ + keyGenerator: createTestKeyGenerator(), + schemaDefinition: defineSchema({}), + initialValue: [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock, + ], + children: ( + { + if (event.type === 'patch') { + patches.push(event.patch) + } + if (event.type === 'mutation') { + mutations.push(event) + } + }} + /> + ), + }) + + const repairedBlock1 = { + _key: 'k2', + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + } + const repairPatch = { + type: 'set', + path: [1, '_key'], + value: 'k2', + origin: 'local', + } + + await vi.waitFor(() => { + expect(patches).toEqual([repairPatch]) + expect(mutations.map((mutation) => mutation.patches)).toEqual([ + [repairPatch], + ]) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + repairedBlock1, + ]) + }) + + const initialValue: Array = [ + { + _key: 'b0', + _type: 'block', + children: [{_key: 's0', _type: 'span', text: 'hello', marks: []}], + markDefs: [], + style: 'normal', + }, + { + _type: 'block', + children: [{_key: 's1', _type: 'span', text: 'world', marks: []}], + markDefs: [], + style: 'normal', + } as unknown as PortableTextBlock, + ] + + expect(applyAll(initialValue, patches)).toEqual([ + initialValue[0], + repairedBlock1, + ]) + }) + test('Scenario: replacing a whole value with the same block/span keys emits no patches', async () => { const patches: Array = [] const {editor} = await createTestEditor({ diff --git a/packages/editor/tests/normalization.test.tsx b/packages/editor/tests/normalization.test.tsx index caad86fb9..c8ebc22e5 100644 --- a/packages/editor/tests/normalization.test.tsx +++ b/packages/editor/tests/normalization.test.tsx @@ -379,7 +379,7 @@ describe('normalization', () => { }) }) - test('Scenario: an orphaned markDef with no referencing span is removed', async () => { + test('Scenario: an orphaned markDef is pruned when a local edit touches the block', async () => { const keyGenerator = createTestKeyGenerator() const blockKey = keyGenerator() const markDefKey = keyGenerator() @@ -406,8 +406,10 @@ describe('normalization', () => { ], }, ] + const onChange = vi.fn() const {editor} = await createTestEditor({ + children: , keyGenerator, initialValue, }) @@ -415,6 +417,9 @@ describe('normalization', () => { editor.send({type: 'focus'}) await vi.waitFor(() => { + expect(onChange).toHaveBeenCalledWith({type: 'ready'}) + // Focusing does not dirty the block, so the orphaned markDef + // stays exactly as the document has it until a local edit. expect(editor.getSnapshot().context.value).toEqual([ { _key: blockKey, @@ -427,6 +432,56 @@ describe('normalization', () => { marks: [], }, ], + markDefs: [ + { + _key: markDefKey, + _type: 'link', + href: 'https://sanity.io', + }, + ], + style: 'normal', + }, + ]) + }) + + editor.send({ + type: 'select', + at: { + anchor: { + path: [{_key: blockKey}, 'children', {_key: spanKey}], + offset: 5, + }, + focus: { + path: [{_key: blockKey}, 'children', {_key: spanKey}], + offset: 5, + }, + }, + }) + editor.send({type: 'insert.text', text: '!'}) + + await vi.waitFor(() => { + expect(onChange).toHaveBeenCalledWith({ + type: 'patch', + patch: { + type: 'set', + path: [{_key: blockKey}, 'markDefs'], + value: [], + origin: 'local', + }, + intakeRepair: false, + }) + expect(editor.getSnapshot().context.value).toEqual([ + { + _key: blockKey, + _type: 'block', + children: [ + { + _key: spanKey, + _type: 'span', + text: 'Hello!', + marks: [], + }, + ], markDefs: [], style: 'normal', }, diff --git a/packages/editor/tests/placeholder-block.test.tsx b/packages/editor/tests/placeholder-block.test.tsx index 003774a6f..b8a47b7cb 100644 --- a/packages/editor/tests/placeholder-block.test.tsx +++ b/packages/editor/tests/placeholder-block.test.tsx @@ -282,6 +282,24 @@ describe(createPlaceholderBlock.name, () => { ), }) + const intakeRepairPatches = [ + { + type: 'setIfMissing', + path: [{_key: 'k0'}, 'children'], + value: [], + }, + { + type: 'insert', + path: [{_key: 'k0'}, 'children', 0], + position: 'before', + items: [{_key: 'k2', _type: 'span', marks: [], text: ''}], + }, + ] + + await vi.waitFor(() => { + expect(patches).toEqual(intakeRepairPatches) + }) + editor.send({ type: 'patches', patches: [ @@ -304,7 +322,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ]) - expect(patches).toEqual([]) + expect(patches).toEqual(intakeRepairPatches) }) editor.send({ @@ -321,7 +339,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ]) - expect(patches).toEqual([]) + expect(patches).toEqual(intakeRepairPatches) }) }) }) diff --git a/packages/editor/tests/self-solving.test.tsx b/packages/editor/tests/self-solving.test.tsx index e76458c4b..fa17b69f7 100644 --- a/packages/editor/tests/self-solving.test.tsx +++ b/packages/editor/tests/self-solving.test.tsx @@ -100,8 +100,8 @@ describe('Feature: Self-solving', () => { // The deferred `markDefs` default rides the same edit that dirtied // the block, after the user's own patch. expect(patchEvents).toEqual([ - {type: 'patch', patch: strongPatch}, - {type: 'patch', patch: blockPatch}, + {type: 'patch', patch: strongPatch, intakeRepair: false}, + {type: 'patch', patch: blockPatch, intakeRepair: false}, ]) expect(mutationEvents).toEqual([ { @@ -386,6 +386,13 @@ describe('Feature: Self-solving', () => { ), }) + const rekeyPatch = { + origin: 'local', + path: [{_key: blockKey}, 'children', 1, '_key'], + type: 'set', + value: 'k4', + } + await vi.waitFor(() => { expect(editor.getSnapshot().context.value).toEqual([ { @@ -400,7 +407,7 @@ describe('Feature: Self-solving', () => { }, ]) - expect(patches).toEqual([]) + expect(patches).toEqual([rekeyPatch]) }) await userEvent.click(locator) @@ -427,12 +434,7 @@ describe('Feature: Self-solving', () => { ) expect(patches).toEqual([ - { - origin: 'local', - path: [{_key: blockKey}, 'children', 1, '_key'], - type: 'set', - value: 'k4', - }, + rekeyPatch, { origin: 'local', path: [{_key: blockKey}, 'children', {_key: 'k4'}, 'text'], @@ -604,13 +606,28 @@ describe('Feature: Self-solving', () => { ), }) + const rekeyPatches = [ + { + origin: 'local', + type: 'set', + path: [1, '_key'], + value: 'k5', + }, + { + origin: 'local', + type: 'set', + path: [2, '_key'], + value: 'k6', + }, + ] + await vi.waitFor(() => { expect(editor.getSnapshot().context.value).toEqual([ block0, {...block1, _key: 'k5'}, {...image, _key: 'k6'}, ]) - expect(patches).toEqual([]) + expect(patches).toEqual(rekeyPatches) }) await userEvent.click(locator) @@ -650,18 +667,7 @@ describe('Feature: Self-solving', () => { }, ]) expect(patches).toEqual([ - { - origin: 'local', - type: 'set', - path: [1, '_key'], - value: 'k5', - }, - { - origin: 'local', - type: 'set', - path: [2, '_key'], - value: 'k6', - }, + ...rekeyPatches, { origin: 'local', type: 'diffMatchPatch', diff --git a/packages/editor/tests/validation.test.tsx b/packages/editor/tests/validation.test.tsx index 421ee5792..a5c35df5d 100644 --- a/packages/editor/tests/validation.test.tsx +++ b/packages/editor/tests/validation.test.tsx @@ -1,4 +1,4 @@ -import {defineSchema} from '@portabletext/schema' +import {defineSchema, type PortableTextBlock} from '@portabletext/schema' import {createTestKeyGenerator, toTextspec} from '@portabletext/test' import {makeDiff, makePatches, stringifyPatches} from '@sanity/diff-match-patch' import {describe, expect, test, vi} from 'vitest' @@ -96,7 +96,7 @@ describe('Value validation', () => { resolution: { action: 'Remove the item', description: - "Child at index '0' in block with key 'k2' is not an object.", + "Child at index '0' in block with _key 'k2' is not an object.", i18n: { action: 'inputs.portable-text.invalid-value.non-object-child.action', @@ -183,6 +183,7 @@ describe('Value validation', () => { path: [{_key: 'k0'}, 'children', {_key: 'k1'}, 'text'], value: stringifyPatches(makePatches(makeDiff('foo', 'foo!'))), }, + intakeRepair: false, }, { type: 'patch', @@ -192,6 +193,7 @@ describe('Value validation', () => { path: [{_key: 'k0'}, 'markDefs'], value: [], }, + intakeRepair: false, }, { type: 'patch', @@ -201,6 +203,7 @@ describe('Value validation', () => { path: [{_key: 'k0'}, 'style'], value: 'normal', }, + intakeRepair: false, }, { type: 'selection', @@ -348,7 +351,7 @@ describe('Value validation', () => { type: 'invalid value', resolution: { action: 'Remove the item', - description: `Child at index '1' in block with key '${blockKey}' is not an object.`, + description: `Child at index '1' in block with _key '${blockKey}' is not an object.`, i18n: { action: 'inputs.portable-text.invalid-value.non-object-child.action', @@ -461,6 +464,7 @@ describe('Value validation', () => { path: [{_key: blockKey}, 'children', {_key: fooKey}, 'text'], value: stringifyPatches(makePatches(makeDiff('foo', 'foo!'))), }, + intakeRepair: false, }, { type: 'patch', @@ -470,6 +474,7 @@ describe('Value validation', () => { path: [{_key: blockKey}, 'markDefs'], value: [], }, + intakeRepair: false, }, { type: 'patch', @@ -479,6 +484,7 @@ describe('Value validation', () => { path: [{_key: blockKey}, 'style'], value: 'normal', }, + intakeRepair: false, }, { type: 'selection', @@ -611,7 +617,7 @@ describe('Value validation', () => { resolution: { action: 'Remove the item', description: - "Child at index '0' in block with key 'k4' is not an object.", + "Child at index '0' in block with _key 'k4' is not an object.", i18n: { action: 'inputs.portable-text.invalid-value.non-object-child.action', @@ -696,6 +702,7 @@ describe('Value validation', () => { path: [{_key: 'k2'}, 'children', {_key: 'k3'}, 'text'], value: stringifyPatches(makePatches(makeDiff('foo', 'foo!'))), }, + intakeRepair: false, }, { type: 'patch', @@ -705,6 +712,7 @@ describe('Value validation', () => { path: [{_key: 'k2'}, 'markDefs'], value: [], }, + intakeRepair: false, }, { type: 'patch', @@ -714,6 +722,7 @@ describe('Value validation', () => { path: [{_key: 'k2'}, 'style'], value: 'normal', }, + intakeRepair: false, }, { type: 'selection', @@ -769,4 +778,92 @@ describe('Value validation', () => { }, ]) }) + + test("Scenario: a startup value with a keyless second block with a disallowed _type anchors the resolution patch on its index, not the first block's", async () => { + const keyGenerator = createTestKeyGenerator() + const events: Array = [] + await createTestEditor({ + keyGenerator, + initialValue: [ + { + _type: 'block', + _key: keyGenerator(), + children: [ + {_type: 'span', _key: keyGenerator(), text: 'foo', marks: []}, + ], + }, + { + _type: 'image', + children: [{_type: 'span', text: 'bar', marks: []}], + } as unknown as PortableTextBlock, + ], + children: ( + { + events.push(event) + }} + /> + ), + }) + + await vi.waitFor(() => { + expect(events).toEqual([ + // Value sync removes the editor's seed block and inserts the valid + // first block, then stops at the second block: keyless, so the + // resolution's patches and description must anchor on its index + // (`1`) in the synced value, not on the sliced validation call's + // own index (`0`). + { + type: 'operation', + operation: {type: 'unset', path: [{_key: 'k2'}]}, + origin: 'remote', + }, + { + type: 'operation', + operation: { + type: 'insert', + path: [0], + position: 'before', + node: { + _type: 'block', + _key: 'k0', + children: [{_type: 'span', _key: 'k1', text: 'foo', marks: []}], + }, + }, + origin: 'remote', + }, + { + type: 'invalid value', + resolution: { + action: 'Remove the block', + description: "Block at index '1' has invalid _type 'image'", + i18n: { + action: + 'inputs.portable-text.invalid-value.disallowed-type.action', + description: + 'inputs.portable-text.invalid-value.disallowed-type.description', + values: {key: undefined, typeName: 'image'}, + }, + item: { + _type: 'image', + children: [{_type: 'span', text: 'bar', marks: []}], + }, + patches: [{type: 'unset', path: [1]}], + }, + value: [ + { + _type: 'block', + _key: 'k0', + children: [{_type: 'span', _key: 'k1', text: 'foo', marks: []}], + }, + { + _type: 'image', + children: [{_type: 'span', text: 'bar', marks: []}], + }, + ], + }, + {type: 'ready'}, + ]) + }) + }) }) diff --git a/packages/plugin-sdk-value/src/plugin.sdk-value.tsx b/packages/plugin-sdk-value/src/plugin.sdk-value.tsx index 2ea96604e..9ce00000c 100644 --- a/packages/plugin-sdk-value/src/plugin.sdk-value.tsx +++ b/packages/plugin-sdk-value/src/plugin.sdk-value.tsx @@ -694,6 +694,72 @@ const pendingRepairs = new WeakMap() */ const unflushedEdits = new WeakMap() +/** + * The store value a still-unpushed read-only intake repair was diffed away + * from. `applySync` normally reads any editor/store divergence as the store + * having drifted and repairs the editor to match it, but while this holds a + * value, a divergence against that exact value is the held repair itself, + * not the store drifting, and must not be reverted. It clears once the + * store moves on, whether because the held mutation finally pushed or a + * genuinely new remote value arrived; either way, the next `applySync` diffs + * against the current store value on its own merits. + */ +const heldReadOnlyRepairBaselines = new WeakMap() + +/** + * Structural equality over plain JSON values: object keys compare + * order-insensitively, array elements compare order-sensitively by index. + * Duplicated from `packages/editor/src/internal-utils/deep-equal-json.ts` + * (not exported from `@portabletext/editor`) rather than exported from + * core; keep the two in sync. + */ +function deepEqualJson(a: unknown, b: unknown): boolean { + if (a === b) { + return true + } + + if (Array.isArray(a) || Array.isArray(b)) { + if (!Array.isArray(a) || !Array.isArray(b) || a.length !== b.length) { + return false + } + for (let index = 0; index < a.length; index++) { + if (!deepEqualJson(a[index], b[index])) { + return false + } + } + return true + } + + if ( + a !== null && + b !== null && + typeof a === 'object' && + typeof b === 'object' + ) { + const recordA = a as Record + const recordB = b as Record + const keysA = Object.keys(recordA) + const keysB = Object.keys(recordB) + + if (keysA.length !== keysB.length) { + return false + } + + for (const key of keysA) { + if ( + !Object.prototype.hasOwnProperty.call(recordB, key) || + !deepEqualJson(recordA[key], recordB[key]) + ) { + return false + } + } + + return true + } + + return false +} + function computeRepair( editor: Editor, remoteValue: PortableTextBlock[], @@ -725,6 +791,25 @@ function cancelPendingRepair(editor: Editor) { } } +/** + * `dropSupersededRepairs` (in the mutation batcher) only runs on a + * completed `update value` pass; a `patches`-only reconciliation like + * `applySync`'s confirmed repair or `'apply remote patches'` never gives it + * that chance, so a read-only intake repair a newer store state has already + * superseded stays queued and flushes stale on the editable flip. Re-entering + * through `update value` with the snapshot just applied hands the pass that + * judgment: the tree already matches (this reconciliation just made it so), + * so the pass settles without emitting anything, but its completion still + * lets the batcher cull a superseded repair. Scoped to read-only: once + * editable, no repair is held this way, so the extra full-tree walk buys + * nothing. + */ +function judgeHeldRepairsAgainst(editor: Editor, value: PortableTextBlock[]) { + if (editor.getSnapshot().context.readOnly) { + editor.send({type: 'update value', value}) + } +} + function applySync({ editor, getRemoteValue, @@ -738,6 +823,22 @@ function applySync({ return } + const heldRepairBaseline = heldReadOnlyRepairBaselines.get(editor) + if (heldRepairBaseline !== undefined) { + if (deepEqualJson(remoteValue, heldRepairBaseline)) { + // The store hasn't moved since the read-only repair that's still + // waiting to flush: the divergence below would be fully explained by + // that held repair, not by the store drifting, so there's nothing to + // reconcile yet. + return + } + // The store moved past the value the held repair was diffed from, + // whether from the eventual push or a genuinely new remote value: + // either way, the divergence is now real and evaluated on its own + // merits below. + heldReadOnlyRepairBaselines.delete(editor) + } + const first = computeRepair(editor, remoteValue) if (first.convertible && first.patches.length === 0) { cancelPendingRepair(editor) @@ -815,43 +916,73 @@ function applySync({ }) } editor.send({type: 'update value', value: latestRemote}) + } else { + judgeHeldRepairsAgainst(editor, latestRemote) } }, REPAIR_CONFIRM_DELAY) pendingRepairs.set(editor, {signature: first.signature, timer}) } -const listenToEditor = fromCallback( - ({sendBack, input}) => { - const patchSubscription = input.editor.on('patch', () => { - // Every 'patch' event is a local edit (remote application suppresses - // patch generation), so the store now lags the editor until the next - // mutation flush. - unflushedEdits.set(input.editor, true) - sendBack({type: 'patch emitted'}) - }) - - const mutationSubscription = input.editor.on('mutation', (event) => { - unflushedEdits.set(input.editor, false) - if (debug.mutation.enabled) { - debug.mutation('flushed %o', { - flushText: debugTextOf(event.value), - snapshotText: debugTextOf(input.editor.getSnapshot().context.value), - }) +const listenToEditor = fromCallback< + AnyEventObject, + { + editor: Editor + getRemoteValue: ValueSyncConfig['getRemoteValue'] + } +>(({sendBack, input}) => { + const patchSubscription = input.editor.on('patch', (event) => { + if (event.intakeRepair && input.editor.getSnapshot().context.readOnly) { + // An intake repair is never user work, regardless of read-only + // state (a handful of Behavior events, like `select`, still run + // their actions while read-only, and a mutating Behavior on one of + // those produces user work, not a repair). While read-only, the + // mutation that would flush this repair is held until the editor + // becomes editable. Latching here would park the machine and + // `unflushedEdits` behind a flush that cannot come, freezing store + // updates for the rest of the read-only session. The held repair + // still pushes once it flushes: 'mutation flushed' is handled in + // every state. + // + // `applySync` still needs to know the repair is unpushed: record + // the store value it was diffed away from, so a divergence against + // that exact value reads as the held repair, not the store + // drifting, and doesn't get reverted before it can flush. + const remoteValue = input.getRemoteValue() + if (remoteValue) { + heldReadOnlyRepairBaselines.set(input.editor, remoteValue) } - sendBack({ - type: 'mutation flushed', - value: event.value, - patches: event.patches, + return + } + // Every remaining `patch` event marks genuine user work: an edit, or + // a mutating Behavior action, never a remote application bouncing its + // own patches back, nor a repair still held read-only. Either way, + // the store now lags the editor until the next mutation flush. + unflushedEdits.set(input.editor, true) + sendBack({type: 'patch emitted'}) + }) + + const mutationSubscription = input.editor.on('mutation', (event) => { + unflushedEdits.set(input.editor, false) + heldReadOnlyRepairBaselines.delete(input.editor) + if (debug.mutation.enabled) { + debug.mutation('flushed %o', { + flushText: debugTextOf(event.value), + snapshotText: debugTextOf(input.editor.getSnapshot().context.value), }) + } + sendBack({ + type: 'mutation flushed', + value: event.value, + patches: event.patches, }) + }) - return () => { - patchSubscription.unsubscribe() - mutationSubscription.unsubscribe() - } - }, -) + return () => { + patchSubscription.unsubscribe() + mutationSubscription.unsubscribe() + } +}) const listenToRemote = fromCallback< AnyEventObject, @@ -950,6 +1081,9 @@ const valueSyncMachine = setup({ return } context.editor.send({type: 'patches', patches, snapshot}) + if (remoteValue) { + judgeHeldRepairsAgainst(context.editor, remoteValue) + } }) }, }, @@ -970,7 +1104,10 @@ const valueSyncMachine = setup({ invoke: [ { src: 'listen to editor', - input: ({context}) => ({editor: context.editor}), + input: ({context}) => ({ + editor: context.editor, + getRemoteValue: context.getRemoteValue, + }), }, { src: 'listen to remote', diff --git a/packages/plugin-sdk-value/src/plugin.value-sync.browser.test.tsx b/packages/plugin-sdk-value/src/plugin.value-sync.browser.test.tsx index 7949aa5ca..697fe8818 100644 --- a/packages/plugin-sdk-value/src/plugin.value-sync.browser.test.tsx +++ b/packages/plugin-sdk-value/src/plugin.value-sync.browser.test.tsx @@ -1,9 +1,15 @@ import type {Editor, Patch as PtePatch} from '@portabletext/editor' import {EditorProvider, PortableTextEditable} from '@portabletext/editor' -import {EditorRefPlugin} from '@portabletext/editor/plugins' +import {defineBehavior, forward, raise} from '@portabletext/editor/behaviors' +import type {Behavior} from '@portabletext/editor/behaviors' +import {BehaviorPlugin, EditorRefPlugin} from '@portabletext/editor/plugins' import {toTextspec} from '@portabletext/editor/test' import {applyAll, type JSONValue} from '@portabletext/patches' -import {defineSchema, type PortableTextBlock} from '@portabletext/schema' +import { + defineSchema, + isTextBlock, + type PortableTextBlock, +} from '@portabletext/schema' import {createTestKeyGenerator} from '@portabletext/test' import {createRef} from 'react' import {afterEach, describe, expect, test, vi} from 'vitest' @@ -120,6 +126,8 @@ async function createSyncedEditor(options: { initialValue?: PortableTextBlock[] store: MockStore | MockPatchStore schemaDefinition?: ReturnType + readOnly?: boolean + behaviors?: Array }) { const editorRef = createRef() const keyGenerator = createTestKeyGenerator() @@ -131,10 +139,14 @@ async function createSyncedEditor(options: { keyGenerator, schemaDefinition: options.schemaDefinition ?? defineSchema({}), initialValue: options.initialValue, + readOnly: options.readOnly, }} > + {options.behaviors ? ( + + ) : null} , ) - const locator = page.getByRole('textbox') + // A read-only editable carries no ARIA `textbox` role, so `getByRole` + // never resolves; the always-present `data-pt-editor` marker locates it + // instead. + const locator = options.readOnly + ? await vi.waitFor(() => { + const element = result.container.querySelector('[data-pt-editor]') + if (element === null) { + throw new Error('Expected to find an element with `data-pt-editor`') + } + return page.elementLocator(element) + }) + : page.getByRole('textbox') await vi.waitFor(() => expect.element(locator).toBeInTheDocument()) return { @@ -173,6 +196,15 @@ function getEditorText(editor: Editor): string { return toTextspec(editor.getSnapshot().context) } +function getFirstChildKey(editor: Editor): string | undefined { + const context = editor.getSnapshot().context + const [firstBlock] = context.value + if (!isTextBlock({schema: context.schema}, firstBlock)) { + return undefined + } + return firstBlock.children[0]?._key +} + // ---- Tests ---- describe('ValueSyncPlugin', () => { @@ -305,6 +337,170 @@ describe('ValueSyncPlugin', () => { }) describe('remote changes apply to editor', () => { + test('remote changes keep applying after an intake repair while read-only', async () => { + const store = createMockValueStore([ + { + _type: 'block', + _key: 'b1', + children: [{_type: 'span', text: 'Hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + const {editor, unmount} = await createSyncedEditor({ + store, + readOnly: true, + }) + cleanup = unmount + + // The keyless child provokes an intake repair: the repair patch + // relays immediately, but the mutation that would flush it is held + // for as long as the editor stays read-only. + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: Hello') + }) + + store.setRemoteValue([makeBlock('b1', 'Goodbye')]) + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: Goodbye') + }) + }) + + test('a held read-only repair survives a store-driven sync pass and flushes once editable', async () => { + const store = createMockValueStore([ + { + _type: 'block', + _key: 'b1', + children: [{_type: 'span', text: 'Hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + const {editor, unmount} = await createSyncedEditor({ + store, + readOnly: true, + }) + cleanup = unmount + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: Hello') + }) + + const mintedKey = getFirstChildKey(editor) + expect(mintedKey).toEqual('k2') + + // Outlast the idle state's quiescent one-shot repair (500ms) plus the + // repair confirmation window (150ms in test mode), so a store-driven + // sync pass runs against the store's still-broken (keyless) value. + // No local edit can reach the editor while it's read-only, so + // there's no flush to anchor this wait on instead; the assertion is + // exactly that the sync pass runs and does nothing. + await new Promise((resolve) => setTimeout(resolve, 800)) + + expect(getEditorText(editor)).toEqual('B: Hello') + expect(getFirstChildKey(editor)).toEqual(mintedKey) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(store.pushValue).toHaveBeenCalledTimes(1) + }) + expect(store.pushValue).toHaveBeenCalledWith([ + { + _type: 'block', + _key: 'b1', + children: [ + {_type: 'span', _key: mintedKey, text: 'Hello', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + }) + + test('a mutating Behavior on `select` while read-only is never diffed away as a held repair', async () => { + const store = createMockValueStore([ + makeBlock('keep', 'keep'), + makeBlock('gone', 'gone'), + ]) + const {editor, unmount} = await createSyncedEditor({ + store, + readOnly: true, + behaviors: [ + defineBehavior({ + on: 'select', + actions: [ + ({event}) => { + const anchorSegment = event.at?.anchor.path[0] + const targetsKeepBlock = + typeof anchorSegment === 'object' && + anchorSegment !== null && + '_key' in anchorSegment && + anchorSegment._key === 'keep' + + // `select` is one of the handful of Behavior events the + // edit-mode machine still admits while read-only, so this + // action runs, and mutates, even though the editor never + // left read-only. + return targetsKeepBlock + ? [ + forward(event), + raise({type: 'delete.block', at: [{_key: 'gone'}]}), + ] + : [forward(event)] + }, + ], + }), + ], + }) + cleanup = unmount + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: keep\nB: gone') + }) + + editor.send({ + type: 'select', + at: { + anchor: {path: [{_key: 'keep'}, 'children', 0], offset: 0}, + focus: {path: [{_key: 'keep'}, 'children', 0], offset: 0}, + }, + }) + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: |keep') + }) + + // A genuinely new remote value, still read-only: the store never + // learned about the delete above (its mutation is held), so this + // is exactly the divergence the background repair diff would + // otherwise "fix" by resurrecting the deleted block. + store.setRemoteValue([ + makeBlock('keep', 'kept'), + makeBlock('gone', 'gone'), + ]) + + // Outlast the repair confirmation window (150ms in test mode) plus + // a retry of it, so a would-be repair has had every chance to + // apply. No local edit can reach the editor while it's read-only, + // so there's no flush to anchor this wait on instead; the assertion + // is exactly that no repair fires. + await new Promise((resolve) => setTimeout(resolve, 400)) + + expect(getEditorText(editor)).toEqual('B: |keep') + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect( + store.pushValue.mock.calls.some(([value]) => + value.every((block: PortableTextBlock) => block._key !== 'gone'), + ), + ).toBe(true) + }) + }) + test('remote change updates editor when idle', async () => { const store = createMockValueStore() const {editor, unmount} = await createSyncedEditor({store}) @@ -483,6 +679,192 @@ describe('ValueSyncPlugin', () => { }) }) + describe('a held read-only repair superseded by a newer store update', () => { + // Field regression: a keyless child provokes a client-side intake + // repair that mints its own key and holds the mutation read-only. + // Before the editor becomes editable, a concurrent client's own fix + // for the same defect lands, minting a different key and editing the + // text. `dropSupersededRepairs` only runs on a completed `update + // value` pass, which a patches-only reconciliation never triggered, so + // the stale held repair used to survive to the editable flip and + // flush the pre-update content over the newer store state. + test('a store transaction over the patch channel supersedes a held repair; its stale key never overwrites the store', async () => { + const store = createMockPatchStore([ + { + _type: 'block', + _key: 'b1', + children: [{_type: 'span', text: 'Hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + const {editor, unmount} = await createSyncedEditor({ + store, + readOnly: true, + }) + cleanup = unmount + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: Hello') + }) + expect(getFirstChildKey(editor)).toEqual('k2') + + // The concurrent client's own key and text for the same child, + // delivered as a transaction over the patch channel. The + // accompanying patch (harmless on its own) only needs to be + // non-empty to drive the plugin's patches-path reconciliation; the + // key/text change itself reaches the editor through the newer store + // value, not through this patch. + store.receiveRemoteTransaction( + [ + { + _type: 'block', + _key: 'b1', + children: [ + { + _type: 'span', + _key: 'server-key', + text: 'server text', + marks: [], + }, + ], + markDefs: [], + style: 'normal', + }, + ], + [ + { + type: 'set', + origin: 'remote', + path: [{_key: 'b1'}, 'style'], + value: 'normal', + }, + ], + ) + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: server text') + }) + expect(getFirstChildKey(editor)).toEqual('server-key') + + editor.send({type: 'update readOnly', readOnly: false}) + + // The held repair was dropped as superseded: nothing ever pushes its + // stale key back over the store's own. + await new Promise((resolve) => setTimeout(resolve, 800)) + + expect(store.pushPatches).not.toHaveBeenCalled() + expect(store.pushValue).not.toHaveBeenCalled() + const [storeBlock] = store.getValue() + expect( + (storeBlock as {children: Array<{_key?: string; text: string}>}) + .children[0], + ).toEqual({ + _type: 'span', + _key: 'server-key', + text: 'server text', + marks: [], + }) + expect(getEditorText(editor)).toEqual('B: server text') + expect(getFirstChildKey(editor)).toEqual('server-key') + }) + + test('a whole-value store update supersedes a held repair; no push ever carries the pre-update text', async () => { + const store = createMockValueStore([ + { + _type: 'block', + _key: 'b1', + children: [{_type: 'span', text: 'Hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ]) + const {editor, unmount} = await createSyncedEditor({ + store, + readOnly: true, + }) + cleanup = unmount + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: Hello') + }) + expect(getFirstChildKey(editor)).toEqual('k2') + + // The concurrent client's own key and text for the same child, + // delivered as a whole-value store update (this host has no patch + // channel at all, so `applySync`'s diff-based repair is the only + // patches path). + store.setRemoteValue([ + { + _type: 'block', + _key: 'b1', + children: [ + {_type: 'span', _key: 'server-key', text: 'server text', marks: []}, + ], + markDefs: [], + style: 'normal', + }, + ]) + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: server text') + }) + expect(getFirstChildKey(editor)).toEqual('server-key') + + editor.send({type: 'update readOnly', readOnly: false}) + + // The held repair was dropped as superseded: it never gets the + // chance to push the pre-update text over the newer content. + await new Promise((resolve) => setTimeout(resolve, 800)) + + expect(store.pushValue).not.toHaveBeenCalled() + expect(getEditorText(editor)).toEqual('B: server text') + }) + + test('an echo of the same broken snapshot never supersedes the held repair, which still flushes exactly once', async () => { + const brokenValue = [ + { + _type: 'block', + _key: 'b1', + children: [{_type: 'span', text: 'Hello', marks: []}], + markDefs: [], + style: 'normal', + }, + ] + const store = createMockValueStore(brokenValue) + const {editor, unmount} = await createSyncedEditor({ + store, + readOnly: true, + }) + cleanup = unmount + + await vi.waitFor(() => { + expect(getEditorText(editor)).toEqual('B: Hello') + }) + const mintedKey = getFirstChildKey(editor) + expect(mintedKey).toEqual('k2') + + // The store re-sends the exact same (still broken) snapshot: no + // divergence from the held repair's baseline, so nothing about this + // echo should touch the held repair at all. + store.setRemoteValue(brokenValue) + + await new Promise((resolve) => setTimeout(resolve, 300)) + + expect(getEditorText(editor)).toEqual('B: Hello') + expect(getFirstChildKey(editor)).toEqual(mintedKey) + + editor.send({type: 'update readOnly', readOnly: false}) + + await vi.waitFor(() => { + expect(store.pushValue).toHaveBeenCalledTimes(1) + }) + // outlast another flush interval: still exactly once + await new Promise((resolve) => setTimeout(resolve, 800)) + expect(store.pushValue).toHaveBeenCalledTimes(1) + }) + }) + describe('echo suppression', () => { test('echo after local edit does not revert editor', async () => { const store = createMockValueStore()