Skip to content

Commit

Permalink
fix(Rx): make sure all necessary operators are added for router inter…
Browse files Browse the repository at this point in the history
…nal use

There was an issue where all unit tests would pass even though all of the appropriate operators for Rx usage inside of the router were not added. This is because importing all of Rx (as the tests had done) adds all operators by default.

This change forces the tests to require the same observable that the router is using, and for any additional operators or functionality, the tests are pulling in those bits from RxJS directly without mutating the prototype.

We should be more careful from now on to make sure no tests bring in all of Rx.
  • Loading branch information
benlesh committed Jan 12, 2017
1 parent 8abb803 commit eab1145
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 27 deletions.
9 changes: 9 additions & 0 deletions src/RouterRx.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,19 @@ var RouterRx = {
require('rxjs/add/observable/defer');
require('rxjs/add/observable/of');
require('rxjs/add/observable/from');
require('rxjs/add/observable/throw');
require('rxjs/add/observable/empty');

require('rxjs/add/operator/mergeMap');
require('rxjs/add/operator/do');
require('rxjs/add/operator/defaultIfEmpty');
require('rxjs/add/operator/materialize');
require('rxjs/add/operator/expand');
require('rxjs/add/operator/reduce');
require('rxjs/add/operator/toArray');
require('rxjs/add/operator/map');
require('rxjs/add/operator/filter');
require('rxjs/add/operator/catch');
require('rxjs/add/operator/concat');

module.exports = RouterRx;
2 changes: 1 addition & 1 deletion test/data/GenrelistRoutes.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var Rx = require('rxjs');
var Rx = require('../../src/RouterRx');
var Observable = Rx.Observable;
var TestRunner = require('./../TestRunner');
var falcor = require('falcor');
Expand Down
2 changes: 1 addition & 1 deletion test/data/VideoRoutes.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var Rx = require('rxjs');
var Rx = require('../../src/RouterRx');
var Observable = Rx.Observable;
var R = require('../../src/Router');
var TestRunner = require('./../TestRunner');
Expand Down
2 changes: 1 addition & 1 deletion test/unit/core/call.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var R = require('../../../src/Router');
var noOp = function() {};
var chai = require('chai');
Expand Down
2 changes: 1 addition & 1 deletion test/unit/core/get.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var expect = chai.expect;
var falcor = require('falcor');
var $ref = falcor.Model.ref;
var $atom = falcor.Model.atom;
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var sinon = require('sinon');

describe('Get', function() {
Expand Down
15 changes: 5 additions & 10 deletions test/unit/functional/collapse-and-batch.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ var expect = chai.expect;
var falcor = require('falcor');
var $ref = falcor.Model.ref;
var $atom = falcor.Model.atom;
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var Promise = require('promise');
var delay = require('rxjs/operator/delay').delay;

describe('Collapse and Batch', function() {
it('should ensure that collapse is being ran.', function(done) {
Expand Down Expand Up @@ -71,9 +72,7 @@ describe('Collapse and Batch', function() {
var routes = [{
route: 'one[{integers:ids}]',
get: function(aliasMap) {
return Observable.
from(aliasMap.ids).
delay(100).
return delay.call(Observable.from(aliasMap.ids), 100).
map(function(id) {
if (id === 0) {
return {
Expand All @@ -91,9 +90,7 @@ describe('Collapse and Batch', function() {
route: 'two.be[{integers:ids}].summary',
get: function(aliasMap) {
called(1);
return Observable.
from(aliasMap.ids).
delay(2000).
return delay.call(Observable.from(aliasMap.ids), 2000).
map(function(id) {
return {
path: ['two', 'be', id, 'summary'],
Expand All @@ -105,9 +102,7 @@ describe('Collapse and Batch', function() {
route: 'three.four[{integers:ids}].summary',
get: function(aliasMap) {
called(2);
return Observable.
from(aliasMap.ids).
delay(2000).
return delay.call(Observable.from(aliasMap.ids), 2000).
map(function(id) {
return {
path: ['three', 'four', id, 'summary'],
Expand Down
13 changes: 6 additions & 7 deletions test/unit/functional/materialized.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,18 @@ var chai = require('chai');
var expect = chai.expect;
var falcor = require('falcor');
var $ref = falcor.Model.ref;
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var delay = require('rxjs/operator/delay').delay;

describe('Materialized Paths.', function() {
function partialRouter() {
return new R([{
route: 'one[{integers:ids}]',
get: function(aliasMap) {
return Observable.
of({
path: ['one', 0],
value: $ref(['two', 'be', 956])
}).
delay(100);
return delay.call(Observable.of({
path: ['one', 0],
value: $ref(['two', 'be', 956])
}), 100);
}
}]);
}
Expand Down
2 changes: 1 addition & 1 deletion test/unit/functional/precedence.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ var R = require('../../../src/Router');
var noOp = function() {};
var chai = require('chai');
var expect = chai.expect;
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var sinon = require('sinon');

describe('Precedence Matching', function() {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/functional/return-types.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ var R = require('../../../src/Router');
var noOp = function() {};
var chai = require('chai');
var expect = chai.expect;
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var Promise = require('promise');

describe('return-types', function() {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/functional/unhandled.call.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ var noOp = function() {};
var chai = require('chai');
var expect = chai.expect;
var sinon = require('sinon');
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;

describe('#call', function() {
it('should ensure a missing function gets chained.', function(done) {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/functional/unhandled.get.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var chai = require('chai');
var expect = chai.expect;
var sinon = require('sinon');
var pathValueMerge = require('./../../../src/cache/pathValueMerge');
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var $atom = require('./../../../src/support/types').$atom;

describe('#get', function() {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/functional/unhandled.set.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var chai = require('chai');
var expect = chai.expect;
var sinon = require('sinon');
var pathValueMerge = require('./../../../src/cache/pathValueMerge');
var Observable = require('rxjs').Observable;
var Observable = require('../../../src/RouterRx').Observable;
var $atom = require('./../../../src/support/types').$atom;
var $ref = require('./../../../src/support/types').$ref;

Expand Down
2 changes: 1 addition & 1 deletion test/unit/internal/rxNewToRxNewAndOld.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var Observable = require('rxjs/Rx').Observable;
var Observable = require('../../../../src/RouterRx').Observable;
var rxNewToRxNewAndOld =
require('../../../../src/run/conversion/rxNewToRxNewAndOld');
var chai = require('chai');
Expand Down

0 comments on commit eab1145

Please sign in to comment.