Remove fast-deep-equal (#4276)

* Remove fast-deep-equal in favour of a custom slate-specific equality check that only supports nested objects

* Add array comparison to deep equal. Fix bug with multiple nested objects.

.changeset/polite-readers-talk.md

@@ -0,0 +1,5 @@
+---
+'slate': minor
+---
+
+Switched from `fast-deep-equal` to a custom deep equality check. This restores the ability for text nodes with mark values set to `undefined` to merge with text nodes missing those keys.

packages/slate/package.json

@@ -16,7 +16,6 @@
   "dependencies": {
     "@types/esrever": "^0.2.0",
     "esrever": "^0.2.0",
-    "fast-deep-equal": "^3.1.3",
     "immer": "^8.0.1",
     "is-plain-object": "^3.0.0",
     "tiny-warning": "^1.0.3"

packages/slate/src/interfaces/text.ts

@@ -1,7 +1,7 @@
 import isPlainObject from 'is-plain-object'
-import isEqual from 'fast-deep-equal'
 import { Range } from '..'
 import { ExtendedType } from './custom-types'
+import { isDeepEqual } from '../utils/deep-equal'
 
 /**
  * `Text` objects represent the nodes that contain the actual text content of a
@@ -27,8 +27,10 @@ export interface TextInterface {
 export const Text: TextInterface = {
   /**
    * Check if two text nodes are equal.
+   *
+   * When loose is set, the text is not compared. This is
+   * used to check whether sibling text nodes can be merged.
    */
-
   equals(
     text: Text,
     another: Text,
@@ -42,7 +44,7 @@ export const Text: TextInterface = {
       return rest
     }
 
-    return isEqual(
+    return isDeepEqual(
       loose ? omitText(text) : text,
       loose ? omitText(another) : another
     )

packages/slate/src/utils/deep-equal.ts

@@ -0,0 +1,46 @@
+import isPlainObject from 'is-plain-object'
+
+/*
+  Custom deep equal comparison for Slate nodes.
+
+  We don't need general purpose deep equality;
+  Slate only supports plain values, Arrays, and nested objects.
+  Complex values nested inside Arrays are not supported.
+
+  Slate objects are designed to be serialised, so
+  missing keys are deliberately normalised to undefined.
+ */
+export const isDeepEqual = (
+  node: Record<string, any>,
+  another: Record<string, any>
+): boolean => {
+  for (const key in node) {
+    const a = node[key]
+    const b = another[key]
+    if (isPlainObject(a)) {
+      if (!isDeepEqual(a, b)) return false
+    } else if (Array.isArray(a) && Array.isArray(b)) {
+      if (a.length !== b.length) return false
+      for (let i = 0; i < a.length; i++) {
+        if (a[i] !== b[i]) return false
+      }
+      return true
+    } else if (a !== b) {
+      return false
+    }
+  }
+
+  /*
+    Deep object equality is only necessary in one direction; in the reverse direction
+    we are only looking for keys that are missing.
+    As above, undefined keys are normalised to missing.
+  */
+
+  for (const key in another) {
+    if (node[key] === undefined && another[key] !== undefined) {
+      return false
+    }
+  }
+
+  return true
+}

packages/slate/test/index.js

@@ -37,6 +37,14 @@ describe('slate', () => {
     assert.deepEqual(editor.children, output.children)
     assert.deepEqual(editor.selection, output.selection)
   })
+  fixtures(__dirname, 'utils', ({ module }) => {
+    let { input, test, output } = module
+    if (Editor.isEditor(input)) {
+      input = withTest(input)
+    }
+    const result = test(input)
+    assert.deepEqual(result, output)
+  })
 })
 const withTest = editor => {
   const { isInline, isVoid } = editor

packages/slate/test/utils/deep-equal/deep-equals.js

@@ -0,0 +1,20 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: {
+    text: 'same text',
+    bold: true,
+    italic: { origin: 'inherited', value: false },
+  },
+  objectB: {
+    text: 'same text',
+    bold: true,
+    italic: { origin: 'inherited', value: false },
+  },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = true

packages/slate/test/utils/deep-equal/deep-not-equal-multiple-objects.js

@@ -0,0 +1,22 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: {
+    text: 'same text',
+    bold: true,
+    italic: { origin: 'inherited', value: true },
+    underline: { origin: 'inherited', value: false },
+  },
+  objectB: {
+    text: 'same text',
+    bold: true,
+    italic: { origin: 'inherited', value: true },
+    underline: { origin: 'inherited', value: true },
+  },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = false

packages/slate/test/utils/deep-equal/deep-not-equal.js

@@ -0,0 +1,20 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: {
+    text: 'same text',
+    bold: true,
+    italic: { origin: 'inherited', value: false },
+  },
+  objectB: {
+    text: 'same text',
+    bold: true,
+    italic: { origin: 'inherited', value: true },
+  },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = false

packages/slate/test/utils/deep-equal/simple-equals.js

@@ -0,0 +1,12 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: { text: 'same text', bold: true },
+  objectB: { text: 'same text', bold: true },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = true

packages/slate/test/utils/deep-equal/simple-not-equal.js

@@ -0,0 +1,12 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: { text: 'same text', bold: true },
+  objectB: { text: 'same text', bold: true, italic: true },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = false

packages/slate/test/utils/deep-equal/undefined-key-equal-backward.js

@@ -0,0 +1,17 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: {
+    text: 'same text',
+  },
+  objectB: {
+    text: 'same text',
+    bold: undefined,
+  },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = true

packages/slate/test/utils/deep-equal/undefined-key-equal-forward.js

@@ -0,0 +1,17 @@
+import { isDeepEqual } from '../../../src/utils/deep-equal'
+
+export const input = {
+  objectA: {
+    text: 'same text',
+    bold: undefined,
+  },
+  objectB: {
+    text: 'same text',
+  },
+}
+
+export const test = ({ objectA, objectB }) => {
+  return isDeepEqual(objectA, objectB)
+}
+
+export const output = true

yarn.lock

@@ -5725,7 +5725,7 @@ faker@^4.1.0:
   resolved "https://registry.yarnpkg.com/faker/-/faker-4.1.0.tgz#1e45bbbecc6774b3c195fad2835109c6d748cc3f"
   integrity sha1-HkW7vsxndLPBlfrSg1EJxtdIzD8=
 
-fast-deep-equal@^3.1.1, fast-deep-equal@^3.1.3:
+fast-deep-equal@^3.1.1:
   version "3.1.3"
   resolved "https://registry.yarnpkg.com/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz#3a7d56b559d6cbc3eb512325244e619a65c6c525"
   integrity sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q==