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

Removal of _.omit in v5.0.0? #2930

Closed
billyjanitsch opened this issue Jan 12, 2017 · 11 comments
Closed

Removal of _.omit in v5.0.0? #2930

billyjanitsch opened this issue Jan 12, 2017 · 11 comments
Labels

Comments

@billyjanitsch
Copy link
Contributor

I noticed that _.omit has been removed in master, slated for v5.0.0. I couldn't find any discussion of the decision; I'm just wondering why you've chosen to remove it.

FWIW, it's one of my most frequently used Lodash methods. The roadmap suggests using _.pick instead, but it's not clear how that supports my typical use case: I want to remove keys x and y, but I don't know the structure of the rest of the object, so I don't know which remaining keys to pick. Maybe I'm missing something?

Cheers.

@jdalton
Copy link
Member

jdalton commented Jan 12, 2017

Hi @billyjanitsch!

The omit method is less optimal and more complex since it has to pull in the universe (inherited and own enumerable string key properties and symbols) and then exclude a handful of properties. On the other hand, _.pick or _.pickBy are explicit. Simply picking what you want without a lot of fuss.

My guidance would be to be explicit. Your code should really know the properties it's consuming.
If not, you can always delete the ones you don't want.

Using the Babel plugin transform-object-rest-spread/ is another great way to handle it.

@jdalton jdalton closed this as completed Jan 12, 2017
@lukeapage
Copy link

We use omit a lot and it's mostly when using objects as maps. We also require immutability, but I guess we can clone and then delete or use object spread even though it's not stage 4 yet.

@olsonpm
Copy link

olsonpm commented Jun 5, 2017

@billyjanitsch - Out of curiosity, are you passing _.omit objects with prototypes? I know my personal usage is strictly plain objects where the in operator is unnecessary. I carry a personal version of _.omit that iterates over Objects.keys, populating a new object with those not specified. I assume this is the common use-case, but I wouldn't know.

@jskrzypek
Copy link

I'm currently using _.omit and passing it property paths to leave out some nested properties. Is the future-proofed way of doing this to iterate over the array pf paths and use _.unset?

@olsonpm
Copy link

olsonpm commented Aug 31, 2017

unset mutates whereas omit doesn't. unset also doesn't take multiple paths (at least in v4, haven't checked if this was changed for v5)

@sinall
Copy link

sinall commented Jan 12, 2018

Sad too hear this, since I'm looking for 'omit' then find this lib. This is all about utilities, don't know why you would deprecate this useful & handy function.

@jdalton
Copy link
Member

jdalton commented Jan 12, 2018

Hi @sinall!

I've explained the reasoning here.
It's a bad pattern and there are other, better, ways to accomplish it.

@danielrob
Copy link

But I didn't come to omit to slim down to the keys I do want, I came to remove the keys I don't want from an unknown collection of keys (from a plain object).

Please excuse my ignorance, is the implementation not as simple as something like:

function omit(object, paths) {
   const objCopy = deepCopy(object)
   paths.forEach(path => _.unset(objCopy, path)
   return objCopy
}

This seems quite a loss to the library.

Other language libraries seem to handle this just fine: https://laravel.com/docs/5.6/collections#method-except

@olsonpm
Copy link

olsonpm commented Mar 4, 2018

the problem isn't the complexity of the algorithm, it's the poor performance of it. This should be explained in the related comments

@danielrob
Copy link

the problem isn't the complexity of the algorithm, it's the poor performance of it. This should be explained in the related comments

Seems to me the performance is O(_.cloneDeep) + O(n _.unset) = O(_.cloneDeep) + O(n)*O(1) ~ O(_.cloneDeep) for large objects. Should we also remove _.cloneDeep 🤔?

@jdalton
Copy link
Member

jdalton commented Mar 5, 2018

Sorry folks, this design choice is not up for debate.

@lodash lodash locked as resolved and limited conversation to collaborators Mar 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

7 participants