refactor: test for side effects, and remove existing ones (#4769) (#4953)

* test: add test for side-effects

* refactor: remove toplevel property access

Author: filipesilva

.circleci/config.yml

@@ -50,6 +50,7 @@ jobs:
     steps:
       - attach_workspace: *attach_options
       - run: npm test
+      - run: npm run test:side-effects
 
   dtslint:
     <<: *defaults

.eslintignore

@@ -0,0 +1 @@
+integration/side-effects/snapshots/

.gitignore

@@ -24,3 +24,7 @@ spec-build/
 # Misc
 npm-debug.log
 .DS_STORE
+
+# The check-side-effects package generates and deletes this file.
+# If the process is killed, it will be left behind.
+check-side-effects.tmp-input.js

integration/side-effects/README.md

@@ -0,0 +1,9 @@
+This test checks if the side effects for loading RxJs packages have changed using <https://github.com/filipesilva/check-side-effects>.
+
+Running `npm test:side-effects` will check all ES modules listed in `side-effects.json`.
+
+Running `npm test:side-effects:update` will update any changed side effects.
+
+To add a new ES module to this test, add a new entry in `side-effects.json`.
+
+Usually the ESM and FESM should have the same output, but retained objects that were renamed during the flattening step will leave behind a different name.

integration/side-effects/side-effects.json

@@ -0,0 +1,52 @@
+{
+  "tests": [
+    {
+      "esModules": "../../dist/esm5/index.js",
+      "expectedOutput": "./snapshots/esm5/index.js"
+    },
+    {
+      "esModules": "../../dist/esm2015/index.js",
+      "expectedOutput": "./snapshots/esm2015/index.js"
+    },
+    {
+      "esModules": "../../dist/esm5/ajax/index.js",
+      "expectedOutput": "./snapshots/esm5/ajax.js"
+    },
+    {
+      "esModules": "../../dist/esm2015/ajax/index.js",
+      "expectedOutput": "./snapshots/esm2015/ajax.js"
+    },
+    {
+      "esModules": "../../dist/esm5/fetch/index.js",
+      "expectedOutput": "./snapshots/esm5/fetch.js"
+    },
+    {
+      "esModules": "../../dist/esm2015/fetch/index.js",
+      "expectedOutput": "./snapshots/esm2015/fetch.js"
+    },
+    {
+      "esModules": "../../dist/esm5/operators/index.js",
+      "expectedOutput": "./snapshots/esm5/operators.js"
+    },
+    {
+      "esModules": "../../dist/esm2015/operators/index.js",
+      "expectedOutput": "./snapshots/esm2015/operators.js"
+    },
+    {
+      "esModules": "../../dist/esm5/testing/index.js",
+      "expectedOutput": "./snapshots/esm5/testing.js"
+    },
+    {
+      "esModules": "../../dist/esm2015/testing/index.js",
+      "expectedOutput": "./snapshots/esm2015/testing.js"
+    },
+    {
+      "esModules": "../../dist/esm5/webSocket/index.js",
+      "expectedOutput": "./snapshots/esm5/websocket.js"
+    },
+    {
+      "esModules": "../../dist/esm2015/webSocket/index.js",
+      "expectedOutput": "./snapshots/esm2015/websocket.js"
+    }
+  ]
+}

integration/side-effects/snapshots/esm2015/ajax.js

@@ -0,0 +1 @@
+

integration/side-effects/snapshots/esm2015/fetch.js

@@ -0,0 +1 @@
+

integration/side-effects/snapshots/esm2015/index.js

@@ -0,0 +1,7 @@
+var NotificationKind;
+
+(function(NotificationKind) {
+    NotificationKind["NEXT"] = "N";
+    NotificationKind["ERROR"] = "E";
+    NotificationKind["COMPLETE"] = "C";
+})(NotificationKind || (NotificationKind = {}));

integration/side-effects/snapshots/esm2015/operators.js

@@ -0,0 +1,7 @@
+var NotificationKind;
+
+(function(NotificationKind) {
+    NotificationKind["NEXT"] = "N";
+    NotificationKind["ERROR"] = "E";
+    NotificationKind["COMPLETE"] = "C";
+})(NotificationKind || (NotificationKind = {}));

integration/side-effects/snapshots/esm2015/testing.js

@@ -0,0 +1,7 @@
+var NotificationKind;
+
+(function(NotificationKind) {
+    NotificationKind["NEXT"] = "N";
+    NotificationKind["ERROR"] = "E";
+    NotificationKind["COMPLETE"] = "C";
+})(NotificationKind || (NotificationKind = {}));

integration/side-effects/snapshots/esm2015/websocket.js

@@ -0,0 +1,7 @@
+var NotificationKind;
+
+(function(NotificationKind) {
+    NotificationKind["NEXT"] = "N";
+    NotificationKind["ERROR"] = "E";
+    NotificationKind["COMPLETE"] = "C";
+})(NotificationKind || (NotificationKind = {}));

integration/side-effects/snapshots/esm5/ajax.js

@@ -0,0 +1 @@
+import "tslib";

integration/side-effects/snapshots/esm5/fetch.js

@@ -0,0 +1 @@
+import "tslib";

integration/side-effects/snapshots/esm5/index.js

@@ -0,0 +1 @@
+import "tslib";

integration/side-effects/snapshots/esm5/operators.js

@@ -0,0 +1 @@
+import "tslib";

integration/side-effects/snapshots/esm5/testing.js

@@ -0,0 +1 @@
+import "tslib";

integration/side-effects/snapshots/esm5/websocket.js

@@ -0,0 +1 @@
+import "tslib";

package-lock.json

@@ -101,6 +101,8 @@
     "test:cover": "nyc npm test",
     "test:circular": "dependency-cruise --validate .dependency-cruiser.json -x \"^node_modules\" dist/esm5",
     "test:systemjs": "node integration/systemjs/systemjs-compatibility-spec.js",
+    "test:side-effects": "check-side-effects --test integration/side-effects/side-effects.json",
+    "test:side-effects:update": "npm run test:side-effects -- --update",
     "tests2png": "mkdirp docs_app/content/img && mocha --opts spec/support/tests2png.opts \"spec/**/*-spec.ts\"",
     "compat_build_all": "npm-run-all compat_clean_dist compat_build_cjs compat_build_esm5 compat_build_esm2015 compat_build_esm5_for_rollup compat_build_umd compat_generate_packages",
     "compat_build_closure": "node ./tools/make-closure-compat.js",
@@ -187,6 +189,7 @@
     "benchmark": "2.1.0",
     "benchpress": "2.0.0-beta.1",
     "chai": "4.1.2",
+    "check-side-effects": "0.0.20",
     "color": "3.0.0",
     "colors": "1.1.2",
     "commitizen": "2.9.6",
@@ -238,6 +241,7 @@
     "tsconfig-paths": "3.2.0",
     "tslint": "5.9.1",
     "tslint-etc": "1.2.6",
+    "tslint-no-toplevel-property-access": "0.0.2",
     "tslint-no-unused-expression-chai": "0.0.3",
     "typescript": "^3.0.1",
     "validate-commit-msg": "2.14.0",

package.json

@@ -55,19 +55,20 @@ export class ConnectableObservable<T> extends Observable<T> {
   }
 }
 
-const connectableProto = <any>ConnectableObservable.prototype;
-
-export const connectableObservableDescriptor: PropertyDescriptorMap = {
-  operator: { value: null },
-  _refCount: { value: 0, writable: true },
-  _subject: { value: null, writable: true },
-  _connection: { value: null, writable: true },
-  _subscribe: { value: connectableProto._subscribe },
-  _isComplete: { value: connectableProto._isComplete, writable: true },
-  getSubject: { value: connectableProto.getSubject },
-  connect: { value: connectableProto.connect },
-  refCount: { value: connectableProto.refCount }
-};
+export const connectableObservableDescriptor: PropertyDescriptorMap = (() => {
+  const connectableProto = <any>ConnectableObservable.prototype;
+  return {
+    operator: { value: null as null },
+    _refCount: { value: 0, writable: true },
+    _subject: { value: null as null, writable: true },
+    _connection: { value: null as null, writable: true },
+    _subscribe: { value: connectableProto._subscribe },
+    _isComplete: { value: connectableProto._isComplete, writable: true },
+    getSubject: { value: connectableProto.getSubject },
+    connect: { value: connectableProto.connect },
+    refCount: { value: connectableProto.refCount }
+  };
+})();
 
 class ConnectableSubscriber<T> extends SubjectSubscriber<T> {
   constructor(destination: Subject<T>,

src/internal/observable/ConnectableObservable.ts

@@ -486,19 +486,21 @@ export interface AjaxErrorCtor {
   new(message: string, xhr: XMLHttpRequest, request: AjaxRequest): AjaxError;
 }
 
-function AjaxErrorImpl(this: any, message: string, xhr: XMLHttpRequest, request: AjaxRequest): AjaxError {
-  Error.call(this);
-  this.message = message;
-  this.name = 'AjaxError';
-  this.xhr = xhr;
-  this.request = request;
-  this.status = xhr.status;
-  this.responseType = xhr.responseType || request.responseType;
-  this.response = parseXhrResponse(this.responseType, xhr);
-  return this;
-}
-
-AjaxErrorImpl.prototype = Object.create(Error.prototype);
+const AjaxErrorImpl = (() => {
+  function AjaxErrorImpl(this: any, message: string, xhr: XMLHttpRequest, request: AjaxRequest): AjaxError {
+    Error.call(this);
+    this.message = message;
+    this.name = 'AjaxError';
+    this.xhr = xhr;
+    this.request = request;
+    this.status = xhr.status;
+    this.responseType = xhr.responseType || request.responseType;
+    this.response = parseXhrResponse(this.responseType, xhr);
+    return this;
+  }
+  AjaxErrorImpl.prototype = Object.create(Error.prototype);
+  return AjaxErrorImpl;
+})();
 
 export const AjaxError: AjaxErrorCtor = AjaxErrorImpl as any;
 

src/internal/observable/dom/AjaxObservable.ts

@@ -79,4 +79,4 @@ import {  AjaxObservable, AjaxCreationMethod  } from './AjaxObservable';
  *
  * ```
  */
-export const ajax: AjaxCreationMethod = AjaxObservable.create;
+export const ajax: AjaxCreationMethod = (() => AjaxObservable.create)();

src/internal/observable/dom/ajax.ts

@@ -4,7 +4,7 @@ import { isFunction } from '../util/isFunction';
 import { Subscriber } from '../Subscriber';
 import { map } from '../operators/map';
 
-const toString: Function = Object.prototype.toString;
+const toString: Function = (() => Object.prototype.toString)();
 
 export interface NodeStyleEventEmitter {
   addListener: (eventName: string | symbol, handler: NodeEventHandler) => this;

src/internal/observable/fromEvent.ts

@@ -8,4 +8,4 @@ declare global {
 }
 
 /** Symbol.observable or a string "@@observable". Used for interop */
-export const observable = typeof Symbol === 'function' && Symbol.observable || '@@observable';
+export const observable = (() => typeof Symbol === 'function' && Symbol.observable || '@@observable')();

src/internal/symbol/observable.ts

@@ -1,8 +1,8 @@
 /** @deprecated do not use, this is no longer checked by RxJS internals */
-export const rxSubscriber =
+export const rxSubscriber = (() =>
   typeof Symbol === 'function'
     ? Symbol('rxSubscriber')
-    : '@@rxSubscriber_' + Math.random();
+    : '@@rxSubscriber_' + Math.random())();
 
 /**
  * @deprecated use rxSubscriber instead

src/internal/symbol/rxSubscriber.ts

@@ -5,14 +5,18 @@ export interface ArgumentOutOfRangeErrorCtor {
   new(): ArgumentOutOfRangeError;
 }
 
-function ArgumentOutOfRangeErrorImpl(this: any) {
-  Error.call(this);
-  this.message = 'argument out of range';
-  this.name = 'ArgumentOutOfRangeError';
-  return this;
-}
+const ArgumentOutOfRangeErrorImpl = (() => {
+  function ArgumentOutOfRangeErrorImpl(this: any) {
+    Error.call(this);
+    this.message = 'argument out of range';
+    this.name = 'ArgumentOutOfRangeError';
+    return this;
+  }
+
+  ArgumentOutOfRangeErrorImpl.prototype = Object.create(Error.prototype);
 
-ArgumentOutOfRangeErrorImpl.prototype = Object.create(Error.prototype);
+  return ArgumentOutOfRangeErrorImpl;
+})();
 
 /**
  * An error thrown when an element was queried at a certain index of an

src/internal/util/ArgumentOutOfRangeError.ts

@@ -5,14 +5,18 @@ export interface EmptyErrorCtor {
   new(): EmptyError;
 }
 
-function EmptyErrorImpl(this: any) {
-  Error.call(this);
-  this.message = 'no elements in sequence';
-  this.name = 'EmptyError';
-  return this;
-}
+const EmptyErrorImpl = (() => {
+  function EmptyErrorImpl(this: any) {
+    Error.call(this);
+    this.message = 'no elements in sequence';
+    this.name = 'EmptyError';
+    return this;
+  }
+
+  EmptyErrorImpl.prototype = Object.create(Error.prototype);
 
-EmptyErrorImpl.prototype = Object.create(Error.prototype);
+  return EmptyErrorImpl;
+})();
 
 /**
  * An error thrown when an Observable or a sequence was queried but has no

src/internal/util/EmptyError.ts

@@ -5,14 +5,18 @@ export interface ObjectUnsubscribedErrorCtor {
   new(): ObjectUnsubscribedError;
 }
 
-function ObjectUnsubscribedErrorImpl(this: any) {
-  Error.call(this);
-  this.message = 'object unsubscribed';
-  this.name = 'ObjectUnsubscribedError';
-  return this;
-}
+const ObjectUnsubscribedErrorImpl = (() => {
+  function ObjectUnsubscribedErrorImpl(this: any) {
+    Error.call(this);
+    this.message = 'object unsubscribed';
+    this.name = 'ObjectUnsubscribedError';
+    return this;
+  }
+
+  ObjectUnsubscribedErrorImpl.prototype = Object.create(Error.prototype);
 
-ObjectUnsubscribedErrorImpl.prototype = Object.create(Error.prototype);
+  return ObjectUnsubscribedErrorImpl;
+})();
 
 /**
  * An error thrown when an action is invalid because the object has been

src/internal/util/ObjectUnsubscribedError.ts

@@ -5,14 +5,18 @@ export interface TimeoutErrorCtor {
   new(): TimeoutError;
 }
 
-function TimeoutErrorImpl(this: any) {
-  Error.call(this);
-  this.message = 'Timeout has occurred';
-  this.name = 'TimeoutError';
-  return this;
-}
+const TimeoutErrorImpl = (() => {
+  function TimeoutErrorImpl(this: any) {
+    Error.call(this);
+    this.message = 'Timeout has occurred';
+    this.name = 'TimeoutError';
+    return this;
+  }
+
+  TimeoutErrorImpl.prototype = Object.create(Error.prototype);
 
-TimeoutErrorImpl.prototype = Object.create(Error.prototype);
+  return TimeoutErrorImpl;
+})();
 
 /**
  * An error thrown when duetime elapses.

src/internal/util/TimeoutError.ts

@@ -6,17 +6,21 @@ export interface UnsubscriptionErrorCtor {
   new(errors: any[]): UnsubscriptionError;
 }
 
-function UnsubscriptionErrorImpl(this: any, errors: any[]) {
-  Error.call(this);
-  this.message = errors ?
-  `${errors.length} errors occurred during unsubscription:
+const UnsubscriptionErrorImpl = (() => {
+  function UnsubscriptionErrorImpl(this: any, errors: any[]) {
+    Error.call(this);
+    this.message = errors ?
+      `${errors.length} errors occurred during unsubscription:
 ${errors.map((err, i) => `${i + 1}) ${err.toString()}`).join('\n  ')}` : '';
-  this.name = 'UnsubscriptionError';
-  this.errors = errors;
-  return this;
-}
+    this.name = 'UnsubscriptionError';
+    this.errors = errors;
+    return this;
+  }
+
+  UnsubscriptionErrorImpl.prototype = Object.create(Error.prototype);
 
-UnsubscriptionErrorImpl.prototype = Object.create(Error.prototype);
+  return UnsubscriptionErrorImpl;
+})();
 
 /**
  * An error thrown when one or more errors have occurred during the

src/internal/util/UnsubscriptionError.ts

@@ -1 +1 @@
-export const isArray = Array.isArray || (<T>(x: any): x is T[] => x && typeof x.length === 'number');
+export const isArray = (() => Array.isArray || (<T>(x: any): x is T[] => x && typeof x.length === 'number'))();

src/internal/util/isArray.ts

@@ -22,8 +22,6 @@
     "no-var-keyword": true,
     "no-empty": true,
     "no-unused-expression-chai": true,
-    "no-unused-variable": true,
-    "no-use-before-declare": true,
     "no-var-requires": true,
     "no-require-imports": true,
     "one-line": [
@@ -59,9 +57,21 @@
       "check-operator",
       "check-separator",
       "check-type"
+    ],
+    "no-toplevel-property-access": [
+      true,
+      "src/index.ts",
+      "src/ajax/",
+      "src/fetch/",
+      "src/internal/",
+      "src/internal-compatibility/",
+      "src/operators/",
+      "src/testing/",
+      "src/webSocket/"
     ]
   },
   "rulesDirectory": [
-    "tslint-no-unused-expression-chai"
+    "tslint-no-unused-expression-chai",
+    "node_modules/tslint-no-toplevel-property-access/rules"
   ]
 }