Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fromEvent): allow fromEvent to handle symbols as event names #7339

Merged
merged 1 commit into from Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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