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

Allow setIn to automatically create arrays. #3

Merged
merged 1 commit into from Dec 7, 2016

Conversation

YellowKirby
Copy link
Contributor

Today, if you use setIn where a path segment includes an integer, timm creates an intermediate object rather than an array:

timm.setIn({}, ['foo', 0, 'bar'], 'value'); // => { foo: { '0': { bar: 'value' } } }

This PR updates set (and therefore setIn) to create an array rather than an object if the new key is an integer:

timm.setIn({}, ['foo', 0, 'bar'], 'value'); // => { foo: [ { bar: 'value' } ] }

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8bfb9a3 on YellowKirby:auto-create-arrays-if-needed into 18004ec on guigrpa:master.

@guigrpa
Copy link
Owner

guigrpa commented Dec 7, 2016

I see. Timm already respected existing arrays, but did not correctly create new nested ones. Thanks for the improvement!

@guigrpa guigrpa merged commit 66e6b2c into guigrpa:master Dec 7, 2016
@YellowKirby
Copy link
Contributor Author

Thanks @guigrpa for the quick release. I just tried this out in the project I'm working on and it turns out that Number.isInteger doesn't currently get transpiled by Babel, so this ends up breaking in older node and IEs (I should've npm linked it before submitting the PR :).

I'd be happy to submit another PR to fix for these environments: are you okay with adding a dependency on babel-runtime? If not, adding an isInteger util function to this project would be really simple.

Also thought of another case: what happens when the integer segment is a negative number?

timm.setIn({}, ['foo', -17, 'bar'], 'value')

Should this case create an object with "-17" as the key?

@YellowKirby YellowKirby deleted the auto-create-arrays-if-needed branch December 7, 2016 05:55
@guigrpa
Copy link
Owner

guigrpa commented Dec 7, 2016

I'd rather not add the babel-runtime dependency, I'd go for an isInteger util function as you suggest.

Re negative indices, I'm happy with the current behaviour (with your PR): it creates an array, and stores the new item in the provided key. It seems the most straightforward approach: provide a string key, you get an object; provide a number (whatever the number is), you get an array.

In fact, expanding on the last paragraph, I would even create an array if the user provides any number, even a decimal one. What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants