From dc575ba00260301d8e83eb8fe91ee9116534a959 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Tue, 6 Mar 2018 15:56:20 -0800 Subject: [PATCH] feat(Symbol.observable): is no longer polyfilled BREAKING CHANGE: RxJS will no longer be polyfilling Symbol.observable. That should be done by an actual polyfill library. This is to prevent duplication of code, and also to prevent having modules with side-effects in rxjs. --- spec/helpers/polyfills.ts | 11 ++++++ spec/support/coverage.opts | 3 +- spec/support/default.opts | 1 + spec/support/tests2png.opts | 3 +- spec/support/webpack.mocha.config.js | 5 +-- spec/symbol/observable-polyfilled-spec.ts | 10 ------ spec/symbol/observable-spec.ts | 16 --------- src/internal/symbol/observable.ts | 41 ++++++++++------------- src/internal/types.ts | 3 +- 9 files changed, 39 insertions(+), 54 deletions(-) create mode 100644 spec/helpers/polyfills.ts delete mode 100644 spec/symbol/observable-polyfilled-spec.ts delete mode 100644 spec/symbol/observable-spec.ts diff --git a/spec/helpers/polyfills.ts b/spec/helpers/polyfills.ts new file mode 100644 index 00000000000..1d5a5e2df45 --- /dev/null +++ b/spec/helpers/polyfills.ts @@ -0,0 +1,11 @@ +if (typeof Symbol !== 'function') { + let id = 0; + const symbolFn: any = (description: string) => + `Symbol_${id++} ${description} (RxJS Testing Polyfill)`; + + Symbol = symbolFn; +} + +if (!(Symbol as any).observable) { + (Symbol as any).observable = Symbol('Symbol.observable polyfill from RxJS Testing'); +} diff --git a/spec/support/coverage.opts b/spec/support/coverage.opts index dd16ba80489..2c874036c19 100644 --- a/spec/support/coverage.opts +++ b/spec/support/coverage.opts @@ -1,3 +1,4 @@ +--require spec/helpers/polyfills.ts --require spec/helpers/testScheduler-ui.ts --ui spec/helpers/testScheduler-ui.ts @@ -6,4 +7,4 @@ --globals WebSocket,FormData,XDomainRequest,ActiveXObject --recursive ---timeout 5000 \ No newline at end of file +--timeout 5000 diff --git a/spec/support/default.opts b/spec/support/default.opts index 10a62777584..6445aa55ab4 100644 --- a/spec/support/default.opts +++ b/spec/support/default.opts @@ -1,3 +1,4 @@ +--require .out/spec/helpers/polyfills.ts --require .out/spec/helpers/testScheduler-ui.js --ui .out/spec/helpers/testScheduler-ui.js diff --git a/spec/support/tests2png.opts b/spec/support/tests2png.opts index df3515ca6cf..fc7c2293bf4 100644 --- a/spec/support/tests2png.opts +++ b/spec/support/tests2png.opts @@ -1,3 +1,4 @@ +--require spec-js/helpers/polyfills.ts --require source-map-support/register --require spec-js/helpers/tests2png/diagram-test-runner.js --require spec-js/helpers/testScheduler-ui.js @@ -6,4 +7,4 @@ --reporter dot --recursive ---timeout 5000 \ No newline at end of file +--timeout 5000 diff --git a/spec/support/webpack.mocha.config.js b/spec/support/webpack.mocha.config.js index dc4ac15129a..c9e2d6b6241 100644 --- a/spec/support/webpack.mocha.config.js +++ b/spec/support/webpack.mocha.config.js @@ -3,7 +3,7 @@ var path = require('path'); var glob = require('glob'); var webpack = require('webpack'); -var globPattern = 'spec-js/**/!(mocha.sauce.gruntfile|mocha.sauce.runner|webpack.mocha.config|painter|diagram-test-runner|testScheduler-ui).js'; +var globPattern = 'spec-js/**/!(mocha.sauce.gruntfile|mocha.sauce.runner|webpack.mocha.config|painter|diagram-test-runner|polyfills|testScheduler-ui).js'; var files = _.map(glob.sync(globPattern), function (x) { return path.resolve('./', x); }); @@ -18,6 +18,7 @@ module.exports = { }, entry: { + 'browser.polyfills': './spec-js/helpers/polyfills.js', 'browser.testscheduler': './spec-js/helpers/testScheduler-ui.js', 'browser.spec': files }, @@ -31,4 +32,4 @@ module.exports = { new webpack.optimize.CommonsChunkPlugin('browser.common.js'), new webpack.IgnorePlugin(/^mocha$/) ] -}; \ No newline at end of file +}; diff --git a/spec/symbol/observable-polyfilled-spec.ts b/spec/symbol/observable-polyfilled-spec.ts deleted file mode 100644 index 79891d3b5fd..00000000000 --- a/spec/symbol/observable-polyfilled-spec.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { expect } from 'chai'; - -import { getSymbolObservable } from '../../src/internal/symbol/observable'; - -describe('observable symbol', () => { - it('should exist in the proper form when Symbol does not exist', () => { - let $$observable = getSymbolObservable({Symbol: undefined}); - expect($$observable).to.equal('@@observable'); - }); -}); diff --git a/spec/symbol/observable-spec.ts b/spec/symbol/observable-spec.ts deleted file mode 100644 index 62b789be1af..00000000000 --- a/spec/symbol/observable-spec.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { expect } from 'chai'; -import $$symbolObservable from 'symbol-observable'; - -import { root } from '../../src/internal/util/root'; -import { getSymbolObservable } from '../../src/internal/symbol/observable'; - -describe('observable symbol', () => { - it('should exist in the proper form', () => { - let $$observable = getSymbolObservable(root); - if (root.Symbol && root.Symbol.for) { - expect($$observable).to.equal($$symbolObservable); - } else { - expect($$observable).to.equal('@@observable'); - } - }); -}); diff --git a/src/internal/symbol/observable.ts b/src/internal/symbol/observable.ts index 4940e851236..efce82db971 100644 --- a/src/internal/symbol/observable.ts +++ b/src/internal/symbol/observable.ts @@ -1,32 +1,27 @@ import { root } from '../util/root'; -export function getSymbolObservable(context: { Symbol: SymbolConstructor; }): symbol { - let $$observable: symbol; - let Symbol = context.Symbol; - - if (typeof Symbol === 'function') { - if (Symbol.observable) { - $$observable = Symbol.observable; - } else { - $$observable = Symbol('observable'); - Symbol.observable = $$observable; - } - } else { - $$observable = '@@observable'; +/** Symbol.observable addition */ +/* Note: This will add Symbol.observable globally for all TypeScript users, + however, we are no longer polyfilling Symbol.observable */ +declare global { + interface SymbolConstructor { + observable: symbol; } - - return $$observable; } -export const observable = getSymbolObservable(root); +/* NOTE: Warning users that they don't have a Symbol.observable + polyfill. It's not something we'd throw on, because it's not + really *required*, however, they should be warned to give them a + clue as to why they might be seeing errors when trying to convert + ObservableLikes to Observables */ +if (!Symbol || !Symbol.observable) { + console.warn('RxJS: Symbol.observable does not exist'); +} /** - * @deprecated use observable instead + * @deprecated use {@link observable} */ -export const $$observable = observable; +export const $$observable = Symbol && Symbol.observable || '@@observable'; -declare global { - interface SymbolConstructor { - observable: symbol; - } -} \ No newline at end of file +/** Symbol.observable or a string "@@observable". Used for interop */ +export const observable = $$observable; diff --git a/src/internal/types.ts b/src/internal/types.ts index e9ce758b99a..54c923b4a54 100644 --- a/src/internal/types.ts +++ b/src/internal/types.ts @@ -1,5 +1,6 @@ import { Observable } from './Observable'; import { PartialObserver } from './types'; +import { observable as Symbol_observable } from './symbol/observable'; /** OPERATOR INTERFACES */ @@ -73,4 +74,4 @@ export interface Observer { next: (value: T) => void; error: (err: any) => void; complete: () => void; -} \ No newline at end of file +}