Skip to content

Commit

Permalink
fix(Symbol.observable): properly defined as a unique symbol. (#5874)
Browse files Browse the repository at this point in the history
Resolves #5861
Resolves #4415

BREAKING CHANGE: `rxjs@7` is only compatible with `@types/node@14.14.3` or higher and `symbol-observable@3.0.0` and heigher. Older versions of `@types/node` incorrectly defined `Symbol.observable` and will be in conflict with `rxjs` and `symbol-observable@3.0.0`.
  • Loading branch information
benlesh authored Nov 2, 2020
1 parent 3340713 commit 374138e
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 17 deletions.
2 changes: 1 addition & 1 deletion api_guard/dist/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export interface ObjectUnsubscribedError extends Error {

export declare const ObjectUnsubscribedError: ObjectUnsubscribedErrorCtor;

export declare const observable: string | symbol;
export declare const observable: string | SymbolConstructor["observable"];

export declare class Observable<T> implements Subscribable<T> {
protected operator: Operator<any, T> | undefined;
Expand Down
6 changes: 3 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
"@types/chai": "^4.2.11",
"@types/lodash": "4.14.102",
"@types/mocha": "^7.0.2",
"@types/node": "^12.12.5",
"@types/node": "^14.14.6",
"@types/shelljs": "^0.8.8",
"@types/sinon": "4.1.3",
"@types/sinon-chai": "2.7.29",
Expand Down
11 changes: 2 additions & 9 deletions src/internal/symbol/observable.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
/** 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 {
readonly observable: symbol;
}
}
/** @prettier */

/** 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')();
22 changes: 19 additions & 3 deletions src/internal/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,22 @@
import { Observable } from './Observable';
import { Subscription } from './Subscription';

/**
* NOTE: This will add Symbol.observable globally for all TypeScript users,
* however, we are no longer polyfilling Symbol.observable. Note that this will be at
* odds with older version of @types/node and symbol-observable which incorrectly define
* `Symbol.observable` as `symbol` rather than `unique symbol`. "What about not defining
* this non-standard symbol at all?" you might ask... Well, that ship has sailed. There are
* dozens of libraries using this symbol now and many of them are quite popular.
* So here we are, and it's probably my fault. Sorry, "the web", if I have hurt you,
* the world just needed a standard way to provide interop for these types. -Ben
*/
declare global {
interface SymbolConstructor {
readonly observable: unique symbol;
}
}

/** OPERATOR INTERFACES */

export interface UnaryFunction<T, R> {
Expand Down Expand Up @@ -217,12 +233,12 @@ export type ObservedValueTupleFromArray<X> = { [K in keyof X]: ObservedValueOf<X

/**
* Used to infer types from arguments to functions like {@link forkJoin}.
* So that you can have `forkJoin([Observable<A>, PromiseLike<B>]): Observable<[A, B]>`
* So that you can have `forkJoin([Observable<A>, PromiseLike<B>]): Observable<[A, B]>`
* et al.
*/
export type ObservableInputTuple<T> = {
[K in keyof T]: ObservableInput<T[K]>
}
[K in keyof T]: ObservableInput<T[K]>;
};

/**
* Constructs a new tuple with the specified type at the head.
Expand Down

0 comments on commit 374138e

Please sign in to comment.