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

refactor(mapTo): smaller implementation #6393

Merged
merged 1 commit into from
May 12, 2021

Conversation

jakovljevic-mladen
Copy link
Member

Description:
Smaller mapTo implementation.

Related issue (if exists):
None

@kwonoj
Copy link
Member

kwonoj commented May 10, 2021

My memory's very vague - didn't we try something similar and back it off before? was it different operator? /cc @benlesh

@benlesh
Copy link
Member

benlesh commented May 12, 2021

Honestly, we talked about doing this at some point (I'm not sure if it was in an issue or a discussion). I mean, it's obvious.

However, it was decided at the time that mapTo was so popular that we should allow it to be as optimized as possible. That said, I'm not entirely sure how much better it is the way it was over the way this PR is proposed.

In fact, I'm inclined to deprecate all xMapTo operators, just because they're silly when they're just hiding a closure. mergeMapTo(x) and mergeMap(() => x) no matter how we'd reasonably write them are pretty much the same thing. Just one does the closure for you.

There was probably an advantage when our operator internals were all class-based, but now it's unlikely. Especially with closures being more optimized in modern runtimes.

@benlesh benlesh merged commit c9b53f4 into ReactiveX:master May 12, 2021
@jakovljevic-mladen jakovljevic-mladen deleted the mapTo_smallify branch May 12, 2021 07:12
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

Successfully merging this pull request may close these issues.

4 participants