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

Reinstating *some* of the resultSelectors #5824

Closed
benlesh opened this issue Oct 13, 2020 · 2 comments
Closed

Reinstating *some* of the resultSelectors #5824

benlesh opened this issue Oct 13, 2020 · 2 comments
Labels
AGENDA ITEM Flagged for discussion at core team meetings

Comments

@benlesh
Copy link
Member

benlesh commented Oct 13, 2020

Currently we have deprecated resultSelectors on the following:

  • bindCallback
  • bindNodeCallback
  • combineLatest
  • forkJoin
  • fromEvent
  • fromEventPattern
  • zip
  • concatMap
  • concatMapTo
  • exhaustMap
  • mergeMap
  • mergeMapTo
  • switchMap
  • swithMapTo

The reasons for this were two fold:

  1. It added size to every operator.
  2. It meant that, in some cases, "outer values" from "outer sources" would need to be retained in memory for the lifetime of inner subscriptions. Causing memory pressure.

Given that I think recently the cost of 1 has gotten to be substantially less, I think we may want to revisit the operators where 2 is not the case (basically anything that isn't a map-and-flatten operator).

I'd propose that we can probably "undeprecate" the resultSelector argument for these creation functions:

  • bindCallback
  • bindNodeCallback
  • combineLatest
  • forkJoin
  • fromEvent
  • fromEventPattern
  • zip

This is providing that they are not encumbered by issues 1 and 2 above. (I'm sure that 2 is not an issue, but 1 may or may or may not be true). I think this is especially okay if we're getting rid of "n-args" for sources for forkJoin, combineLatest, and (probably) zip.

@benlesh benlesh added AGENDA ITEM Flagged for discussion at core team meetings type: discussion labels Oct 13, 2020
@benlesh
Copy link
Member Author

benlesh commented Oct 13, 2020

Also worth noting: The secondary function/mapping argument is a little confusing to read, IMO: mergeMap(fn, fn2). If we can eliminate the confusing UX there, and mitigate 1 and 2, I might be okay with undeprecating all resultSelectors. Just a thought.

While we have until v8 to make this decision, I think we should try to decide quickly so we can get our messaging across sooner rather than later.

@niklas-wortmann
Copy link
Member

I remember that @cartant wanted to discuss some of the deprecations as well, this is probably a good time to do this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AGENDA ITEM Flagged for discussion at core team meetings
Projects
None yet
Development

No branches or pull requests

2 participants