Rxjs: shareReplay: it skips values on initial subscription if the source is sync

[josepot]

Bug Report

Current Behavior
shareReplay only emits the latest bufferSize values on the initial subscription when the source is synchronous.

Reproduction

• REPL or Repo link:
  https://stackblitz.com/edit/rxjs-qc2bsv?devtoolsheight=60

import { from } from 'rxjs'; 
import { shareReplay } from 'rxjs/operators';

const source = from([1, 2, 3, 4]).pipe(
  shareReplay(1)
);

source.subscribe(x => console.log(x));
// the console only logs 4

Expected behavior
IMO the first subscription should log all the values (1, 2, 3, 4).

Environment

• Runtime: all
• RxJS version: 6.5.5

Possible Solution
Change these lines to:

    let innerSub: Subscription;
    if (!subject || hasError) {
      hasError = false;
      subject = new ReplaySubject<T>(bufferSize, windowTime, scheduler);
      innerSub = subject.subscribe(this);
      subscription = source.subscribe({
        next(value) { subject!.next(value); },
        error(err) {
          hasError = true;
          subject!.error(err);
        },
        complete() {
          subscription = undefined;
          subject!.complete();
        },
      });
    } else {
      innerSub = subject.subscribe(this);
    }

I'm having a hard time coming up with a marble-test that tests this :thinking:

[cartant]

Yeah, definitely a bug. The subscriber to the operator should subscribe to the subject before the subject subscribes to the source.

BTW, I was just perusing the source for re-rxjs. Very interesting.

[josepot]

Yeah, definitely a bug. The subscriber to the operator should subscribe to the subject before the subject subscribes to the source.

Np. I will send a PR, thanks!

BTW, I was just perusing the source for re-rxjs. Very interesting.

Thanks, that means a lot to me! How did you find re-rxjs?

Actually we found this bug thanks to re-rxjs :sweat_smile:

We are still working on it: we need much better tests, better docs, a blog-post introducing it, etc. I was hopping to get all that done before the end of this week and to publicly announce it once all that stuff is done.

I really think that we got something great there. We have a few teams that are using those bindings at Adaptive and they seem to like them so far. I won't lie: my dream would be for those bindings to end-up becoming the "official" React-RxJS bindings and to have them under this org... But one step at the time :smile:

[cartant]

I'm having a hard time coming up with a marble-test that tests this 🤔

The marble test would likely be something like this:

```ts
const a = cold(" (abcd!)");
const x = cold(" x-------x");
const excpected = "(abcd)--d";

const shared = a.pipe(shareReplay(1));
const result = x.pipe(mergeMapTo(shared));
````

How did you find re-rxjs?

I saw that it was a recent inclusion in Daishi's table in will-this-react-global-state-work-in-concurrent-mode and was surprised at the ✅ count

[josepot]

@cartant can I close this?

[cartant]

Yep, this can be closed.