dot-diver icon indicating copy to clipboard operation
dot-diver copied to clipboard

Does not handle empty string correctly

Open Tofandel opened this issue 1 year ago • 4 comments

const fields = { foo: 'bar' };
console.log(getByPath(fields, '')); // undefined

setByPath(fields, '', 'baz');
console.log(fields); // { "": "baz", foo: "bar" }

Expected

const fields = { foo: 'bar' };
console.log(getByPath(fields, '')); // { foo: 'bar' }

setByPath(fields, '', 'baz'); // Throw exception path is empty

Tofandel avatar Apr 23 '24 15:04 Tofandel

Ohhh interesting edge case, because "" is a valid property key. Ouch, @djfhe what do you think about this cursed case?

We may need to talk about how we want to handle this case and what meaning the path parameter has.

But we should definitely add some test cases around those weird cases and adjust the behavior. Thanks for raising the issue!

saibotk avatar Apr 23 '24 22:04 saibotk

Added some test cases for this, but it seems to work fine thou? 31 Sorry for getting to this so late.

djfhe avatar May 20 '24 18:05 djfhe

Yes but that's not really the expected behavior is it?

I think an empty path should mean the current object, instead of a key with an empty string and the code seems to agree, https://github.com/clickbar/dot-diver/blob/main/src/index.ts#L286

But this line would never happen because array pop on a split string will always return a string, never undefined

Tofandel avatar May 20 '24 23:05 Tofandel

I agree on that and will include the change as part of the next major release. Currently trying to optimize performance here, so that should (hopefully) not be to far off.

And yes, the check is useless and only exists to satisfy the typescript compiler, it will also get removed.

djfhe avatar May 21 '24 06:05 djfhe