Warn about duplicate keys (#2081)

* Fix duplicate key in insert node

* Add warning on duplicate keys

* A lot of keys are duplicated

* Ensure error logging

* Finally Fixed

* Change PR

* bug fix

* comment fix

* Add getKeysToPathsTable into text

* refactor forbidden interface

* Refactor forbidden function

* Linting

* Change warn message

* Fix doc

package.json

@@ -108,7 +108,7 @@
     "release": "yarn build:production && yarn test && yarn lint && lerna publish && yarn gh-pages",
     "server": "webpack-dev-server --config ./support/webpack/config.js",
     "start": "npm-run-all --parallel --print-label watch server",
-    "test": "cross-env BABEL_ENV=test FORBID_DEPRECATIONS=true mocha --require babel-core/register ./packages/*/test/index.js",
+    "test": "cross-env BABEL_ENV=test FORBID_DEPRECATIONS=true FORBID_WARNINGS=true mocha --require babel-core/register ./packages/*/test/index.js",
     "test:coverage": "cross-env NODE_ENV=test nyc yarn test",
     "watch": "rollup --config ./support/rollup/config.js --watch"
   }

packages/slate-dev-logger/src/index.js

@@ -7,6 +7,12 @@
 const FORBID_DEPRECATE =
   process && process.env && process.env.FORBID_DEPRECATIONS
 
+/**
+ * Is warning scenarios forbidden?
+ */
+
+const FORBID_WARNING = process && process.env && process.env.FORBID_WARNINGS
+
 /**
  * Is in development?
  *
@@ -69,7 +75,8 @@ function error(message, ...args) {
  */
 
 function warn(message, ...args) {
-  log('warn', `Warning: ${message}`, ...args)
+  const logger = FORBID_WARNING ? forbidden : log
+  logger('warn', `Warning: ${message}`, ...args)
 }
 
 /**
@@ -82,11 +89,12 @@ function warn(message, ...args) {
  */
 
 function deprecate(version, message, ...args) {
-  if (FORBID_DEPRECATE) {
+  const logger = FORBID_DEPRECATE ? forbidden : log
-    throw new Error(`Deprecation (${version}): ${message}`)
+  logger('warn', `Deprecation (${version}): ${message}`, ...args)
-  }
+}
 
-  log('warn', `Deprecation (${version}): ${message}`, ...args)
+function forbidden(level, message) {
+  throw new Error(message)
 }
 
 /**

packages/slate/src/models/node.js

@@ -905,15 +905,19 @@ class Node {
     }
 
     this.nodes.forEach((node, i) => {
-      ret[node.key] = [i]
+      const nested = node.getKeysToPathsTable()
 
-      if (node.object !== 'text') {
+      for (const key in nested) {
-        const nested = node.getKeysToPathsTable()
+        const path = nested[key]
 
-        for (const key in nested) {
+        if (ret[key]) {
-          const path = nested[key]
+          logger.warn(
-          ret[key] = [i, ...path]
+            `A node with a duplicate key of "${key}" was found! Duplicate keys are not allowed, you should use \`node.regenerateKey\` before inserting if you are reusing an existing node.`,
+            this
+          )
         }
+
+        ret[key] = [i, ...path]
       }
     })
 

packages/slate/src/models/text.js

@@ -143,6 +143,18 @@ class Text extends Record(DEFAULTS) {
     return List.isList(any) && any.every(item => Text.isText(item))
   }
 
+  /**
+   * Get an object mapping all the keys in the node to their paths.
+   *
+   * @return {Object}
+   */
+
+  getKeysToPathsTable() {
+    return {
+      [this.key]: [],
+    }
+  }
+
   /**
    * Object.
    *
@@ -806,6 +818,7 @@ memoize(Text.prototype, [
   'normalize',
   'validate',
   'getString',
+  'getKeysToPathsTable',
 ])
 
 /**

packages/slate/test/changes/by-key/insert-node-by-key/duplicate.js

@@ -27,3 +27,9 @@ export const output = (
     </document>
   </value>
 )
+
+/*
+ * Slate v0.37: We no longer support duplicate key check in either insertNode and replaceNode
+*/
+
+export const skip = true