Update marble diagram images for operators that accept "notifiers"

[jakovljevic-mladen]

Since RxJS v7 has been released, many operators that accept "notifiers" have been adjusted to listen to only "next" notifications. This has become mandatory as per RxJS Core Semantics guide:

Operators that accept "notifiers" (as described above), MUST ONLY recognized next values from the notifier as "notifications". Emitted completions may not be used a source of notification.

However, many marble diagram images have to be updated as they don't reliably picture this behavior. For example, audit operator displays this image:

A user "reading" this image might be deceived to think that an Observable that has been passed to audit operator needs to complete before destination will receive next "next" notification. This is not true now.

Marble diagram images of these operators need to be updated:

• audit (fixed in PR: docs(audit): fix marble diagram #6636)
• debounce (fixed in PR: docs(debounce): fix marble diagram #6639)
• throttle (fixed in PR: docs(throttle): fix marble diagram #6638)
• bufferWhen (fixed in PR: docs(bufferWhen): fix marble diagram #6767)
• windowWhen (PR: fix(windowWhen): don't signal on complete #7052)

[niklas-wortmann]

We introduced swirly a while back to generate svg marble diagrams. I did not have the time to update/add all the marble diagrams, but I did a poc with concatAll that is already deployed to rxjs.dev, you basically describe the marble diagrams in a ascii format, so there should be a concatAll.txt somewhere in the repo, additionally there are dedicated npm scripts to use those text files and generate the marble diagram. Lastly, you would just have to update the path in the tsdoc comment of the operator, likely just replacing the fileformat png with svg

[niklas-wortmann]

I would very much appreciate your support with that, if you wanna start doing this for audit and throttle that would be amazing!!!

[jakovljevic-mladen]

Hey @niklas-wortmann, thanks for letting me know. I'll check what's going on with Swirly and possibly create a (couple of) PR(s) fixing issues described in the ticket description. I may also go and re-implement all existing marble diagrams using Swirly 😁

[jakovljevic-mladen]

@benlesh, since you were mainly involved in normalizing behavior between various operators that receive notifying Observables so that they all "act" on next notification, could you please review the list I created and possibly give suggestions?

There are a lot more operators that receive notifying Observables than what's on the list, but it looks like they have relatively good marble diagrams, so I didn't add them here.

[jakovljevic-mladen]

@niklas-wortmann, all done, could you please review PRs when you have some time? I also included debounce as that one wasn't good. Also, in those PRs, I updated the first test to match the new marble diagrams.

Also, still waiting for Ben (or anyone else) to confirm bufferWhen and windowWhen and, possibly others.

[ladyleet]

@niklas-wortmann @benlesh following up on this one!

[jakovljevic-mladen]

@benlesh, I was once again reviewing this issue and I'm still not sure whether windowWhen marble diagram should be fixed. What it looks like, a windowWhen test suggests that it also accepts complete notifications. This is also suggested here where OperatorSubscriber accepts third parameter which is the same as the second one (the second openWindow) - the third parameter is a function that gets called on complete notifications, which indicates that the same thing will happen on both next and complete events.

In contrary, bufferWhen seems to accept noop as the third parameter to OperatorSubscriber, which suggests that bufferWhen does not accept complete notifications. Is this a bug or this behavior seems like an exception for some reason? If it is a bug, please let me know, I can create a separate ticket for that.