fix selection operations being duplicated (#2023)

#### Is this adding or improving a _feature_ or fixing a _bug_?

Bug.

#### What's the new behavior?

Fixes selection operations from being duplicated.

#### How does this change work?

Previously the selection properties were compared by reference, but paths are immutable `List` objects, which always show up as having changed, resulting in extra selection operations that without any real changes. We now use `Immutable.is` to remove those duplicates, fixing the undo history stack.

#### Have you checked that...?

<!-- 
Please run through this checklist for your pull request: 
-->

* [x] The new code matches the existing patterns and styles.
* [x] The tests pass with `yarn test`.
* [x] The linter passes with `yarn lint`. (Fix errors with `yarn prettier`.)
* [x] The relevant examples still work. (Run examples with `yarn watch`.)

#### Does this fix any issues or need any specific reviewers?

Fixes: #2006

examples/app.js

@@ -102,14 +102,14 @@ const TabList = styled('div')`
   }
 `
 
-const Tab = styled(RouterLink)`
+const Tab = styled(({ active, ...props }) => <RouterLink {...props} />)`
   display: inline-block;
   margin-bottom: 0.2em;
   padding: 0.2em 0.5em;
   border-radius: 0.2em;
   text-decoration: none;
-  color: ${props => (props.active ? 'white' : '#777')};
-  background: ${props => (props.active ? '#333' : 'transparent')};
+  color: ${p => (p.active ? 'white' : '#777')};
+  background: ${p => (p.active ? '#333' : 'transparent')};
 
   &:hover {
     background: #333;

packages/slate-react/src/plugins/after.js

@@ -347,6 +347,7 @@ function AfterPlugin() {
     const entire = selection
       .moveAnchorTo(point.key, start)
       .moveFocusTo(point.key, end)
+      .normalize(document)
 
     // Change the current value to have the leaf's text replaced.
     change.insertTextAtRange(entire, textContent, leaf.marks).select(corrected)

packages/slate-react/src/utils/find-range.js

@@ -60,7 +60,7 @@ function findRange(native, value) {
     }
   }
 
-  const range = Range.create({
+  let range = Range.create({
     anchorKey: anchor.key,
     anchorOffset: anchor.offset,
     focusKey: focus.key,
@@ -69,6 +69,7 @@ function findRange(native, value) {
     isFocused: true,
   })
 
+  range = range.normalize(value.document)
   return range
 }
 

packages/slate/src/changes/on-selection.js

@@ -1,3 +1,4 @@
+import { is } from 'immutable'
 import isEmpty from 'is-empty'
 import pick from 'lodash/pick'
 
@@ -20,45 +21,48 @@ const Changes = {}
 
 Changes.select = (change, properties, options = {}) => {
   properties = Range.createProperties(properties)
-
   const { snapshot = false } = options
   const { value } = change
   const { document, selection } = value
   const props = {}
-  const sel = selection.toJSON()
   const next = selection.merge(properties).normalize(document)
+
+  // Re-compute the properties, to ensure that we get their normalized values.
   properties = pick(next, Object.keys(properties))
 
   // Remove any properties that are already equal to the current selection. And
   // create a dictionary of the previous values for all of the properties that
   // are being changed, for the inverse operation.
   for (const k in properties) {
-    if (snapshot == false && properties[k] == sel[k]) continue
+    if (snapshot === true || !is(properties[k], selection[k])) {
       props[k] = properties[k]
     }
+  }
 
   // If the selection moves, clear any marks, unless the new selection
   // properties change the marks in some way.
-  const moved = ['anchorKey', 'anchorOffset', 'focusKey', 'focusOffset'].some(
-    p => props.hasOwnProperty(p)
-  )
-
-  if (sel.marks && properties.marks == sel.marks && moved) {
+  if (
+    selection.marks &&
+    !props.marks &&
+    (props.hasOwnProperty('anchorKey') ||
+      props.hasOwnProperty('anchorOffset') ||
+      props.hasOwnProperty('focusKey') ||
+      props.hasOwnProperty('focusOffset'))
+  ) {
     props.marks = null
   }
 
-  // If there are no new properties to set, abort.
+  // If there are no new properties to set, abort to avoid extra operations.
   if (isEmpty(props)) {
     return
   }
 
-  // Apply the operation.
   change.applyOperation(
     {
       type: 'set_selection',
       value,
       properties: props,
-      selection: sel,
+      selection: selection.toJSON(),
     },
     snapshot ? { skip: false, merge: false } : {}
   )