From 2dad21d1d75750e7148b10bdea3ce921a79cbf33 Mon Sep 17 00:00:00 2001 From: Andrew Herron Date: Wed, 14 Apr 2021 01:06:07 +1000 Subject: [PATCH] set_node operations did not invert correctly after serialization (#4078) * Use null instead of undefined as the old value for set_node operations that add a property * Changelog * Omit undefined values from `set_node` operations completely. Delete missing property values in `Transforms.apply()` instead. * Add tests for removing set_node properties with null and undefined --- .changeset/silent-bees-sing.md | 5 ++++ packages/slate/src/transforms/general.ts | 9 +++++- packages/slate/src/transforms/node.ts | 3 +- .../test/operations/set_node/remove-null.tsx | 28 +++++++++++++++++++ .../test/operations/set_node/remove-omit.tsx | 28 +++++++++++++++++++ .../operations/set_node/remove-undefined.tsx | 28 +++++++++++++++++++ .../basic-structure/can-be-serialized.tsx | 25 +++++++++++++++++ .../invert-after-serialization.tsx | 25 +++++++++++++++++ 8 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 .changeset/silent-bees-sing.md create mode 100644 packages/slate/test/operations/set_node/remove-null.tsx create mode 100644 packages/slate/test/operations/set_node/remove-omit.tsx create mode 100644 packages/slate/test/operations/set_node/remove-undefined.tsx create mode 100644 packages/slate/test/transforms/setNodes/basic-structure/can-be-serialized.tsx create mode 100644 packages/slate/test/transforms/setNodes/basic-structure/invert-after-serialization.tsx diff --git a/.changeset/silent-bees-sing.md b/.changeset/silent-bees-sing.md new file mode 100644 index 000000000..10d5602e7 --- /dev/null +++ b/.changeset/silent-bees-sing.md @@ -0,0 +1,5 @@ +--- +'slate': patch +--- + +set_node operations did not invert correctly after serialization diff --git a/packages/slate/src/transforms/general.ts b/packages/slate/src/transforms/general.ts index a3d17a4ec..49fd0757a 100644 --- a/packages/slate/src/transforms/general.ts +++ b/packages/slate/src/transforms/general.ts @@ -182,7 +182,7 @@ export const GeneralTransforms: GeneralTransforms = { } case 'set_node': { - const { path, newProperties } = op + const { path, properties, newProperties } = op if (path.length === 0) { throw new Error(`Cannot set properties on the root node!`) @@ -204,6 +204,13 @@ export const GeneralTransforms: GeneralTransforms = { } } + // properties that were previously defined, but are now missing, must be deleted + for (const key in properties) { + if (!newProperties.hasOwnProperty(key)) { + delete node[key] + } + } + break } diff --git a/packages/slate/src/transforms/node.ts b/packages/slate/src/transforms/node.ts index 4fac582a8..2a4d2d061 100644 --- a/packages/slate/src/transforms/node.ts +++ b/packages/slate/src/transforms/node.ts @@ -631,7 +631,8 @@ export const NodeTransforms: NodeTransforms = { } if (props[k] !== node[k]) { - properties[k] = node[k] + // Omit new properties from the old property list rather than set them to undefined + if (node.hasOwnProperty(k)) properties[k] = node[k] newProperties[k] = props[k] } } diff --git a/packages/slate/test/operations/set_node/remove-null.tsx b/packages/slate/test/operations/set_node/remove-null.tsx new file mode 100644 index 000000000..918d65809 --- /dev/null +++ b/packages/slate/test/operations/set_node/remove-null.tsx @@ -0,0 +1,28 @@ +/** @jsx jsx */ +import { jsx } from 'slate-hyperscript' +import { Transforms, Editor } from 'slate' + +export const input = ( + + + + + +) + +export const operations = [ + { + type: 'set_node', + path: [0, 0], + properties: { key: true }, + newProperties: { key: null }, + }, +] + +export const output = ( + + + + + +) diff --git a/packages/slate/test/operations/set_node/remove-omit.tsx b/packages/slate/test/operations/set_node/remove-omit.tsx new file mode 100644 index 000000000..615c5c89a --- /dev/null +++ b/packages/slate/test/operations/set_node/remove-omit.tsx @@ -0,0 +1,28 @@ +/** @jsx jsx */ +import { jsx } from 'slate-hyperscript' +import { Transforms, Editor } from 'slate' + +export const input = ( + + + + + +) + +export const operations = [ + { + type: 'set_node', + path: [0, 0], + properties: { key: true }, + newProperties: {}, + }, +] + +export const output = ( + + + + + +) diff --git a/packages/slate/test/operations/set_node/remove-undefined.tsx b/packages/slate/test/operations/set_node/remove-undefined.tsx new file mode 100644 index 000000000..0c8cc7d96 --- /dev/null +++ b/packages/slate/test/operations/set_node/remove-undefined.tsx @@ -0,0 +1,28 @@ +/** @jsx jsx */ +import { jsx } from 'slate-hyperscript' +import { Transforms, Editor } from 'slate' + +export const input = ( + + + + + +) + +export const operations = [ + { + type: 'set_node', + path: [0, 0], + properties: { key: true }, + newProperties: { key: undefined }, + }, +] + +export const output = ( + + + + + +) diff --git a/packages/slate/test/transforms/setNodes/basic-structure/can-be-serialized.tsx b/packages/slate/test/transforms/setNodes/basic-structure/can-be-serialized.tsx new file mode 100644 index 000000000..7e9cc31bd --- /dev/null +++ b/packages/slate/test/transforms/setNodes/basic-structure/can-be-serialized.tsx @@ -0,0 +1,25 @@ +/** @jsx jsx */ +import assert from 'assert' +import { Editor, Transforms, Operation } from 'slate' +import { jsx } from '../../..' + +export const run = (editor: Editor) => { + Transforms.setNodes(editor, { key: true }, { at: [0] }) + const [op] = editor.operations + const roundTrip: Operation = JSON.parse(JSON.stringify(op)) + assert.deepStrictEqual(op, roundTrip) +} +export const input = ( + + + + + +) +export const output = ( + + + + + +) diff --git a/packages/slate/test/transforms/setNodes/basic-structure/invert-after-serialization.tsx b/packages/slate/test/transforms/setNodes/basic-structure/invert-after-serialization.tsx new file mode 100644 index 000000000..4e7218194 --- /dev/null +++ b/packages/slate/test/transforms/setNodes/basic-structure/invert-after-serialization.tsx @@ -0,0 +1,25 @@ +/** @jsx jsx */ +import { Editor, Transforms, Operation } from 'slate' +import { jsx } from '../../..' + +export const run = (editor: Editor) => { + Transforms.setNodes(editor, { key: true }, { at: [0] }) + const [op] = editor.operations + const roundTrip: Operation = JSON.parse(JSON.stringify(op)) + const inverted = Operation.inverse(roundTrip) + editor.apply(inverted) +} +export const input = ( + + + + + +) +export const output = ( + + + + + +)