Skip to content

Commit 8412c73

Browse files
cartantcartant
authored
fix(fromEvent): match targets properly; fix result selector type (#6208)
* test(fromEvent): make dtslint tests fail * fix(fromEvent): use methods not props * refactor: remove redundant optional qualifiers * fix: result selector and target cannot both use T * chore: update api_guardian
1 parent 1d96831 commit 8412c73

File tree

3 files changed

+57
-26
lines changed

3 files changed

+57
-26
lines changed

‎api_guard/dist/types/index.d.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ export declare function from<O extends ObservableInput<any>>(input: O): Observab
142142
export declare function from<O extends ObservableInput<any>>(input: O, scheduler: SchedulerLike): Observable<ObservedValueOf<O>>;
143143

144144
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string): Observable<T>;
145-
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, resultSelector?: (...args: any[]) => T): Observable<T>;
146-
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options?: EventListenerOptions): Observable<T>;
147-
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options: EventListenerOptions, resultSelector: (...args: any[]) => T): Observable<T>;
145+
export declare function fromEvent<T>(target: FromEventTarget<any>, eventName: string, resultSelector: (...args: any[]) => T): Observable<T>;
146+
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options: EventListenerOptions): Observable<T>;
147+
export declare function fromEvent<T>(target: FromEventTarget<any>, eventName: string, options: EventListenerOptions, resultSelector: (...args: any[]) => T): Observable<T>;
148148

149149
export declare function fromEventPattern<T>(addHandler: (handler: NodeEventHandler) => any, removeHandler?: (handler: NodeEventHandler, signal?: any) => void): Observable<T>;
150150
export declare function fromEventPattern<T>(addHandler: (handler: NodeEventHandler) => any, removeHandler?: (handler: NodeEventHandler, signal?: any) => void, resultSelector?: (...args: any[]) => T): Observable<T>;

‎spec-dtslint/observables/fromEvent-spec.ts

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,75 @@
11
import { fromEvent } from 'rxjs';
2-
import { JQueryStyleEventEmitter } from '../../src/internal/observable/fromEvent';
3-
import { A, B } from '../helpers';
2+
import {
3+
HasEventTargetAddRemove,
4+
NodeStyleEventEmitter,
5+
NodeCompatibleEventEmitter,
6+
JQueryStyleEventEmitter
7+
} from '../../src/internal/observable/fromEvent';
8+
import { B } from '../helpers';
49

510
declare const eventTargetSource: EventTarget;
611

712
it('should support an event target source', () => {
13+
const source: HasEventTargetAddRemove<Event> = eventTargetSource;
814
const a = fromEvent(eventTargetSource, "click"); // $ExpectType Observable<Event>
915
});
1016

17+
it('should support an event target source result selector', () => {
18+
const a = fromEvent(eventTargetSource, "click", () => "clunk"); // $ExpectType Observable<string>
19+
});
20+
1121
declare const documentSource: HTMLDocument;
1222

1323
it('should support a document source', () => {
24+
const source: HasEventTargetAddRemove<Event> = documentSource;
1425
const a = fromEvent(documentSource, "click"); // $ExpectType Observable<Event>
1526
});
1627

17-
interface NodeStyleSource {
18-
addListener: (eventName: string | symbol, handler: (...args: any[]) => void) => this;
19-
removeListener: (eventName: string | symbol, handler: (...args: any[]) => void) => this;
20-
};
28+
it('should support a document source result selector', () => {
29+
const a = fromEvent(documentSource, "click", () => "clunk"); // $ExpectType Observable<string>
30+
});
2131

22-
declare const nodeStyleSource : NodeStyleSource;
32+
// Pick the parts that will match NodeStyleEventEmitter. If this isn't done, it
33+
// will match JQueryStyleEventEmitter - because of the `on` and `off` methods -
34+
// despite the latter being declared last in the EventTargetLike union.
35+
declare const nodeStyleSource: Pick<typeof process, 'addListener' | 'removeListener'>;
2336

2437
it('should support a node-style source', () => {
25-
const a = fromEvent(nodeStyleSource, "something"); // $ExpectType Observable<unknown>
26-
const b = fromEvent<B>(nodeStyleSource, "something"); // $ExpectType Observable<B>
38+
const source: NodeStyleEventEmitter = nodeStyleSource;
39+
const a = fromEvent(nodeStyleSource, "exit"); // $ExpectType Observable<unknown>
40+
const b = fromEvent<B>(nodeStyleSource, "exit"); // $ExpectType Observable<B>
41+
});
42+
43+
it('should support a node-style source result selector', () => {
44+
const a = fromEvent(nodeStyleSource, "exit", () => "bye"); // $ExpectType Observable<string>
2745
});
2846

29-
declare const nodeCompatibleSource: {
30-
addListener: (eventName: string, handler: (...args: any[]) => void) => void;
31-
removeListener: (eventName: string, handler: (...args: any[]) => void) => void;
47+
const nodeCompatibleSource = {
48+
addListener(eventName: "something", handler: () => void) {},
49+
removeListener(eventName: "something", handler: () => void) {}
3250
};
3351

3452
it('should support a node-compatible source', () => {
53+
const source: NodeCompatibleEventEmitter = nodeCompatibleSource;
3554
const a = fromEvent(nodeCompatibleSource, "something"); // $ExpectType Observable<unknown>
3655
const b = fromEvent<B>(nodeCompatibleSource, "something"); // $ExpectType Observable<B>
3756
});
3857

39-
declare const jQueryStyleSource: JQueryStyleEventEmitter<A, B>;
58+
it('should support a node-compatible source result selector', () => {
59+
const a = fromEvent(nodeCompatibleSource, "something", () => "something else"); // $ExpectType Observable<string>
60+
});
61+
62+
const jQueryStyleSource = {
63+
on(eventName: "something", handler: (this: any, b: B) => any) {},
64+
off(eventName: "something", handler: (this: any, b: B) => any) {}
65+
};
4066

4167
it('should support a jQuery-style source', () => {
68+
const source: JQueryStyleEventEmitter<any, any> = jQueryStyleSource;
4269
const a = fromEvent(jQueryStyleSource, "something"); // $ExpectType Observable<B>
4370
const b = fromEvent<B>(jQueryStyleSource, "something"); // $ExpectType Observable<B>
4471
});
72+
73+
it('should support a jQuery-style source result selector', () => {
74+
const a = fromEvent(jQueryStyleSource, "something", () => "something else"); // $ExpectType Observable<string>
75+
});

‎src/internal/observable/fromEvent.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ const eventTargetMethods = ['addEventListener', 'removeEventListener'] as const;
1111
const jqueryMethods = ['on', 'off'] as const;
1212

1313
export interface NodeStyleEventEmitter {
14-
addListener: (eventName: string | symbol, handler: NodeEventHandler) => this;
15-
removeListener: (eventName: string | symbol, handler: NodeEventHandler) => this;
14+
addListener(eventName: string | symbol, handler: NodeEventHandler): this;
15+
removeListener(eventName: string | symbol, handler: NodeEventHandler): this;
1616
}
1717

1818
export type NodeEventHandler = (...args: any[]) => void;
@@ -21,15 +21,15 @@ export type NodeEventHandler = (...args: any[]) => void;
2121
// not use the same arguments or return EventEmitter values
2222
// such as React Native
2323
export interface NodeCompatibleEventEmitter {
24-
addListener: (eventName: string, handler: NodeEventHandler) => void | {};
25-
removeListener: (eventName: string, handler: NodeEventHandler) => void | {};
24+
addListener(eventName: string, handler: NodeEventHandler): void | {};
25+
removeListener(eventName: string, handler: NodeEventHandler): void | {};
2626
}
2727

2828
// Use handler types like those in @types/jquery. See:
2929
// https://github.com/DefinitelyTyped/DefinitelyTyped/blob/847731ba1d7fa6db6b911c0e43aa0afe596e7723/types/jquery/misc.d.ts#L6395
3030
export interface JQueryStyleEventEmitter<TContext, T> {
31-
on: (eventName: string, handler: (this: TContext, t: T, ...args: any[]) => any) => void;
32-
off: (eventName: string, handler: (this: TContext, t: T, ...args: any[]) => any) => void;
31+
on(eventName: string, handler: (this: TContext, t: T, ...args: any[]) => any): void;
32+
off(eventName: string, handler: (this: TContext, t: T, ...args: any[]) => any): void;
3333
}
3434

3535
export interface EventListenerObject<E> {
@@ -70,11 +70,11 @@ export interface AddEventListenerOptions extends EventListenerOptions {
7070

7171
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string): Observable<T>;
7272
/** @deprecated resultSelector no longer supported, pipe to map instead */
73-
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string, resultSelector?: (...args: any[]) => T): Observable<T>;
74-
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options?: EventListenerOptions): Observable<T>;
73+
export function fromEvent<T>(target: FromEventTarget<any>, eventName: string, resultSelector: (...args: any[]) => T): Observable<T>;
74+
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options: EventListenerOptions): Observable<T>;
7575
/** @deprecated resultSelector no longer supported, pipe to map instead */
7676
export function fromEvent<T>(
77-
target: FromEventTarget<T>,
77+
target: FromEventTarget<any>,
7878
eventName: string,
7979
options: EventListenerOptions,
8080
resultSelector: (...args: any[]) => T
@@ -211,7 +211,7 @@ export function fromEvent<T>(
211211
}
212212
if (resultSelector) {
213213
// DEPRECATED PATH
214-
return fromEvent<T>(target, eventName, options as EventListenerOptions | undefined).pipe(mapOneOrManyArgs(resultSelector));
214+
return fromEvent<T>(target, eventName, options as EventListenerOptions).pipe(mapOneOrManyArgs(resultSelector));
215215
}
216216

217217
// Figure out our add and remove methods. In order to do this,

0 commit comments

Comments
 (0)