Default to array #34
Default to array #34
Conversation
| it('can create array if all keys are numbers', () => { | ||
| const result = u.updateIn('a.1', 3, null); | ||
|
|
||
| expect(result).to.eql({ a: [, 3] }); |
There was a problem hiding this comment.
This leaves the linter unhappy, but I am not sure how to test this. Creating a new array and setting a value at an index i where i is greater than array.length results in some an array with some elements missing (docs). This seems fine from an application level, but testing it in this way makes the linter unhappy.
Should I just disable the linter warnings for this line?
|
This looks, good, once you get stuff passing can you rebase into a single commit and update the readme/changelog too? Thanks! |
a-b-r-o-w-n
force-pushed
the
default-to-array
branch
2 times, most recently
from
2bd6ae6 to
c33ae8f
Compare
| it('can create array if all keys are numbers', () => { | ||
| const result = u.updateIn('a.1', 3, null); | ||
|
|
||
| expect(result).to.eql({ a: [, 3] }); // eslint-disable-line no-sparse-arrays |
There was a problem hiding this comment.
I'm actually surprised that this is valid syntax. It's not pretty though. Another option would be:
expect(result.a[1]).to.eql(3);There was a problem hiding this comment.
well, you'd also need to test that it is an array if you did that. You could also just use the 0 index. I'm good w/ any of these (including what you did)
There was a problem hiding this comment.
I'll do 0 index
a-b-r-o-w-n
force-pushed
the
default-to-array
branch
from
c33ae8f to
0b0f6dc
Compare
fixes #22