Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deep-assign): add deepAssign to objects in strategy create's #1

Merged
merged 5 commits into from
May 21, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
]
},
"dependencies": {
"deep-assign": "^2.0.0",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screen shot 2017-05-21 at 9 41 19 am

sindresorhus/deep-assign#26

Not sure what the deal is here. Perhaps consider using https://www.npmjs.com/package/lodash.merge, which performs a deep merge w/o pulling in the entirety of lodash.

"halcyon": "^0.19.1"
}
}
5 changes: 4 additions & 1 deletion src/plugins/array.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const deepAssign = require('deep-assign')
const { without } = require('halcyon')

module.exports = {
Expand Down Expand Up @@ -30,7 +31,9 @@ module.exports = {
if (mods.remove && !Array.isArray(mods.remove)) throw new Error(`Remove property must be an array`)
mods.add = mods.add || []
mods.remove = mods.remove || []
// Deep assign nested objects
const assign = (arr) => arr.map((x) => typeof x === 'object' ? deepAssign({}, x) : x)
// Apply modifications and return
return without(mods.remove, [ ...data ]).concat([ ...mods.add ])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you want is:

without(mods.remove, [...data.map(assign)].concat([...mods.add.map(assign)])

You're already shallow-cloning the array with ...[], so unless deep-assign maps over all array elements and deep clones them, that's what you'll need to do yourself.

return without(mods.remove, [ ...assign(data) ]).concat([ ...assign(mods.add) ])
}
}
4 changes: 3 additions & 1 deletion src/plugins/json.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const deepAssign = require('deep-assign')

module.exports = {
/**
* @property {String} plugin name
Expand All @@ -24,6 +26,6 @@ module.exports = {
*/
create: (data, mods = {}) => {
if (typeof mods !== 'object') throw new Error(`Must supply a valid object`)
return JSON.stringify(Object.assign({}, data, mods))
return JSON.stringify(deepAssign({}, data, mods))
}
}
4 changes: 3 additions & 1 deletion src/plugins/object.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const deepAssign = require('deep-assign')

module.exports = {
/**
* @property {String} plugin name
Expand All @@ -21,6 +23,6 @@ module.exports = {
*/
create: (data, mods = {}) => {
if (typeof mods !== 'object') throw new Error(`Must supply a valid object`)
return Object.assign({}, data, mods)
return deepAssign({}, data, mods)
}
}
6 changes: 3 additions & 3 deletions test/src/plugins/array.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ describe('plugins/array', () => {
})
describe('create', () => {
it('returns a new instance of the array with mods applied', () => {
const original = [ 'foo', 'bar' ]
const original = [ 'foo', { fizz: 'buzz' }, 'bar' ]
const actual = array.create(original, {
add: [ 'baz' ],
remove: [ 'bar' ]
})
expect(original).to.deep.equal([ 'foo', 'bar' ])
expect(actual).to.deep.equal([ 'foo', 'baz' ])
expect(original).to.deep.equal([ 'foo', { fizz: 'buzz' }, 'bar' ])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to perform two comparisons here, one to check that references are broken, and the other to confirm deep equality:

const obj = { fizz: 'buzz' }
const original = [ 'foo', { fizz: 'buzz' }, 'bar' ]

// compare references
expect(original[1]).to.not.equal(actual[1])

// compare equality
expect(original[1]).to.deep.equal(actual[1])

This way you're ensuring the feature fundamentally offered by fixd, namely that objects cannot be mutated between tests.

expect(actual).to.deep.equal([ 'foo', { fizz: 'buzz' }, 'baz' ])
})
it('throws if mods argument is not an object', () => {
expect(() => array.create([ 'foo' ], 'bar')).to.throw(/Must supply a valid object/)
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ decamelize@^1.0.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290"

deep-assign@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/deep-assign/-/deep-assign-2.0.0.tgz#ebe06b1f07f08dae597620e3dd1622f371a1c572"
dependencies:
is-obj "^1.0.0"

deep-eql@^0.1.3:
version "0.1.3"
resolved "https://registry.yarnpkg.com/deep-eql/-/deep-eql-0.1.3.tgz#ef558acab8de25206cd713906d74e56930eb69f2"
Expand Down Expand Up @@ -794,6 +800,10 @@ is-my-json-valid@^2.10.0:
jsonpointer "^4.0.0"
xtend "^4.0.0"

is-obj@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/is-obj/-/is-obj-1.0.1.tgz#3e4729ac1f5fde025cd7d83a896dab9f4f67db0f"

is-path-cwd@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/is-path-cwd/-/is-path-cwd-1.0.0.tgz#d225ec23132e89edd38fda767472e62e65f1106d"
Expand Down