From 5d1a02939d89dfc9005b2c6fdbd59062bbaa50d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Hamburger=20Gr=C3=B8ngaard?= Date: Tue, 10 Feb 2026 09:47:15 +0100 Subject: [PATCH] fix: use `set` instead of `setIfMissing` + `insert` when target value is empty There are reports from users who sometimes hit `Error: Attempt to apply insert patch to non-array value` when adding content immediately after removing the editor content (for example, by selecting all the content, hitting Delete and then pasting). The error comes from Studio and could be an indication that, for some reason, the patches for the editor content are targeting a field with value `null`: https://github.com/sanity-io/sanity/blob/d3374ad116fa176c51addf91183f291226e9ad8e/packages/%40sanity/mutator/src/patch/InsertPatch.ts#L23 `setIfMissing` treats `null` as "present", so when a field value is `null` (rather than `undefined` or `[]`), `setIfMissing([], [])` wouldn't do anything. And any subsequent insert patches would fail because there is no array to insert into. I'm sure *how* the field could become `null`, but a potential fix in PTE is to emit a single `set` patch when the editor goes from empty to non-empty rather than emitting a `setIfMissing` followed by an `insert`. This changes two code paths that emit patches during the empty-to-non-empty editor transition to use a single `set` patch instead of `setIfMissing` + `insert`: - `insertNodePatch` when inserting into an empty block array - The empty-to-non-empty guard in the patches plugin `set` is less merge-friendly than `insert` for concurrent edits, but the old code was already broken for `null` values, so this is a net improvement. --- .changeset/fix-insert-patch-bug.md | 11 ++++++ .../operation-to-patches.test.ts | 22 +++--------- .../internal-utils/operation-to-patches.ts | 13 ++++--- .../src/slate-plugins/slate-plugin.patches.ts | 9 +++-- packages/editor/tests/change-subject.test.tsx | 2 +- .../editor/tests/event.block.set.test.tsx | 25 ++++++------- .../editor/tests/event.block.unset.test.tsx | 20 +++++------ .../editor/tests/event.child.unset.test.tsx | 7 ++-- .../tests/event.delete.backward.test.tsx | 6 ++-- .../editor/tests/event.delete.block.test.tsx | 6 ++-- .../tests/event.delete.forward.test.tsx | 6 ++-- .../editor/tests/event.insert.block.test.tsx | 14 ++++---- packages/editor/tests/event.patch.test.tsx | 29 +++++++-------- packages/editor/tests/event.patches.test.tsx | 27 ++++++-------- .../editor/tests/placeholder-block.test.tsx | 36 ++++++++----------- packages/editor/tests/self-solving.test.tsx | 14 ++++---- 16 files changed, 107 insertions(+), 140 deletions(-) create mode 100644 .changeset/fix-insert-patch-bug.md diff --git a/.changeset/fix-insert-patch-bug.md b/.changeset/fix-insert-patch-bug.md new file mode 100644 index 000000000..06e64f02a --- /dev/null +++ b/.changeset/fix-insert-patch-bug.md @@ -0,0 +1,11 @@ +--- +'@portabletext/editor': patch +--- + +fix: use `set` instead of `setIfMissing` + `insert` when target value is empty + +When a remote Portable Text field had a `null` value (as opposed to `undefined` +or `[]`), typing or pasting into the editor would produce patches that failed to +apply on the receiving end. This is because `setIfMissing` treats `null` as +"present", so the subsequent `insert` would attempt to insert into `null` rather +than an array. diff --git a/packages/editor/src/internal-utils/operation-to-patches.test.ts b/packages/editor/src/internal-utils/operation-to-patches.test.ts index 6914d3ade..9cc37950f 100644 --- a/packages/editor/src/internal-utils/operation-to-patches.test.ts +++ b/packages/editor/src/internal-utils/operation-to-patches.test.ts @@ -126,19 +126,13 @@ describe(insertNodePatch.name, () => { ).toEqual([ { path: [], - type: 'setIfMissing', - value: [], - }, - { - path: [0], - type: 'insert', - items: [ + type: 'set', + value: [ { _key: 'k2', _type: 'image', }, ], - position: 'before', }, ]) }) @@ -277,21 +271,13 @@ describe('operationToPatches', () => { [ { "path": [], - "type": "setIfMissing", - "value": [], - }, - { - "items": [ + "type": "set", + "value": [ { "_key": "c130395c640c", "_type": "someObject", }, ], - "path": [ - 0, - ], - "position": "before", - "type": "insert", }, ] `) diff --git a/packages/editor/src/internal-utils/operation-to-patches.ts b/packages/editor/src/internal-utils/operation-to-patches.ts index e1ae248d8..e40a0acc1 100644 --- a/packages/editor/src/internal-utils/operation-to-patches.ts +++ b/packages/editor/src/internal-utils/operation-to-patches.ts @@ -280,10 +280,12 @@ export function insertNodePatch( beforeValue: Array, ): Array { const block = beforeValue[operation.path[0]!] + if (operation.path.length === 1) { const position = operation.path[0] === 0 ? 'before' : 'after' const beforeBlock = beforeValue[operation.path[0]! - 1] const targetKey = operation.path[0] === 0 ? block?._key : beforeBlock?._key + if (targetKey) { return [ insert( @@ -293,12 +295,15 @@ export function insertNodePatch( ), ] } + + // When inserting into an empty array (no targetKey), use `set` instead of + // `setIfMissing` + `insert`. This handles the case where the field value is + // null (not just undefined or []), since `setIfMissing` treats null as + // "present". return [ - setIfMissing(beforeValue, []), - insert( + set( [fromSlateBlock(operation.node as Descendant, schema.block.name)], - 'before', - [operation.path[0]!], + [], ), ] } else if ( diff --git a/packages/editor/src/slate-plugins/slate-plugin.patches.ts b/packages/editor/src/slate-plugins/slate-plugin.patches.ts index c9d26522c..2f7c81c25 100644 --- a/packages/editor/src/slate-plugins/slate-plugin.patches.ts +++ b/packages/editor/src/slate-plugins/slate-plugin.patches.ts @@ -1,4 +1,4 @@ -import {insert, setIfMissing, unset, type Patch} from '@portabletext/patches' +import {set, setIfMissing, unset, type Patch} from '@portabletext/patches' import type {PortableTextBlock} from '@portabletext/schema' import {Editor, type Operation} from 'slate' import type {EditorActor} from '../editor/editor-machine' @@ -128,13 +128,16 @@ export function createPatchesPlugin({ return editor } - // If the editor was empty and now isn't, insert the placeholder into it. if ( editorWasEmpty && !editorIsEmpty && operation.type !== 'set_selection' ) { - patches.push(insert(previousValue, 'before', [0])) + // If the editor was empty and now isn't, set the value atomically. + // Using `set` instead of `insert` handles the case where the field is + // null (not just undefined or []), since `setIfMissing` treats null as + // "present". + patches.push(set(previousValue, [])) } switch (operation.type) { diff --git a/packages/editor/tests/change-subject.test.tsx b/packages/editor/tests/change-subject.test.tsx index 7f86e94ca..da6c38ddb 100644 --- a/packages/editor/tests/change-subject.test.tsx +++ b/packages/editor/tests/change-subject.test.tsx @@ -32,7 +32,7 @@ describe('change$', () => { }), expect.objectContaining({ type: 'patch', - patch: expect.objectContaining({type: 'insert'}), + patch: expect.objectContaining({type: 'set'}), }), expect.objectContaining({ type: 'patch', diff --git a/packages/editor/tests/event.block.set.test.tsx b/packages/editor/tests/event.block.set.test.tsx index ec327f1a0..fc2d664c8 100644 --- a/packages/editor/tests/event.block.set.test.tsx +++ b/packages/editor/tests/event.block.set.test.tsx @@ -51,7 +51,7 @@ describe('event.block.set', () => { ]) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -61,8 +61,7 @@ describe('event.block.set', () => { style: 'normal', }, ], - 'before', - [0], + [], ), insert( [ @@ -149,7 +148,7 @@ describe('event.block.set', () => { ]) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -159,8 +158,7 @@ describe('event.block.set', () => { style: 'normal', }, ], - 'before', - [0], + [], ), insert( [ @@ -411,7 +409,7 @@ describe('event.block.set', () => { ]) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -421,8 +419,7 @@ describe('event.block.set', () => { style: 'normal', }, ], - 'before', - [0], + [], ), insert( [ @@ -471,7 +468,7 @@ describe('event.block.set', () => { expect(patches.slice(7)).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: textBlockKey, @@ -488,8 +485,7 @@ describe('event.block.set', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('h1', [{_key: textBlockKey}, 'style']), ]) @@ -592,7 +588,7 @@ describe('event.block.set', () => { expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -602,8 +598,7 @@ describe('event.block.set', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k0'}, 'foo']), ]) diff --git a/packages/editor/tests/event.block.unset.test.tsx b/packages/editor/tests/event.block.unset.test.tsx index f8293f443..6bb44bb33 100644 --- a/packages/editor/tests/event.block.unset.test.tsx +++ b/packages/editor/tests/event.block.unset.test.tsx @@ -65,7 +65,7 @@ describe('event.block.unset', () => { expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -75,8 +75,7 @@ describe('event.block.unset', () => { style: 'normal', }, ], - 'before', - [0], + [], ), insert( [ @@ -137,7 +136,7 @@ describe('event.block.unset', () => { ]) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -147,8 +146,7 @@ describe('event.block.unset', () => { style: 'normal', }, ], - 'before', - [0], + [], ), insert( [ @@ -243,7 +241,7 @@ describe('event.block.unset', () => { expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -253,8 +251,7 @@ describe('event.block.unset', () => { style: 'normal', }, ], - 'before', - [0], + [], ), insert( [ @@ -390,7 +387,7 @@ describe('event.block.unset', () => { ]) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k0', @@ -400,8 +397,7 @@ describe('event.block.unset', () => { markDefs: [], }, ], - 'before', - [0], + [], ), insert( [ diff --git a/packages/editor/tests/event.child.unset.test.tsx b/packages/editor/tests/event.child.unset.test.tsx index 7829575b4..80a0a7b27 100644 --- a/packages/editor/tests/event.child.unset.test.tsx +++ b/packages/editor/tests/event.child.unset.test.tsx @@ -390,10 +390,9 @@ describe('event.child.unset', () => { }, { origin: 'local', - type: 'insert', - path: [0], - position: 'before', - items: [ + type: 'set', + path: [], + value: [ { _key: blockKey, _type: 'block', diff --git a/packages/editor/tests/event.delete.backward.test.tsx b/packages/editor/tests/event.delete.backward.test.tsx index 23c160b3b..b2953fa0f 100644 --- a/packages/editor/tests/event.delete.backward.test.tsx +++ b/packages/editor/tests/event.delete.backward.test.tsx @@ -1,6 +1,5 @@ import { applyAll, - insert, set, setIfMissing, unset, @@ -95,7 +94,7 @@ describe('event.delete.backward', () => { expect(foreignValue).toEqual(expectedValue) expect(patches.slice(1)).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -105,8 +104,7 @@ describe('event.delete.backward', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k3'}, 'foo']), ]) diff --git a/packages/editor/tests/event.delete.block.test.tsx b/packages/editor/tests/event.delete.block.test.tsx index 4816a60dd..4e5c6ebea 100644 --- a/packages/editor/tests/event.delete.block.test.tsx +++ b/packages/editor/tests/event.delete.block.test.tsx @@ -1,6 +1,5 @@ import { applyAll, - insert, set, setIfMissing, unset, @@ -87,7 +86,7 @@ describe('event.delete.block', () => { expect(foreignValue).toEqual(expectedValue) expect(patches.slice(1)).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -97,8 +96,7 @@ describe('event.delete.block', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k3'}, 'foo']), ]) diff --git a/packages/editor/tests/event.delete.forward.test.tsx b/packages/editor/tests/event.delete.forward.test.tsx index 7b28f7d67..859e269fa 100644 --- a/packages/editor/tests/event.delete.forward.test.tsx +++ b/packages/editor/tests/event.delete.forward.test.tsx @@ -1,6 +1,5 @@ import { applyAll, - insert, set, setIfMissing, unset, @@ -91,7 +90,7 @@ describe('event.delete.forward', () => { expect(foreignValue).toEqual(expectedValue) expect(patches.slice(1)).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -101,8 +100,7 @@ describe('event.delete.forward', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k3'}, 'foo']), ]) diff --git a/packages/editor/tests/event.insert.block.test.tsx b/packages/editor/tests/event.insert.block.test.tsx index dc9d632f3..cb7843a37 100644 --- a/packages/editor/tests/event.insert.block.test.tsx +++ b/packages/editor/tests/event.insert.block.test.tsx @@ -457,10 +457,9 @@ describe('event.insert.block', () => { }, { origin: 'local', - path: [0], - type: 'insert', - position: 'before', - items: [ + path: [], + type: 'set', + value: [ { _type: 'block', _key: 'k0', @@ -554,10 +553,9 @@ describe('event.insert.block', () => { }, { origin: 'local', - path: [0], - type: 'insert', - position: 'before', - items: [ + path: [], + type: 'set', + value: [ { _type: 'block', _key: 'k0', diff --git a/packages/editor/tests/event.patch.test.tsx b/packages/editor/tests/event.patch.test.tsx index 70d78b8b4..8a10afb5b 100644 --- a/packages/editor/tests/event.patch.test.tsx +++ b/packages/editor/tests/event.patch.test.tsx @@ -148,7 +148,7 @@ describe('event.patch', () => { // Initial setting up patch setIfMissing([], []), // Inserting placeholder block - insert( + set( [ { _type: 'block', @@ -158,8 +158,7 @@ describe('event.patch', () => { style: 'normal', }, ], - 'before', - [0], + [], ), // Inserting new empty paragraph before placeholder insert([emptyParagraph], 'before', [{_key: 'k0'}]), @@ -171,7 +170,7 @@ describe('event.patch', () => { // Initial setting up patch setIfMissing([], []), // Inserting the empty paragraph which can now be considered the placeholder - insert([emptyParagraph], 'before', [0]), + set([emptyParagraph], []), // Inserting the h1 insert([h1], 'after', [{_key: emptyParagraph._key}]), ].map((patch) => ({...patch, origin: 'local'})), @@ -217,10 +216,9 @@ describe('event.patch', () => { }, { origin: 'local', - path: [0], - type: 'insert', - position: 'before', - items: [ + path: [], + type: 'set', + value: [ { _key: 'k0', _type: 'block', @@ -292,7 +290,7 @@ describe('event.patch', () => { expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -302,8 +300,7 @@ describe('event.patch', () => { style: 'normal', }, ], - 'before', - [0], + [], ), diffMatchPatch('', 'f', [ {_key: 'k0'}, @@ -354,7 +351,7 @@ describe('event.patch', () => { expect(patches.slice(5)).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -364,8 +361,7 @@ describe('event.patch', () => { style: 'normal', }, ], - 'before', - [0], + [], ), diffMatchPatch('', 'f', [ {_key: 'k0'}, @@ -422,7 +418,7 @@ describe('event.patch', () => { expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -432,8 +428,7 @@ describe('event.patch', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k0'}, 'foo']), diffMatchPatch('', 'f', [ diff --git a/packages/editor/tests/event.patches.test.tsx b/packages/editor/tests/event.patches.test.tsx index edd9e8d4f..2258fc4d8 100644 --- a/packages/editor/tests/event.patches.test.tsx +++ b/packages/editor/tests/event.patches.test.tsx @@ -1,6 +1,5 @@ import { applyAll, - insert, set, setIfMissing, unset, @@ -34,11 +33,10 @@ describe('event.patches', () => { expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', patch: { - type: 'insert', + type: 'set', origin: 'local', - path: [0], - position: 'before', - items: [ + path: [], + value: [ { _type: 'block', _key: 'ea-k0', @@ -101,11 +99,10 @@ describe('event.patches', () => { expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', patch: { - type: 'insert', + type: 'set', origin: 'local', - path: [0], - position: 'before', - items: [ + path: [], + value: [ { _type: 'block', _key: 'ea-k0', @@ -191,11 +188,10 @@ describe('event.patches', () => { expect(onEditorEvent).toHaveBeenCalledWith({ type: 'patch', patch: { - type: 'insert', + type: 'set', origin: 'local', - path: [0], - position: 'before', - items: [ + path: [], + value: [ { _type: 'block', _key: 'ea-k0', @@ -743,7 +739,7 @@ describe('event.patches', () => { expect(foreignValue).toEqual(expectedValue) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _key: 'k2', @@ -753,8 +749,7 @@ describe('event.patches', () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k2'}, 'foo']), ]) diff --git a/packages/editor/tests/placeholder-block.test.tsx b/packages/editor/tests/placeholder-block.test.tsx index 003774a6f..3562bfdc7 100644 --- a/packages/editor/tests/placeholder-block.test.tsx +++ b/packages/editor/tests/placeholder-block.test.tsx @@ -1,7 +1,6 @@ import { applyAll, diffMatchPatch, - insert, set, setIfMissing, unset, @@ -402,7 +401,7 @@ describe(createPlaceholderBlock.name, () => { expect(foreignValue).toEqual(expectedValue) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -412,8 +411,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), diffMatchPatch('', 'f', [ {_key: 'k3'}, @@ -492,7 +490,7 @@ describe(createPlaceholderBlock.name, () => { expect(foreignValue).toEqual(expectedValue) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -502,8 +500,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), diffMatchPatch('', 'f', [ {_key: 'k3'}, @@ -547,7 +544,7 @@ describe(createPlaceholderBlock.name, () => { // The editor is set up setIfMissing([], []), // A placeholder block is inserted - insert( + set( [ { _type: 'block', @@ -557,8 +554,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), // The text is added diffMatchPatch('', 'f', [ @@ -611,7 +607,7 @@ describe(createPlaceholderBlock.name, () => { expect(patches.slice(5)).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -621,8 +617,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), diffMatchPatch('', 'f', [ {_key: 'k0'}, @@ -673,7 +668,7 @@ describe(createPlaceholderBlock.name, () => { expect(foreignValue).toEqual(expectedValue) expect(patches).toEqual([ setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -683,8 +678,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), diffMatchPatch('', 'f', [ {_key: 'k0'}, @@ -728,7 +722,7 @@ describe(createPlaceholderBlock.name, () => { ]), unset([]), setIfMissing([], []), - insert( + set( [ { _type: 'block', @@ -738,8 +732,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), set('bar', [{_key: 'k0'}, 'foo']), ]) @@ -822,7 +815,7 @@ describe(createPlaceholderBlock.name, () => { // The editor is reset setIfMissing([], []), // A placeholder block is inserted - insert( + set( [ { _type: 'block', @@ -832,8 +825,7 @@ describe(createPlaceholderBlock.name, () => { style: 'normal', }, ], - 'before', - [0], + [], ), // The text is added diffMatchPatch('', 'f', [ diff --git a/packages/editor/tests/self-solving.test.tsx b/packages/editor/tests/self-solving.test.tsx index 3938589b5..3d11f7886 100644 --- a/packages/editor/tests/self-solving.test.tsx +++ b/packages/editor/tests/self-solving.test.tsx @@ -213,10 +213,9 @@ describe('Feature: Self-solving', () => { }, { origin: 'local', - type: 'insert', - path: [0], - position: 'before', - items: [ + type: 'set', + path: [], + value: [ { ...initialValue[0], style: 'normal', @@ -323,10 +322,9 @@ describe('Feature: Self-solving', () => { }, { origin: 'local', - type: 'insert', - path: [0], - position: 'before', - items: [ + type: 'set', + path: [], + value: [ { _key: 'k0', _type: 'block',