sindresorhus / dot-prop

Get, set, or delete a property from a nested object using a dot path
MIT License
812 stars 69 forks source link

Support array index access #71

Closed saibotsivad closed 2 years ago

saibotsivad commented 3 years ago

I ran into this issue when upgrading from 4->6, which of course is a breaking change, but I think the new behavior is inconsistent. 🙇

Consider some objects that have arrays, like:

const emptyArray = { foo: [] }
const oneItem = { foo: [ { fizz: 'buzz' } ] }

I want to access properties on the array's objects, and provide a default value if it doesn't exist.

// passes: default value is found
t.is(dotProp.get(emptyArray, 'foo.0.fizz', 'bazz'), 'bazz')
// passes: existing value is found
t.is(dotProp.get(oneItem, 'foo.0.fizz', 'bazz'), 'buzz')

However, in 4->6 the behavior changed when accessing an array index that doesn't exist:

// passes in 4 but fails in 6
t.is(dotProp.get({ foo: [ 'bar' ] }, 'foo.1', 'bazz'), 'bazz')
// passes in 6 but fails in 4
t.is(dotProp.get({ foo: [ 'bar' ] }, 'foo.1', 'bazz'), undefined)

I think that this behavior is ... inconsistent, probably?

If accessing a deeper property in an array element triggers a default value fallback, shouldn't accessing an array element trigger a fallback if that index doesn't exist?

saibotsivad commented 3 years ago

For now I've had to update a few points in the code to set a default value manually, e.g. something like this:

// previously
const myValue = dotProp.get(data, 'foo.bar.3', defaultValue)
// after 4->6
let myValue = dotProp.get(data, 'foo.bar.3')
if (myValue === undefined) {
    myValue = defaultValue
}
sindresorhus commented 3 years ago

It's not even a breaking change as the behavior was never documented or officially supported. It only worked as a side-effect of the fact that arrays in JS are just objects.

That being said, the behavior you describe does make sense. However, this is a new feature and will need lots of tests and docs.

johansteffner commented 3 years ago

The expected behaviour here seems to have been fixed by https://github.com/sindresorhus/dot-prop/pull/75.

sindresorhus commented 3 years ago

@johansteffner

That being said, the behavior you describe does make sense. However, this is a new feature and will need lots of tests and docs.

As long as it's not documented and tested, it could break in any patch release.