refactor(switchLatest): make switchLatest a standalone impl, add virtual time tests #317

benlesh · 2015-09-16T00:59:14Z

completely refactors switchLatest to be a standalone implementation
adds a lot of tests around switch latest

kwonoj · 2015-09-16T07:14:38Z

spec/operators/switchLatest-spec.js

@@ -3,15 +3,166 @@ var Rx = require('../../dist/cjs/Rx');
 var Observable = Rx.Observable;
 var immediateScheduler = Rx.Scheduler.immediate;

-describe('Observable.prototype.switchLatest()', function () {
+fdescribe('Observable.prototype.switchLatest()', function () {


Oops. Good catch.

That's to limit jasmine tests to just that grouping during development. I meant to remove that.

- adds a lot of virtual tests around switchLatest - switchLatest no longer depends on flatMap addresses #302

…unsub - ensures that inner subscriptions are NOT provided a destination, which changes the executiong path of unsubscribe() fixes #302

trxcllnt · 2015-09-17T19:04:32Z

src/operators/switchAll.ts

@@ -58,13 +66,18 @@ class SwitchSubscriber<T> extends Subscriber<T> {
 }

 class InnerSwitchSubscriber<T> extends Subscriber<T> {
-  constructor(destination: Observer<any>, private parent: SwitchSubscriber<T>) {
-    super(destination);


@Blesh couldn't we still set destination on the InnerSwitchSubscriber, but invoke super with no argument? I believe the only logic we want to short-circuit here is the shared _subscription assignment. Then we don't have to override _next or _error.

Yes, that would probably work. If you want to make that change, feel free to PR.

I'm going to devote some thought to the whole shared-subscription approach and see if I can poke any more holes in it. The solution may be as simple as explicitly passing the shared subscription to Subscriber's constructor, instead of reading it off the destination. Will let you know.

lock · 2018-06-07T01:41:39Z

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.

benlesh mentioned this pull request Sep 16, 2015

SwitchAll / SwitchLatest are broken #302

Closed

benlesh changed the title ~~Switch latest tests~~ refactor(switchLatest): make switchLatest a standalone impl, add virtual time tests Sep 16, 2015

kwonoj reviewed Sep 16, 2015
View reviewed changes

benlesh added 2 commits September 17, 2015 10:50

refactor(switchLatest): now switchLatest is a standalone implementation

c78f940

- adds a lot of virtual tests around switchLatest - switchLatest no longer depends on flatMap addresses #302

fix(switchAll/switchLatest): inner subscriptions should now properly …

38a45f8

…unsub - ensures that inner subscriptions are NOT provided a destination, which changes the executiong path of unsubscribe() fixes #302

benlesh merged commit 38a45f8 into ReactiveX:master Sep 17, 2015

trxcllnt reviewed Sep 17, 2015
View reviewed changes

benlesh deleted the switchLatest-tests branch April 27, 2016 17:20

lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(switchLatest): make switchLatest a standalone impl, add virtual time tests #317

refactor(switchLatest): make switchLatest a standalone impl, add virtual time tests #317

benlesh commented Sep 16, 2015

kwonoj Sep 16, 2015

benlesh Sep 16, 2015

trxcllnt Sep 17, 2015

benlesh Sep 17, 2015

trxcllnt Sep 17, 2015

lock bot commented Jun 7, 2018

refactor(switchLatest): make switchLatest a standalone impl, add virtual time tests #317

refactor(switchLatest): make switchLatest a standalone impl, add virtual time tests #317

Conversation

benlesh commented Sep 16, 2015

kwonoj Sep 16, 2015

Choose a reason for hiding this comment

benlesh Sep 16, 2015

Choose a reason for hiding this comment

trxcllnt Sep 17, 2015

Choose a reason for hiding this comment

benlesh Sep 17, 2015

Choose a reason for hiding this comment

trxcllnt Sep 17, 2015

Choose a reason for hiding this comment

lock bot commented Jun 7, 2018