Repository: vigetlabs/microcosm
Pull request #505 (2018-04-17 11:16:39)

[microcosm] Add remove data helper. Setting undefined removes props
This commit updates the way `set` works such that it removes a property if it is undefined. It also adds an `remove` function, which is an alias for `set(object, prop, undefined)`.

```javascript
import { set, remove } from 'microcosm'

val records = { 1: { name: 'Billy' }, 2: { name: 'Sal' } }

// These are the same
set(records, 1, undefined) // { 2: { name: 'Sal' } }
remove(records, 1)         // { 2: { name: 'Sal' } }
```

I also took this as an opportunity to rework flow types and refactor some of the data operation code to avoid null checks.

codecov-io: # [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=h1) Report
> Merging [#505](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=desc) into [master](https://codecov.io/gh/vigetlabs/microcosm/commit/d7a8a962289621d2a25c6a2aed880996ce48f2f0?src=pr&el=desc) will **decrease** coverage by `0.29%`.
> The diff coverage is `93.33%`.

[![Impacted file tree graph](https://codecov.io/gh/vigetlabs/microcosm/pull/505/graphs/tree.svg?src=pr&token=AaE0FLz9sU&width=650&height=150)](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##           master     #505     +/-   ##
=========================================
- Coverage   99.69%   99.39%   -0.3%     
=========================================
  Files          27       27             
  Lines         658      664      +6     
  Branches      129      130      +1     
=========================================
+ Hits          656      660      +4     
- Misses          2        4      +2
```


| [Impacted Files](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [packages/microcosm/src/data.js](https://codecov.io/gh/vigetlabs/microcosm/pull/505/diff?src=pr&el=tree#diff-cGFja2FnZXMvbWljcm9jb3NtL3NyYy9kYXRhLmpz) | `97.01% <93.33%> (-2.99%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=footer). Last update [d7a8a96...d98a06f](https://codecov.io/gh/vigetlabs/microcosm/pull/505?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/microcosm/src/data.js b/packages/microcosm/src/data.js
index b6e66b8c..35101e1b 100644
--- a/packages/microcosm/src/data.js
+++ b/packages/microcosm/src/data.js
@@ -1,11 +1,11 @@
 // @flow
 
-import { EMPTY_ARRAY } from './empty'
+import { EMPTY_OBJECT, EMPTY_ARRAY } from './empty'
 
-type KeyPath = Array<string>
-type MixedObject = { [key: string]: mixed }
+type Path = string | Array<string>
+type NormalizedPath = Array<string>
 
-function castPath(value: string | KeyPath): KeyPath {
+function normalizePath(value: Path): NormalizedPath {
   if (Array.isArray(value)) {
     return value
   }
@@ -20,38 +20,36 @@ function castPath(value: string | KeyPath): KeyPath {
     .split('.')
 }
 
-function isObject(value: *) {
+function isObject(value: *): boolean {
   return value !== null && typeof value === 'object'
 }
 
 /**
  * A helper combination of get and set
  */
-export function update(
-  state: *,
-  keyPath: string | KeyPath,
-  updater: *,
-  fallback?: *
-) {
-  let path = castPath(keyPath)
-
+export function update(state: *, path: Path, updater: *, fallback?: *) {
   if (typeof updater !== 'function') {
     return set(state, path, updater)
   }
 
+  path = normalizePath(path)
+
   let last = get(state, path, fallback)
-  let next = updater(last)
 
-  return set(state, path, next)
+  return set(state, path, updater(last))
 }
 
 /**
  * Non-destructively assign a value to a provided object at a given key. If the
  * value is the same, don't do anything. Otherwise return a new object.
  */
-export function set(object: Object, key: *, value: *): any {
-  // Ensure we're working with a key path, like: ['a', 'b', 'c']
-  let path = castPath(key)
+export function set(object: Object, path: Path, value: *): * {
+  console.assert(
+    path != null,
+    `Expected path to be defined. Instead got ${String(path)}`
+  )
+
+  path = normalizePath(path)
 
   let len = path.length
 
@@ -83,21 +81,35 @@ export function set(object: Object, key: *, value: *): any {
       }
     }
 
-    // Assign the value, then continue on to the next iteration of the loop
-    // using the next step down
-    node[key] = next
-    node = node[key]
+    // If the value is defined
+    if (next !== undefined) {
+      // Assign the value, then continue on to the next iteration of the loop
+      // using the next step down
+      node[key] = next
+      node = node[key]
+    } else {
+      // Otherwise clear the value from the object
+      delete node[key]
+    }
   }
 
   return root
 }
 
+/**
+ * Immutably remove a key from an object.
+ */
+export function remove(target: *, path: Path): * {
+  return set(target, path, undefined)
+}
+
 /**
  * Retrieve a value from an object. If no key is provided, just return
  * the object.
  */
-export function get(object: ?Object, keyPath: *, fallback?: *) {
-  let path = castPath(keyPath)
+export function get(object: ?Object, path: Path, fallback?: *) {
+  path = normalizePath(path)
+
   let value = object
 
   for (var i = 0, len = path.length; i < len; i++) {
@@ -117,25 +129,24 @@ export function get(object: ?Object, keyPath: *, fallback?: *) {
 
 /**
  * Merge any number of objects into a provided object.
+ * $FlowFixMe This function enumerates on variable arguments
  */
-export function merge(...args: Array<?Object>): Object {
-  let copy = null
-  let subject = null
-
-  for (var i = 0, len = args.length; i < len; i++) {
-    if (isObject(args[i]) === false) {
-      continue
-    }
+export function merge(): Object {
+  let cloned = false
+  let copy = EMPTY_OBJECT
 
-    copy = copy || args[i] || {}
+  for (var i = 0, len = arguments.length; i < len; i++) {
+    var next = isObject(arguments[i]) ? arguments[i] : null
 
-    subject = subject || copy
+    if (copy === EMPTY_OBJECT) {
+      copy = next || EMPTY_OBJECT
+    }
 
-    var next = args[i]
     for (var key in next) {
       if (copy[key] !== next[key]) {
-        if (copy === subject) {
-          copy = clone(subject)
+        if (cloned === false) {
+          copy = clone(copy)
+          cloned = true
         }
 
         copy[key] = next[key]
@@ -143,23 +154,23 @@ export function merge(...args: Array<?Object>): Object {
     }
   }
 
-  return copy || {}
+  return copy == EMPTY_OBJECT ? {} : copy
 }
 
 /**
  * Shallow copy an object
  */
-export function clone<T: MixedObject>(target: T): $Shape<T> {
+export function clone<T: Object>(target: T): $Shape<T> {
   if (Array.isArray(target)) {
     return target.slice(0)
-  } else if (isObject(target) === false) {
-    return {}
   }
 
   let copy = {}
 
-  for (let key in target) {
-    copy[key] = target[key]
+  if (isObject(target)) {
+    for (var key in target) {
+      copy[key] = target[key]
+    }
   }
 
   return copy
diff --git a/packages/microcosm/src/index.js b/packages/microcosm/src/index.js
index fe5d83ab..c170328f 100644
--- a/packages/microcosm/src/index.js
+++ b/packages/microcosm/src/index.js
@@ -8,7 +8,7 @@ export { Subject } from './subject'
 export { RESET as reset, PATCH as patch } from './lifecycle'
 
 // Data helpers, useful in domain handlers for immutable updates
-export { get, set, merge, update } from './data'
+export { get, set, remove, update, merge } from './data'
 
 export { tag } from './tag'
 


Test patch:
diff --git a/packages/microcosm/test/unit/data/merge.test.js b/packages/microcosm/test/unit/data/merge.test.js
index cd96f180..8ae26a07 100644
--- a/packages/microcosm/test/unit/data/merge.test.js
+++ b/packages/microcosm/test/unit/data/merge.test.js
@@ -26,6 +26,11 @@ describe('Utils.merge', function() {
     })
   })
 
+  it('returns an extensible object - not the EMPTY_OBJECT constant', function() {
+    let answer = merge()
+    expect(Object.isExtensible(answer)).toBe(true)
+  })
+
   it('returns a new copy when the left most value is an empty object', function() {
     const a = {}
     const b = { foo: 'bar' }
diff --git a/packages/microcosm/test/unit/data/remove.test.js b/packages/microcosm/test/unit/data/remove.test.js
new file mode 100644
index 00000000..4ebc6c90
--- /dev/null
+++ b/packages/microcosm/test/unit/data/remove.test.js
@@ -0,0 +1,31 @@
+import { remove } from 'microcosm'
+
+describe('Utils.remove', function() {
+  const subject = {
+    styles: {
+      color: 'blue',
+      font: 'Helvetica, sans-serif'
+    }
+  }
+
+  it('removes an existing key', function() {
+    let next = remove(subject, 'styles.color')
+
+    expect(next).not.toHaveProperty('styles.color')
+    expect(next).not.toBe(subject)
+    expect(next.styles).not.toBe(subject.styles)
+  })
+
+  it('copies the object on removal', function() {
+    let next = remove(subject, 'styles.color')
+
+    expect(next).not.toBe(subject)
+    expect(next.styles).not.toBe(subject.styles)
+  })
+
+  it('does not copy if removing a non-existant key', function() {
+    let next = remove(subject, 'something-else')
+
+    expect(next).toBe(subject)
+  })
+})
diff --git a/packages/microcosm/test/unit/data/set.test.js b/packages/microcosm/test/unit/data/set.test.js
index 905f5dc8..22fa47d6 100644
--- a/packages/microcosm/test/unit/data/set.test.js
+++ b/packages/microcosm/test/unit/data/set.test.js
@@ -72,6 +72,31 @@ describe('Utils.set', function() {
     expect(next.root).toEqual({ segment: true })
   })
 
+  it('removes undefined keys', function() {
+    let next = set({ prop: true }, 'prop', undefined)
+
+    expect(next).not.toHaveProperty('root')
+  })
+
+  it('removes deeply nested keys', function() {
+    let next = set({ a: { b: true } }, 'a.b', undefined)
+
+    expect(next).not.toHaveProperty('a.b')
+  })
+
+  it('respects other keys keys', function() {
+    let next = set({ a: { b: true, c: true } }, 'a.b', undefined)
+
+    expect(next).not.toHaveProperty('a.b')
+    expect(next).toHaveProperty('a.c')
+  })
+
+  it('does not create garbage when setting a value to undefined', function() {
+    let next = set({}, 'a.b.c', undefined)
+
+    expect(next).not.toHaveProperty('a')
+  })
+
   describe('arrays', function() {
     it('can operate on arrays', function() {
       let list = ['a', 'b', 'c']