Skip to content

Commit

Permalink
⚡ Avoid unnecessary mutations in apply (#144)
Browse files Browse the repository at this point in the history
* ⚡ Avoid unnecessary mutations in apply

* 👌 review @frinyvonnick
  • Loading branch information
nlepage committed Nov 29, 2017
1 parent 345146d commit 132d1ff
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 27 deletions.
3 changes: 2 additions & 1 deletion misc/test.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ export const immutaTest = (cb, input, ...paths) => {

expect(input).toBeDeep(inputRefs)
expect(output).toBeDeep(inputRefs, { exclude: paths })
expect(output).not.toBeDeep(inputRefs, { include: paths })
if (paths.length !== 0)
expect(output).not.toBeDeep(inputRefs, { include: paths })
}
41 changes: 31 additions & 10 deletions packages/immutadot/src/core/apply.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,21 @@ const copy = (value, asArray) => {
return { ...value }
}

/**
* Makes a copy of <code>value</code> if necessary.
* @function
* @param {*} value The value to make a copy of
* @param {string} prop The accessed property in <code>value</code>
* @param {boolean} doCopy Whether to make a copy or not
* @returns {Object|Array} A copy of value, or not ;)
* @private
* @since 1.0.0
*/
const copyIfNecessary = (value, prop, doCopy) => {
if (!doCopy) return value
return copy(value, isIndex(prop))
}

/**
* Operation to apply on a nested property of an object, to be called by {@link core.apply|apply}.
* @memberof core
Expand Down Expand Up @@ -72,29 +87,35 @@ const apply = operation => {
const [start, end] = getSliceBounds(prop, length(curObj))

const newArr = copy(curObj, true)
let noop = true

for (let i = start; i < end; i++)
walkPath(newArr, [i, ...pathRest], false)
for (let i = start; i < end; i++) {
const [iNoop] = walkPath(newArr, [i, ...pathRest], false)
noop = noop && iNoop
}

return newArr
if (noop) return [true, curObj]
return [false, newArr]
}

const value = isNil(curObj) ? undefined : curObj[prop]

let newObj = curObj
if (doCopy) newObj = copy(curObj, isIndex(prop))

if (curPath.length === 1) {
const newObj = copyIfNecessary(curObj, prop, doCopy)
operation(newObj, prop, value, ...args)
return newObj
return [false, newObj]
}

newObj[prop] = walkPath(value, pathRest)
const [noop, newValue] = walkPath(value, pathRest)
if (noop) return [true, curObj]

return newObj
const newObj = copyIfNecessary(curObj, prop, doCopy)
newObj[prop] = newValue
return [false, newObj]
}

return walkPath(obj, unsafeToPath(path))
const [, result] = walkPath(obj, unsafeToPath(path))
return result
}

const uncurried = (obj, path, ...args) => curried(path, ...args)(obj)
Expand Down
66 changes: 50 additions & 16 deletions packages/immutadot/src/core/apply.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,23 +105,57 @@ describe('Apply', () => {
'nested.prop.1.val',
'nested.prop.2.val',
)

immutaTest(
input => {
const output = inc(input, 'nested.prop[3:5].val', 6)
expect(output).toEqual({
nested: { prop: [{ val: 0 }, { val: 1 }, undefined, { val: 6 }, { val: 6 }] },
other: {},
})
return output
},
{
nested: { prop: [{ val: 0 }, { val: 1 }] },
other: {},
},
'nested.prop.2',
'nested.prop.3.val',
'nested.prop.4.val',
)
})

immutaTest(
input => {
const output = inc(input, 'nested.prop[3:5].val', 6)
expect(output).toEqual({
nested: { prop: [{ val: 0 }, { val: 1 }, undefined, { val: 6 }, { val: 6 }] },
it('should avoid unnecessary mutations', () => {
immutaTest(
input => inc(input, 'nested.prop[0:0].val', 6),
{
nested: { prop: [{ val: 0 }, { val: 1 }] },
other: {},
},
)

immutaTest(
input => inc(input, 'nested.prop[:].arr[0:0].val', 6),
{
nested: { prop: [{ arr: [{ val: 0 }, { val: 1 }] }, { arr: [{ val: 2 }] }] },
other: {},
},
)

immutaTest(
input => {
const output = inc(input, 'nested.prop[:].arr[1:].val', 6)
expect(output).toEqual({
nested: { prop: [{ arr: [{ val: 0 }, { val: 7 }] }, { arr: [{ val: 2 }] }] },
other: {},
})
return output
},
{
nested: { prop: [{ arr: [{ val: 0 }, { val: 1 }] }, { arr: [{ val: 2 }] }] },
other: {},
})
return output
},
{
nested: { prop: [{ val: 0 }, { val: 1 }] },
other: {},
},
'nested.prop.2',
'nested.prop.3.val',
'nested.prop.4.val',
)
},
'nested.prop.0.arr.1.val',
)
})
})

0 comments on commit 132d1ff

Please sign in to comment.