⚡ flow: Avoid multiple reinstanciations (#149)

* ♻ Apply: Add path property on applier, add curPath arg to walkPath

* 🚧 flow: Send appliedPaths to applier

* ♻ Inline flowReduceCallback

* 🚧 apply: No unnecessary copies on already applied paths (no slice management)

* 🚧 apply: Add FIXME for slice management

* 🐛 pathAlreadyApplied: Filter paths containing slices, not sure if these are applied...

* 💡✅ pathAlreadyApplied documentation and tests

packages/immutadot/src/core/apply.js

@@ -2,6 +2,7 @@ import {
   getSliceBounds,
   isIndex,
   isSlice,
+  pathAlreadyApplied,
 } from './path.utils'
 
 import {
@@ -79,43 +80,52 @@ const copyIfNecessary = (value, prop, doCopy) => {
  * @since 1.0.0
  */
 const apply = operation => {
-  const curried = (path, ...args) => obj => {
-    const walkPath = (curObj, curPath, doCopy = true) => {
-      const [prop, ...pathRest] = curPath
+  const curried = (pPath, ...args) => {
+    const path = unsafeToPath(pPath)
 
-      if (isSlice(prop)) {
-        const [start, end] = getSliceBounds(prop, length(curObj))
+    const applier = (obj, appliedPaths = []) => {
+      const walkPath = (curObj, curPath, remPath, isCopy = false) => {
+        const [prop, ...pathRest] = remPath
 
-        const newArr = copy(curObj, true)
-        let noop = true
+        if (isSlice(prop)) {
+          const [start, end] = getSliceBounds(prop, length(curObj))
 
-        for (let i = start; i < end; i++) {
-          const [iNoop] = walkPath(newArr, [i, ...pathRest], false)
-          noop = noop && iNoop
+          const newArr = copy(curObj, true)
+          let noop = true
+
+          for (let i = start; i < end; i++) {
+            const [iNoop] = walkPath(newArr, curPath, [i, ...pathRest], true)
+            noop = noop && iNoop
+          }
+
+          if (noop) return [true, curObj]
+          return [false, newArr]
         }
 
-        if (noop) return [true, curObj]
-        return [false, newArr]
-      }
+        const value = isNil(curObj) ? undefined : curObj[prop]
+        const doCopy = !isCopy && !pathAlreadyApplied(curPath, appliedPaths)
 
-      const value = isNil(curObj) ? undefined : curObj[prop]
+        if (remPath.length === 1) {
+          const newObj = copyIfNecessary(curObj, prop, doCopy)
+          operation(newObj, prop, value, ...args)
+          return [false, newObj]
+        }
+
+        const [noop, newValue] = walkPath(value, [...curPath, prop], pathRest)
+        if (noop) return [true, curObj]
 
-      if (curPath.length === 1) {
         const newObj = copyIfNecessary(curObj, prop, doCopy)
-        operation(newObj, prop, value, ...args)
+        newObj[prop] = newValue
         return [false, newObj]
       }
 
-      const [noop, newValue] = walkPath(value, pathRest)
-      if (noop) return [true, curObj]
-
-      const newObj = copyIfNecessary(curObj, prop, doCopy)
-      newObj[prop] = newValue
-      return [false, newObj]
+      const [, result] = walkPath(obj, [], path)
+      return result
     }
 
-    const [, result] = walkPath(obj, unsafeToPath(path))
-    return result
+    applier.path = path
+
+    return applier
   }
 
   const uncurried = (obj, path, ...args) => curried(path, ...args)(obj)

packages/immutadot/src/core/apply.spec.js

@@ -125,7 +125,7 @@ describe('Apply', () => {
     )
   })
 
-  it('should avoid unnecessary mutations', () => {
+  it('should avoid unnecessary copies with slice operator', () => {
     immutaTest(
       input => inc(input, 'nested.prop[0:0].val', 6),
       {

packages/immutadot/src/core/path.utils.js

@@ -2,7 +2,7 @@ import {
   isNaturalInteger,
 } from 'util/lang'
 
-const getSliceBound = (value, defaultValue, length) => {
+export const getSliceBound = (value, defaultValue, length) => {
   if (value === undefined) return defaultValue
   if (value < 0) return Math.max(length + value, 0)
   return value
@@ -18,7 +18,7 @@ const getSliceBound = (value, defaultValue, length) => {
  * @private
  * @since 1.0.0
  */
-const getSliceBounds = ([start, end], length) => ([
+export const getSliceBounds = ([start, end], length) => ([
   getSliceBound(start, 0, length),
   getSliceBound(end, length, length),
 ])
@@ -30,7 +30,7 @@ const getSliceBounds = ([start, end], length) => ([
  * @private
  * @since 1.0.0
  */
-const isIndex = isNaturalInteger
+export const isIndex = isNaturalInteger
 
 /**
  * Tests whether <code>arg</code> is a valid slice index, that is an integer or <code>undefined</code>.
@@ -41,7 +41,7 @@ const isIndex = isNaturalInteger
  * @private
  * @since 1.0.0
  */
-const isSliceIndex = arg => arg === undefined || Number.isSafeInteger(arg)
+export const isSliceIndex = arg => arg === undefined || Number.isSafeInteger(arg)
 
 /**
  * Tests whether <code>arg</code> is a "slice", that is an array containing exactly 2 slice indexes.
@@ -52,15 +52,29 @@ const isSliceIndex = arg => arg === undefined || Number.isSafeInteger(arg)
  * @private
  * @since 1.0.0
  */
-const isSlice = arg => {
+export const isSlice = arg => {
   if (!Array.isArray(arg)) return false
   if (arg.length !== 2) return false
   return isSliceIndex(arg[0]) && isSliceIndex(arg[1])
 }
 
-export {
-  getSliceBounds,
-  isIndex,
-  isSlice,
-  isSliceIndex,
+/**
+ * Tests whether <code>path</code> has already been applied using a list of already applied paths.
+ * @param {Array} path The path to test.
+ * @param {Array} pAppliedPaths Already applied paths.
+ * @returns {boolean} <code>true></code> if <code>path</code> has already been applied, <code>false</code> otherwise.
+ * @memberof core
+ * @private
+ * @since 1.0.0
+ */
+export function pathAlreadyApplied(path, pAppliedPaths) {
+  const appliedPaths = pAppliedPaths.filter(appliedPath => !appliedPath.some(isSlice))
+  if (appliedPaths.length === 0) return false
+  if (path.length === 0 && appliedPaths.length !== 0) return true
+  return appliedPaths.some(appliedPath => pathIncludes(appliedPath, path))
+}
+
+function pathIncludes(path, otherPath) {
+  if (otherPath.length > path.length) return false
+  return otherPath.every((otherProp, i) => path[i] === otherProp)
 }

packages/immutadot/src/core/path.utils.spec.js

@@ -3,6 +3,7 @@ import {
   getSliceBounds,
   isSlice,
   isSliceIndex,
+  pathAlreadyApplied,
 } from './path.utils'
 
 describe('Path Utils', () => {
@@ -73,4 +74,23 @@ describe('Path Utils', () => {
       expect(isSlice([0, ''])).toBe(false)
     })
   })
+
+  describe('PathAlreadyApplied', () => {
+    it('should return true if path is included in already applied paths', () => {
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [['foo', 123, 'bar']])).toBe(true)
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [['foo', 123, 'bar', 'baz']])).toBe(true)
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [[], ['bar'], ['foo', 123, 'bar', 'baz']])).toBe(true)
+      expect(pathAlreadyApplied([], [['foo']])).toBe(true)
+    })
+
+    it('should return false if path isn\'t included in already applied paths', () => {
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [])).toBe(false)
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [['foo', 123]])).toBe(false)
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [['foo', 123, 'baz']])).toBe(false)
+    })
+
+    it('should return false if already applied paths contain slices', () => {
+      expect(pathAlreadyApplied(['foo', 123, 'bar'], [['foo', 123, 'bar', 'baz', [0, 10]]])).toBe(false)
+    })
+  })
 })

packages/immutadot/src/flow/flow.js

@@ -16,6 +16,15 @@
  * @returns {flow.flowFunction} A function successively calling <code>fns</code>
  * @since 1.0.0
  */
-const flow = (...fns) => pObj => fns.reduce((obj, fn) => fn(obj), pObj)
+const flow = (...fns) => pObj => {
+  const [result] = fns.reduce(
+    ([obj, appliedPaths], fn) => [
+      fn(obj, appliedPaths),
+      [...appliedPaths, fn.path],
+    ],
+    [pObj, []],
+  )
+  return result
+}
 
 export { flow }