Skip to content

Commit

Permalink
fix(fromEvent): match targets properly; fix result selector type (#6208)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cartant committed Apr 9, 2021
1 parent 1d96831 commit 8412c73
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 26 deletions.
6 changes: 3 additions & 3 deletions api_guard/dist/types/index.d.ts
Expand Up @@ -142,9 +142,9 @@ export declare function from<O extends ObservableInput<any>>(input: O): Observab
export declare function from<O extends ObservableInput<any>>(input: O, scheduler: SchedulerLike): Observable<ObservedValueOf<O>>;

export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string): Observable<T>;
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, resultSelector?: (...args: any[]) => T): Observable<T>;
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options?: EventListenerOptions): Observable<T>;
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options: EventListenerOptions, resultSelector: (...args: any[]) => T): Observable<T>;
export declare function fromEvent<T>(target: FromEventTarget<any>, eventName: string, resultSelector: (...args: any[]) => T): Observable<T>;
export declare function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options: EventListenerOptions): Observable<T>;
export declare function fromEvent<T>(target: FromEventTarget<any>, eventName: string, options: EventListenerOptions, resultSelector: (...args: any[]) => T): Observable<T>;

export declare function fromEventPattern<T>(addHandler: (handler: NodeEventHandler) => any, removeHandler?: (handler: NodeEventHandler, signal?: any) => void): Observable<T>;
export declare function fromEventPattern<T>(addHandler: (handler: NodeEventHandler) => any, removeHandler?: (handler: NodeEventHandler, signal?: any) => void, resultSelector?: (...args: any[]) => T): Observable<T>;
Expand Down
57 changes: 44 additions & 13 deletions spec-dtslint/observables/fromEvent-spec.ts
@@ -1,44 +1,75 @@
import { fromEvent } from 'rxjs';
import { JQueryStyleEventEmitter } from '../../src/internal/observable/fromEvent';
import { A, B } from '../helpers';
import {
HasEventTargetAddRemove,
NodeStyleEventEmitter,
NodeCompatibleEventEmitter,
JQueryStyleEventEmitter
} from '../../src/internal/observable/fromEvent';
import { B } from '../helpers';

declare const eventTargetSource: EventTarget;

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

it('should support an event target source result selector', () => {
const a = fromEvent(eventTargetSource, "click", () => "clunk"); // $ExpectType Observable<string>
});

declare const documentSource: HTMLDocument;

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

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

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

it('should support a node-style source', () => {
const a = fromEvent(nodeStyleSource, "something"); // $ExpectType Observable<unknown>
const b = fromEvent<B>(nodeStyleSource, "something"); // $ExpectType Observable<B>
const source: NodeStyleEventEmitter = nodeStyleSource;
const a = fromEvent(nodeStyleSource, "exit"); // $ExpectType Observable<unknown>
const b = fromEvent<B>(nodeStyleSource, "exit"); // $ExpectType Observable<B>
});

it('should support a node-style source result selector', () => {
const a = fromEvent(nodeStyleSource, "exit", () => "bye"); // $ExpectType Observable<string>
});

declare const nodeCompatibleSource: {
addListener: (eventName: string, handler: (...args: any[]) => void) => void;
removeListener: (eventName: string, handler: (...args: any[]) => void) => void;
const nodeCompatibleSource = {
addListener(eventName: "something", handler: () => void) {},
removeListener(eventName: "something", handler: () => void) {}
};

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

declare const jQueryStyleSource: JQueryStyleEventEmitter<A, B>;
it('should support a node-compatible source result selector', () => {
const a = fromEvent(nodeCompatibleSource, "something", () => "something else"); // $ExpectType Observable<string>
});

const jQueryStyleSource = {
on(eventName: "something", handler: (this: any, b: B) => any) {},
off(eventName: "something", handler: (this: any, b: B) => any) {}
};

it('should support a jQuery-style source', () => {
const source: JQueryStyleEventEmitter<any, any> = jQueryStyleSource;
const a = fromEvent(jQueryStyleSource, "something"); // $ExpectType Observable<B>
const b = fromEvent<B>(jQueryStyleSource, "something"); // $ExpectType Observable<B>
});

it('should support a jQuery-style source result selector', () => {
const a = fromEvent(jQueryStyleSource, "something", () => "something else"); // $ExpectType Observable<string>
});
20 changes: 10 additions & 10 deletions src/internal/observable/fromEvent.ts
Expand Up @@ -11,8 +11,8 @@ const eventTargetMethods = ['addEventListener', 'removeEventListener'] as const;
const jqueryMethods = ['on', 'off'] as const;

export interface NodeStyleEventEmitter {
addListener: (eventName: string | symbol, handler: NodeEventHandler) => this;
removeListener: (eventName: string | symbol, handler: NodeEventHandler) => this;
addListener(eventName: string | symbol, handler: NodeEventHandler): this;
removeListener(eventName: string | symbol, handler: NodeEventHandler): this;
}

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

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

export interface EventListenerObject<E> {
Expand Down Expand Up @@ -70,11 +70,11 @@ export interface AddEventListenerOptions extends EventListenerOptions {

export function fromEvent<T>(target: FromEventTarget<T>, eventName: string): Observable<T>;
/** @deprecated resultSelector no longer supported, pipe to map instead */
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string, resultSelector?: (...args: any[]) => T): Observable<T>;
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options?: EventListenerOptions): Observable<T>;
export function fromEvent<T>(target: FromEventTarget<any>, eventName: string, resultSelector: (...args: any[]) => T): Observable<T>;
export function fromEvent<T>(target: FromEventTarget<T>, eventName: string, options: EventListenerOptions): Observable<T>;
/** @deprecated resultSelector no longer supported, pipe to map instead */
export function fromEvent<T>(
target: FromEventTarget<T>,
target: FromEventTarget<any>,
eventName: string,
options: EventListenerOptions,
resultSelector: (...args: any[]) => T
Expand Down Expand Up @@ -211,7 +211,7 @@ export function fromEvent<T>(
}
if (resultSelector) {
// DEPRECATED PATH
return fromEvent<T>(target, eventName, options as EventListenerOptions | undefined).pipe(mapOneOrManyArgs(resultSelector));
return fromEvent<T>(target, eventName, options as EventListenerOptions).pipe(mapOneOrManyArgs(resultSelector));
}

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

0 comments on commit 8412c73

Please sign in to comment.