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',