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

RFC: Hold back on releasing mapToAction #1907

Closed
MikeRyanDev opened this issue Jun 3, 2019 · 1 comment · Fixed by #1927
Closed

RFC: Hold back on releasing mapToAction #1907

MikeRyanDev opened this issue Jun 3, 2019 · 1 comment · Fixed by #1927
Labels
Need Discussion Request For Comment needs input

Comments

@MikeRyanDev
Copy link
Member

I've been playing with mapToAction since it landed and I have some concerns about the ergonomics of this utility function:

  1. It doesn't meaningfully reduce lines-of-code (some effects may increase in size)
  2. It abstracts over too many parts of rxjs. I don't think it should be selecting a merge operator for you. As designed, it makes it awkward to use an unusual merging operator like groupBy
  3. On the flip side, I think it should handle mapping results into actions without the user having to import the map operator.

That is a pretty brief description of some of my concerns about mapToAction. What I would like to suggest is that we hold off on releasing it in v8.0.0 and save it for v8.1.0. It may be fine as-is but I want to spend a little more time on the design of this function before it becomes part of our stable API.

[X] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

@brandonroberts brandonroberts added Need Discussion Request For Comment needs input 8.x and removed 8.x labels Jun 3, 2019
@alex-okrushko
Copy link
Member

For anyone following this issue - after having a team discussion we decided to rename the operator to act (similar to what @cartant named it) and not promote it for the time being (until 8.1.0+) and review it one more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Need Discussion Request For Comment needs input
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants