you-dont-need / You-Dont-Need-Lodash-Underscore

List of JavaScript methods which you can use natively + ESLint Plugin
MIT License
18.61k stars 813 forks source link

_.pick example is incorrect when some specified key doesn't exist #393

Closed yoshikazusawa closed 3 months ago

yoshikazusawa commented 5 months ago
var schema = { 'a': 1, 'b': '2' };

// Underscore/Lodash
var result = _.pick(schema, ['a', 'c']);
console.log(result)
// output: { a: 1 }

// Native
const { a, c } = schema;
result = { a, c };
console.log(result);
// output: { a: 1, c: undefined }

Which is important to c describes optional but non-null value.

binury commented 5 months ago

True and this is covered by the native implementation outlined directly after the destructured assignment example, isn't it?

https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore/blob/f1abfc5c6dc480e8b41aac3cabc2ff05c857d7ee/README.md?plain=1#L2832-L2843

binury commented 5 months ago

I agree, though. Destructuring is not directly comparable to using _.pick really, at all. Looking at the blame, it seems like these lines were added as a shoehorned change in #168. And it's unclear enough as to why that I almost wonder if the changes slipped through review, all those years ago.

https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore/pull/168/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R1918-R1924

yoshikazusawa commented 5 months ago

@binury Yes, function pick(object, keys) implementation is correct.

Thank you for the information about #168.

binury commented 5 months ago

@binury

Yes, function pick(object, keys) implementation is correct.

Thank you for the information about #168.

maybe a PR to remove the destructured assignment lines, if you'd be willing/like to?