Skip to content

Commit

Permalink
fix(fromEvent): allow fromEvent to handle symbols as event names
Browse files Browse the repository at this point in the history
Allow passing a value of type `symbol` to `eventName` if
a `NodeStyleEventEmitter` is passed to `source`, according to
[the documentation](https://nodejs.org/api/events.html#emitteraddlistenereventname-listener).

Closes #7338
  • Loading branch information
demensky committed Oct 28, 2023
1 parent 0bd47ea commit a9904bf
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 6 deletions.
7 changes: 7 additions & 0 deletions packages/rxjs/spec-dtslint/observables/fromEvent-spec.ts
Expand Up @@ -56,6 +56,13 @@ it('should support a node-style source', () => {
const b = fromEvent<B>(nodeStyleSource, "exit"); // $ExpectType Observable<B>
});

it('should support a node-style source and symbol eventName', () => {
const SYMBOL_EVENT = Symbol();
const source: NodeStyleEventEmitter = nodeStyleSource;
const a = fromEvent(nodeStyleSource, SYMBOL_EVENT); // $ExpectType Observable<unknown>
const b = fromEvent<B>(nodeStyleSource, SYMBOL_EVENT); // $ExpectType Observable<B>
});

it('should deprecate explicit type parameters for a node-style source', () => {
const source: NodeStyleEventEmitter = nodeStyleSource;
const a = fromEvent(nodeStyleSource, "exit"); // $ExpectNoDeprecation
Expand Down
33 changes: 33 additions & 0 deletions packages/rxjs/spec/observables/fromEvent-spec.ts
Expand Up @@ -121,6 +121,39 @@ describe('fromEvent', () => {
expect(offHandler).to.equal(onHandler);
});

it('should pass symbol to "addListener" and "removeListener"', () => {
let onEventName;
let onHandler;
let offEventName;
let offHandler;

const SYMBOL_EVENT = Symbol();

const obj = {
addListener(a: string | symbol, b: (...args: any[]) => void) {
onEventName = a;
onHandler = b;
return this;
},
removeListener(a: string | symbol, b: (...args: any[]) => void) {
offEventName = a;
offHandler = b;
return this;
},
};

const subscription = fromEvent(obj, SYMBOL_EVENT).subscribe(() => {
//noop
});

subscription.unsubscribe();

expect(onEventName).to.equal(SYMBOL_EVENT);
expect(typeof onHandler).to.equal('function');
expect(offEventName).to.equal(onEventName);
expect(offHandler).to.equal(onHandler);
});

it('should setup an event observable on objects with "addListener" and "removeListener" returning nothing', () => {
let onEventName;
let onHandler;
Expand Down
21 changes: 15 additions & 6 deletions packages/rxjs/src/internal/observable/fromEvent.ts
Expand Up @@ -77,12 +77,15 @@ export function fromEvent<T, R>(
resultSelector: (event: T) => R
): Observable<R>;

export function fromEvent(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string): Observable<unknown>;
export function fromEvent(
target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>,
eventName: string | symbol
): Observable<unknown>;
/** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */
export function fromEvent<T>(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string): Observable<T>;
export function fromEvent<T>(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string | symbol): Observable<T>;
export function fromEvent<R>(
target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>,
eventName: string,
eventName: string | symbol,
resultSelector: (...args: any[]) => R
): Observable<R>;

Expand Down Expand Up @@ -238,7 +241,7 @@ export function fromEvent<T, R>(
*/
export function fromEvent<T>(
target: any,
eventName: string,
eventName: string | symbol,
options?: EventListenerOptions | ((...args: any[]) => T),
resultSelector?: (...args: any[]) => T
): Observable<T> {
Expand All @@ -248,7 +251,7 @@ export function fromEvent<T>(
}

if (resultSelector) {
return fromEvent<T>(target, eventName, options as EventListenerOptions).pipe(mapOneOrManyArgs(resultSelector));
return fromEvent<T>(target, eventName as string, options as EventListenerOptions).pipe(mapOneOrManyArgs(resultSelector));
}

const isValidTarget = isNodeStyleEventEmitter(target) || isJQueryStyleEventEmitter(target) || isEventTarget(target);
Expand All @@ -274,7 +277,13 @@ export function fromEvent<T>(
});
}

function doSubscribe(handler: (...args: any[]) => void, subscriber: Subscriber<any>, subTarget: any, eventName: string, options: any) {
function doSubscribe(
handler: (...args: any[]) => void,
subscriber: Subscriber<any>,
subTarget: any,
eventName: string | symbol,
options: any
) {
const [addMethod, removeMethod] = getRegistryMethodNames(subTarget);
if (!addMethod || !removeMethod) {
throw new TypeError('Invalid event target');
Expand Down

0 comments on commit a9904bf

Please sign in to comment.