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

Why is the pluck deprecated? #5453

Closed
jakovljevic-mladen opened this issue May 24, 2020 · 6 comments
Closed

Why is the pluck deprecated? #5453

jakovljevic-mladen opened this issue May 24, 2020 · 6 comments

Comments

@jakovljevic-mladen
Copy link
Member

I saw this commit and I'm wondering why pluck is deprecated? I couldn't find any issue where this is discussed.
By the logic behind the explanation about using optional chaining, this means that you can easily deprecate lot's of other simple operators like startWith, endWith, timestamp and so on, because it's always possible to use pipe + something in case of these. I just wonder what's the reason behind deprecating this operator.

@kolodny
Copy link
Member

kolodny commented May 25, 2020

pluck('prop') is a shorthand for map(x => x.prop), the main advantage to using pluck was the path traversal safety, meaning you can do

pluck('foo', 'bar', 'baz')

and this would be fine even if the emitted value didn't have a foo property. Contrast this with
map(x => x.foo.bar.baz) which will error if the emitted value doesn't have a foo property

With optional chaining this advantage is no longer true since you can now do

map(x => x?.foo?.bar?.baz)

And it will be as safe as the pluck usage. Note that having proper TypeScript typing for pluck is quite complex and not as robust as the map usage which is a major reason to prefer the map method

@cartant
Copy link
Collaborator

cartant commented May 25, 2020

FWIW, deprecated parts of the API won't necessarily be removed in the very next major version of the library. ATM, one of the things on which we are working is improving the deprecation messages to make them easier to understand and to include links to the documentation to explain what is being deprecated and when it's likely to be removed. IMO, pluck won't be removed from the API until version 8 or later.

@jakovljevic-mladen
Copy link
Member Author

Thanks for the clarifications @kolodny, @cartant. I like this operator and I use it a lot because it's easier (and more clean IMO) to write pluck than map.

@rdohms
Copy link

rdohms commented Sep 6, 2022

Just dropping my 2 cents here.

As much as the code is equivalent and the usage of optional chaining is preferred, with the transition to map you are losing a huge "readability" advantage for the person reading the code.

It's mentally a lot easier to recognize a pluck than to identify the map call is doing the same. Maybe an alias function that still uses the underlying code but allows quicker recognition?

@alberto-chiesa
Copy link

alberto-chiesa commented Jun 7, 2023

Pluck has a place, exactly like every other small method.

// if I have an input containing a single property, or a property list:
const pathToSubProperty = ['foo', 'bar', 'baz'];

// ... pluck allows me to easily use it:
pluck(...pathToSubProperty),

Good luck doing it with map. You're probably going to rewrite pluck by yourself.

Sorry for being rude, but I find the ease of the RxJS team in deprecating methods disturbing.
Major versions are coming, and everyone using RxJS either directly or as a dependency (e.g. in Angular applications) is going to be impacted.

Removing pluck has NO BENEFIT FOR ANYONE.
Bundle size? We are already tree shaking it if its not used.
It's not difficult to test or explain.

Cost of removal: need to update every (thousands, possibly millions) of applications using pluck.

Same thing with the subscribe and tap signatures.

You are literally making my code legacy. Please, stop and remove the deprecations!
RxJS is the best thing happened to JavaScript in a very long time. Don't make it impossible to follow the updates.

@krawaller
Copy link

And it will be as safe as the pluck usage. Note that having proper TypeScript typing for pluck is quite complex and not as robust as the map usage which is a major reason to prefer the map method

Late to the party, but, I just wanted to echo this part ☝️ of @kolodny's argument for preferring map. We got bit by an unintended action payload shape change, which was hidden from TS via pluck. The path was no longer valid, which meant that the output of pluck was typed as unknown, but with some bad luck in exactly how that was consumed downstream (straight comparisons and string concatenation), there was no TS error. If we'd used map instead from the beginning, we would have immediately noticed the erroneous payload shape, instead of shipping broken code..

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

No branches or pull requests

6 participants