Skip to content

WIP: Add static multicasters#5432

Closed
benlesh wants to merge 1 commit intoReactiveX:masterfrom
benlesh:static-publish-variants
Closed

WIP: Add static multicasters#5432
benlesh wants to merge 1 commit intoReactiveX:masterfrom
benlesh:static-publish-variants

Conversation

@benlesh
Copy link
Copy Markdown
Member

@benlesh benlesh commented May 14, 2020

This is another go at adding proposed static multicast functions. Returning ConnectableObservable from operators is/was a mistake.

TODO:

  • Settle on names (currently, multicastFrom (static), multicastWith (operator), publishFrom, etc)
  • Deprecate publish operator variants that do not have a supplied selector
  • Add tests
  • Add more docs

NOTE:

Also adds a test showing a weakness of our current approach with regards to composing custom observable types through operator chains. This could probably get added at another time, but I wanted to be sure to capture it while I was thinking of it, and seeing some of the ConnectableObservable code reminded me of the issue.

To discuss:

  • Names? xWith makes sense, but what about xFrom?
  • Should we have a static variant for every permutation, or just for multicast?
  • How frequently do we use these things?

@benlesh
Copy link
Copy Markdown
Member Author

benlesh commented May 14, 2020

Related #5431

@benlesh benlesh force-pushed the static-publish-variants branch from e90dde0 to 134bc87 Compare May 14, 2020 05:03
@benlesh benlesh force-pushed the static-publish-variants branch from 134bc87 to e4fdd3f Compare May 14, 2020 05:05
Copy link
Copy Markdown
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I like the names and the approach. This ought to make the transition easier than it would have been if multicast had been made static without being renamed, etc.

I'll have another look when this is no longer a WIP.

@jgbpercy
Copy link
Copy Markdown

Related #3833

@benlesh
Copy link
Copy Markdown
Member Author

benlesh commented Aug 5, 2020

Closing in favor of #5634

@benlesh benlesh closed this Aug 5, 2020
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

Successfully merging this pull request may close these issues.

3 participants