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

feat(shareReplay): adds shareReplay variant of publishReplay #2443

Merged
merged 1 commit into from
May 9, 2017
Merged

feat(shareReplay): adds shareReplay variant of publishReplay #2443

merged 1 commit into from
May 9, 2017

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Mar 4, 2017

shareReplay returns an observable that is the source multicasted over a ReplaySubject. That replay subject is recycled on error from the source, but not on completion of the source. This makes shareReplay ideal for handling things like caching AJAX results, as it's retryable. It's repeat behavior, however, differs from share in that it will not repeat the source observable, rather it will repeat the source observable's values.

related #2013, #453, #2043

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 97.592% when pulling f1e8cc0 on benlesh:shareReplay into 7c41c08 on ReactiveX:master.

`shareReplay` returns an observable that is the source multicasted over a `ReplaySubject`. That replay subject is recycled on error from the `source`, but not on completion of the source. This makes `shareReplay` ideal for handling things like caching AJAX results, as it's retryable. It's repeat behavior, however, differs from `share` in that it will not repeat the `source` observable, rather it will repeat the `source` observable's values.

related #2013, #453, #2043
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.009%) to 97.679% when pulling b0f7266 on benlesh:shareReplay into 7c41c08 on ReactiveX:master.

@benlesh
Copy link
Member Author

benlesh commented Mar 4, 2017

I suspect the 0.009% change in coverage isn't worth blocking this over, as it's pretty much a rounding error.

@robwormald
Copy link
Contributor

It's repeat behavior, however, differs from share in that it will not repeat the source observable, rather it will repeat the source observable's values.

can you expand on the behavior differences? Perhaps with an example?

@coveralls
Copy link

coveralls commented Mar 4, 2017

Coverage Status

Coverage decreased (-0.1%) to 97.592% when pulling f1e8cc0 on benlesh:shareReplay into 7c41c08 on ReactiveX:master.

@benlesh
Copy link
Member Author

benlesh commented Mar 4, 2017

can you expand on the behavior differences? Perhaps with an example?

@robwormald The tests show the behavior, but in case they aren't clear enough...

repeat will work exactly the same way repeat works with publishReplay().refCount().

assume that source$ below is using shareReplay()...

--a--b--c--|             // source$
--a--b--c--(abcabc|)     // source$.repeat(3);

retry on the otherhand, will work exactly the same way .multicast(() => new ReplaySubject()).refCount() does.

--a--b--c--#                                      // source$
--a--b--c----a--b--c----a--b--c----a--b--c--#     // source$.retry(3)

... hopefully, that illustrates the behavior?

@trxcllnt
Copy link
Member

trxcllnt commented Mar 5, 2017

The only reservation I have is that the ReplaySubject isn't deallocated when the refCount Observable is disposed. By referencing the RS in the Observable definition, it'll only be GC'd when the shareReplay Observable is dereferenced. None of the other operators work this way, and it seems this could introduce leaks when people unfamiliar with the implementation use Observables like normal, expecting they teardown when all the Subscriptions are disposed.

@wardbell
Copy link
Contributor

wardbell commented Mar 5, 2017

This seems promising. How do I use if for the following scenario.

I want to cache a list of customers from the server but periodically refresh that list, say, if it is more than an hour old.

  1. First subscriber triggers the first fetch (in the source$, yes?).
  2. If a second subscriber asks for customers before the first fetch succeeds, it waits until that first fetch succeeds or errors.
  3. What does the second subscriber see if it errors and we retry? Should it wait? Is there a race?
  4. How do I trigger the re-fetch after the cached value expires? That only works if the fetching source (unlike Angular http) does not complete, right?

In the absence of the shareReplay operator, I handle this scenario by managing both the fetching Observable and the ReplaySubject on my own in an imperative manner. Should I continue to do so?

Be gentle ... I'm a comparative noob.

const subscriber2 = hot(' b| ').mergeMapTo(shared);
const expected2 = ' (12)-3-# ';
const subscriber3 = hot(' (c|) ').mergeMapTo(shared);
const expected3 = ' -1-2-----3-#';
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't expected3 be (23)#?

Copy link
Member

Choose a reason for hiding this comment

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

Ignore me. It's correct. (because the source errored, it didn't complete, so we create a new ReplaySubject for the next multicast)

@staltz
Copy link
Member

staltz commented Mar 5, 2017

Apart from what has already been said (e.g. by Paul), this looks good to merge IMO.

@benlesh
Copy link
Member Author

benlesh commented Mar 5, 2017

@staltz, in discussing this with @trxcllnt on Slack, I pointed out that the memory behavior here is no different than how any of the publish operators, or multicast(subject) have worked since RxJS 2.

Thought I'd add it to the thread though, for visibility.

@benlesh benlesh merged commit 5a2266a into ReactiveX:master May 9, 2017
@QuentinFchx
Copy link

Shall the MIGRATION.md be updated aswell? It might be confusing otherwise

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants