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): infer from Node.js EventEmitter with types #6669

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f4eddaa
fix(fromEvent): add unit test to show the failure of infer string lit…
huan Nov 7, 2021
0ac11dd
fix(fromEvent): add code to fix the failed unit test
huan Nov 7, 2021
bde7c40
fix(fromEvent): make dtslint happy
huan Nov 7, 2021
b069b10
fix(fromEvent): fix code
huan Nov 7, 2021
29910b9
fix(fromEvent): add AnyToUnknown converter
huan Nov 7, 2021
62bced1
fix(fromEvent): fix dtslint spec
huan Nov 7, 2021
f80414f
fix(fromEvent): update api_guard
huan Nov 7, 2021
f25a174
fix(fromEvent): add two event name for multi type testing
huan Nov 7, 2021
62c7582
fix(fromEvent): add helper file with TypeScript overload function wor…
huan Nov 7, 2021
c145cac
fix(fromEvent): add helper functio unit test
huan Nov 7, 2021
a3fa4a7
fix(fromEvent): add util helper for workaround TypeScript limitation
huan Nov 7, 2021
b17c418
fix(fromEvent): clean util function code
huan Nov 8, 2021
ff95a6c
fix(fromEvent): clean fromEvent.ts code to make sure its clean
huan Nov 8, 2021
740abf3
fix(fromEvent): integrate code to fromEvent
huan Nov 8, 2021
42aee0d
fix(fromEvent): code clean & add unit test for AnyToUnknown type caster
huan Nov 8, 2021
f2638fa
fix(fromEvent): code clean
huan Nov 8, 2021
fa79faf
fix(fromEvent): update api guard
huan Nov 8, 2021
d6da834
fix(fromEvent): CI green! code clean
huan Nov 8, 2021
1554c9f
fix(fromEvent): increase overload max number to 9
huan Nov 8, 2021
7b7739e
fix(fromEvent): increase overload max number to 9
huan Nov 8, 2021
88acd54
chore(fromEvent): move types test to dtslint & remove comments
huan Dec 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api_guard/dist/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ export declare function fromEvent<T>(target: HasEventTargetAddRemove<T> | ArrayL
export declare function fromEvent<T, R>(target: HasEventTargetAddRemove<T> | ArrayLike<HasEventTargetAddRemove<T>>, eventName: string, resultSelector: (event: T) => R): Observable<R>;
export declare function fromEvent<T>(target: HasEventTargetAddRemove<T> | ArrayLike<HasEventTargetAddRemove<T>>, eventName: string, options: EventListenerOptions): Observable<T>;
export declare function fromEvent<T, R>(target: HasEventTargetAddRemove<T> | ArrayLike<HasEventTargetAddRemove<T>>, eventName: string, options: EventListenerOptions, resultSelector: (event: T) => R): Observable<R>;
export declare function fromEvent<T extends string, E extends NamedNodeEventEmitter<T>>(target: E | ArrayLike<E>, eventName: T): Observable<NodeEventEmitterDataTypeUnknown<E, T>>;
export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string): Observable<unknown>;
export declare function fromEvent<T>(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string): Observable<T>;
export declare function fromEvent<R>(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string, resultSelector: (...args: any[]) => R): Observable<R>;
Expand Down
21 changes: 21 additions & 0 deletions spec-dtslint/observables/fromEvent-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,24 @@ it('should support a jQuery-style source', () => {
it('should support a jQuery-style source result selector', () => {
const a = fromEvent(jQueryStyleSource, "something", () => "something else"); // $ExpectType Observable<string>
});

/**
* Huan(202111): Correct typing inference for Node.js EventEmitter as `number`
* @see https://github.com/ReactiveX/rxjs/pull/6669
*/
it('should successful inference the first argument from the listener of Node.js EventEmitter', (done) => {
class NodeEventeEmitterTest {
addListener(eventName: 'foo', listener: (foo: false) => void): this
addListener(eventName: 'bar', listener: (bar: boolean) => void): this
addListener(eventName: 'foo' | 'bar', listener: ((foo: false) => void) | ((bar: boolean) => void)): this { return this; }

removeListener(eventName: 'foo', listener: (foo: false) => void ): this
removeListener(eventName: 'bar', listener: (bar: boolean) => void ): this
removeListener(eventName: 'foo' | 'bar', listener: ((foo: false) => void) | ((bar: boolean) => void)): this { return this; }
}

const test = new NodeEventeEmitterTest();

const foo$ = fromEvent(test, 'foo'); // $ExpectType Observable<false>
const bar$ = fromEvent(test, 'bar'); // $ExpectType Observable<boolean>
});
96 changes: 96 additions & 0 deletions spec-dtslint/util/NodeEventEmitterDataType-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import {
NodeEventEmitterNameDataPair,
NodeEventEmitterDataType,
NodeEventEmitterDataTypeUnknown,
AnyToUnknown,
} from '../../src/internal/util/NodeEventEmitterDataType';

it('NodeEventEmitterDataType smoke testing', () => {
const fooEvent = 'fooEvent';
const barEvent = 'barEvent';

type FOO_DATA = typeof fooEvent;
type BAR_DATA = typeof barEvent;

class NodeEventEmitterFixture {
addListener(eventName: 'foo', listener: (foo: FOO_DATA) => void): this
addListener(eventName: 'bar', listener: (bar: BAR_DATA) => void): this
addListener(eventName: 'foo' | 'bar', listener: ((foo: FOO_DATA) => void) | ((bar: BAR_DATA) => void)): this { return this; }

removeListener(eventName: 'foo', listener: (foo: FOO_DATA) => void ): this
removeListener(eventName: 'bar', listener: (bar: BAR_DATA) => void ): this
removeListener(eventName: 'foo' | 'bar', listener: ((foo: FOO_DATA) => void) | ((bar: BAR_DATA) => void)): this { return this; }

/**
* TODO: JQueryStyle compatible in the future
*/
// on(eventName: 'foo', listener: (foo: number) => void): void
// on(eventName: 'bar', listener: (bar: string) => void): void
// on(eventName: 'foo' | 'bar', listener: ((foo: number) => void) | ((bar: string) => void)): void {}

// off(eventName: 'foo', listener: (foo: number) => void ): void
// off(eventName: 'bar', listener: (bar: string) => void ): void
// off(eventName: 'foo' | 'bar', listener: ((foo: number) => void) | ((bar: string) => void)): void {}
}

it('should get emitter name & data types correctly', () => {
// $ExpectType "fooEvent"
type Foo = NodeEventEmitterDataType<NodeEventEmitterFixture, 'foo'>;
// $ExpectType "barEvent"
type Bar = NodeEventEmitterDataType<NodeEventEmitterFixture, 'bar'>;
});

it('should get name & data from NodeEventEmitterNameDataPair', () => {
// type EVENT_PAIR = TypeEventPair<NodeEventEmitterTest['addListener']>
type EVENT_PAIR = NodeEventEmitterNameDataPair<NodeEventEmitterFixture>;

// $ExpectType "foo" | "bar"
type EVENT_NAME = EVENT_PAIR[0];
// $ExpecTType FOO_DATA | BAR_DATA
type EVENT_DATA = EVENT_PAIR[1];
});

it('should get `unknown` for `process` events by NodeEventEmitterDataTypeUnknown', () => {
// $ExpectType unknown
type Exit = NodeEventEmitterDataTypeUnknown<
Pick<
typeof process,
'addListener' | 'removeListener'
>,
'exit'
>;
});

it('should get `never` for `process` events by NodeEventEmitterDataType', () => {
// $ExpectType never
type Exit = NodeEventEmitterDataType<
Pick<
typeof process,
'addListener' | 'removeListener'
>,
'exit'
>
});
});

it('AnyToUnknown smoke testing', () => {
it('should only convert any to unknown', () => {
type T_ANY = AnyToUnknown<any>
type T_UNKNOWN = AnyToUnknown<unknown>

type T_BOOLEAN = AnyToUnknown<boolean>
type T_NULL = AnyToUnknown<null>
type T_OBJ = AnyToUnknown<object>
type T_STRING = AnyToUnknown<string>
type T_UNDEFINED = AnyToUnknown<undefined>
type T_VOID = AnyToUnknown<void>
type T_NEVER = AnyToUnknown<never>

// $ExpectType unknown
type UNKNOWN_TYPE = T_ANY & T_UNKNOWN

type KNOWN_TYPE = T_VOID | T_BOOLEAN | T_STRING | T_UNDEFINED | T_NULL | T_OBJ | T_NEVER
// $ExpectType true
type T = unknown extends KNOWN_TYPE ? never : true
});
});
3 changes: 2 additions & 1 deletion spec/observables/fromEvent-spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from 'chai';
import { expectObservable } from '../helpers/marble-testing';
import { fromEvent, NEVER, timer } from 'rxjs';
import { fromEvent, NEVER, Observable, timer } from 'rxjs';
import { mapTo, take, concat } from 'rxjs/operators';
import { TestScheduler } from 'rxjs/testing';

Expand Down Expand Up @@ -432,4 +432,5 @@ describe('fromEvent', () => {
expect(nodeList[0]._removeEventListenerArgs).to.deep.equal(nodeList[0]._addEventListenerArgs);
expect(nodeList[1]._removeEventListenerArgs).to.deep.equal(nodeList[1]._addEventListenerArgs);
});

});
12 changes: 12 additions & 0 deletions src/internal/observable/fromEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { mergeMap } from '../operators/mergeMap';
import { isArrayLike } from '../util/isArrayLike';
import { isFunction } from '../util/isFunction';
import { mapOneOrManyArgs } from '../util/mapOneOrManyArgs';
import { NamedNodeEventEmitter, NodeEventEmitterDataTypeUnknown } from '../util/NodeEventEmitterDataType';

// These constants are used to create handler registry functions using array mapping below.
const nodeEventEmitterMethods = ['addListener', 'removeListener'] as const;
Expand Down Expand Up @@ -78,6 +79,17 @@ export function fromEvent<T, R>(
resultSelector: (event: T) => R
): Observable<R>;

/**
* Automatically infer overloaded Node.js EventEmitter event types:
* get event data type (`typeof args[0]` of the listener) by event name.
*
* @see https://github.com/ReactiveX/rxjs/pull/6669
*/
export function fromEvent<T extends string, E extends NamedNodeEventEmitter<T>>(
target: E | ArrayLike<E>,
eventName: T
): Observable<NodeEventEmitterDataTypeUnknown<E, T>>;

export function fromEvent(target: NodeStyleEventEmitter | ArrayLike<NodeStyleEventEmitter>, eventName: string): 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>;
Expand Down
220 changes: 220 additions & 0 deletions src/internal/util/NodeEventEmitterDataType.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
/* eslint-disable no-use-before-define */
/* eslint-disable max-len */

/**
* Node.js EventEmitter Add/Remove Listener interface
*/
interface L<N, D> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great if we used more descriptive names here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the descriptive name will be more readable, and at first, I was using the descriptive name.

However, the reason that I finally use the short name (L for Listener, N for Name, and D for Data) is that the below code will have lots of repeated those type names:

For example:

type EventNameDataPair9<AddRemoveListener> = AddRemoveListener extends L9<
  infer N1,
  infer N2,
  infer N3,
  infer N4,
  infer N5,
  infer N6,
  infer N7,
  infer N8,
  infer N9,
  infer D1,
  infer D2,
  infer D3,
  infer D4,
  infer D5,
  infer D6,
  infer D7,
  infer D8,
  infer D9
>
  ? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] | [N5, D5] | [N6, D6] | [N7, D7] | [N8, D8] | [N9, D9]
  : never;

The above code will be very long and I feel the short version of the name is more readable, so I'd like to suggest that we can keep this short name because they are very to be understood:

  1. L - Listener
  2. N - Name
  3. D - Data

On the other hand, I'm OK with using a more descriptive(long) name if you think the long version is better.

Please let me know your final decision and I'll follow it.

(name: N, listener: (data: D, ..._: any[]) => any): any;
}

/**
*
* Overload function inferencer
*
* - L: Listener interface with Add/Remove methods
* - N: Name of the event
* - D: Data of the event
*
*/
interface L1<N1, D1> extends L<N1, D1> {}
interface L2<N1, N2, D1, D2> extends L<N1, D1>, L<N2, D2> {}
interface L3<N1, N2, N3, D1, D2, D3> extends L<N1, D1>, L<N2, D2>, L<N3, D3> {}
interface L4<N1, N2, N3, N4, D1, D2, D3, D4> extends L<N1, D1>, L<N2, D2>, L<N3, D3>, L<N4, D4> {}
interface L5<N1, N2, N3, N4, N5, D1, D2, D3, D4, D5> extends L<N1, D1>, L<N2, D2>, L<N3, D3>, L<N4, D4>, L<N5, D5> {}
interface L6<N1, N2, N3, N4, N5, N6, D1, D2, D3, D4, D5, D6> extends L<N1, D1>, L<N2, D2>, L<N3, D3>, L<N4, D4>, L<N5, D5>, L<N6, D6> {}
interface L7<N1, N2, N3, N4, N5, N6, N7, D1, D2, D3, D4, D5, D6, D7>
extends L<N1, D1>,
L<N2, D2>,
L<N3, D3>,
L<N4, D4>,
L<N5, D5>,
L<N6, D6>,
L<N7, D7> {}
interface L8<N1, N2, N3, N4, N5, N6, N7, N8, D1, D2, D3, D4, D5, D6, D7, D8>
extends L<N1, D1>,
L<N2, D2>,
L<N3, D3>,
L<N4, D4>,
L<N5, D5>,
L<N6, D6>,
L<N7, D7>,
L<N8, D8> {}
interface L9<N1, N2, N3, N4, N5, N6, N7, N8, N9, D1, D2, D3, D4, D5, D6, D7, D8, D9>
extends L<N1, D1>,
L<N2, D2>,
L<N3, D3>,
L<N4, D4>,
L<N5, D5>,
L<N6, D6>,
L<N7, D7>,
L<N8, D8>,
L<N9, D9> {}

type EventNameDataPair1<AddRemoveListener> = AddRemoveListener extends L1<infer N1, infer D1> ? [N1, D1] : never;
type EventNameDataPair2<AddRemoveListener> = AddRemoveListener extends L2<infer N1, infer N2, infer D1, infer D2>
? [N1, D1] | [N2, D2]
: never;
type EventNameDataPair3<AddRemoveListener> = AddRemoveListener extends L3<infer N1, infer N2, infer N3, infer D1, infer D2, infer D3>
? [N1, D1] | [N2, D2] | [N3, D3]
: never;
type EventNameDataPair4<AddRemoveListener> = AddRemoveListener extends L4<
infer N1,
infer N2,
infer N3,
infer N4,
infer D1,
infer D2,
infer D3,
infer D4
>
? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4]
: never;
type EventNameDataPair5<AddRemoveListener> = AddRemoveListener extends L5<
infer N1,
infer N2,
infer N3,
infer N4,
infer N5,
infer D1,
infer D2,
infer D3,
infer D4,
infer D5
>
? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] | [N5, D5]
: never;
type EventNameDataPair6<AddRemoveListener> = AddRemoveListener extends L6<
infer N1,
infer N2,
infer N3,
infer N4,
infer N5,
infer N6,
infer D1,
infer D2,
infer D3,
infer D4,
infer D5,
infer D6
>
? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] | [N5, D5] | [N6, D6]
: never;
type EventNameDataPair7<AddRemoveListener> = AddRemoveListener extends L7<
infer N1,
infer N2,
infer N3,
infer N4,
infer N5,
infer N6,
infer N7,
infer D1,
infer D2,
infer D3,
infer D4,
infer D5,
infer D6,
infer D7
>
? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] | [N5, D5] | [N6, D6] | [N7, D7]
: never;
type EventNameDataPair8<AddRemoveListener> = AddRemoveListener extends L8<
infer N1,
infer N2,
infer N3,
infer N4,
infer N5,
infer N6,
infer N7,
infer N8,
infer D1,
infer D2,
infer D3,
infer D4,
infer D5,
infer D6,
infer D7,
infer D8
>
? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] | [N5, D5] | [N6, D6] | [N7, D7] | [N8, D8]
: never;
type EventNameDataPair9<AddRemoveListener> = AddRemoveListener extends L9<
infer N1,
infer N2,
infer N3,
infer N4,
infer N5,
infer N6,
infer N7,
infer N8,
infer N9,
infer D1,
infer D2,
infer D3,
infer D4,
infer D5,
infer D6,
infer D7,
infer D8,
infer D9
>
? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] | [N5, D5] | [N6, D6] | [N7, D7] | [N8, D8] | [N9, D9]
: never;

interface HasNodeEventEmitterAddRemove<N, D> {
addListener(name: N, listener: (data: D, ...args: any[]) => void): this;
removeListener(name: N, listener: (data: D, ...args: any[]) => void): this;
}

/**
* Get the event name/data pair types from an event emitter
*
* @return `['foo', number] | ['bar', string]`
*/
type EventNameDataPair<AddRemoveListener extends HasNodeEventEmitterAddRemove<any, any>['addListener']> =
| EventNameDataPair9<AddRemoveListener>
| EventNameDataPair8<AddRemoveListener>
| EventNameDataPair7<AddRemoveListener>
| EventNameDataPair6<AddRemoveListener>
| EventNameDataPair5<AddRemoveListener>
| EventNameDataPair4<AddRemoveListener>
| EventNameDataPair3<AddRemoveListener>
| EventNameDataPair2<AddRemoveListener>
| EventNameDataPair1<AddRemoveListener>;

/**
* Convert the `any` type to `unknown for a better safety
*
* @return `AnyToUnknown<any> -> unknown`
*
* TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases
*/
type AnyToUnknown<T> = unknown extends T ? unknown : T;

// the [eventName, eventData] types array
type NodeEventEmitterNameDataPair<E extends HasNodeEventEmitterAddRemove<any, any>> = EventNameDataPair<E['addListener']>;

/**
*
* Tada! Get event emitter data type by event name 8-D
*
*/
type NodeEventEmitterDataType<E extends HasNodeEventEmitterAddRemove<T, any>, T> = Extract<NodeEventEmitterNameDataPair<E>, [T, any]>[1];

// Convert `never` to `unknown`
type NodeEventEmitterDataTypeUnknown<E extends HasNodeEventEmitterAddRemove<T, any>, T> = NodeEventEmitterDataType<E, T> extends never
? unknown
: NodeEventEmitterDataType<E, T>;

interface NamedNodeEventEmitter<N> {
addListener(name: N, handler: (data: NodeEventEmitterDataType<HasNodeEventEmitterAddRemove<N, any>, N>, ...args: any[]) => any): this;
removeListener(name: N, handler: (data: NodeEventEmitterDataType<HasNodeEventEmitterAddRemove<N, any>, N>, ...args: any[]) => any): this;
}

export type {
AnyToUnknown,
NamedNodeEventEmitter,
NodeEventEmitterDataType,
NodeEventEmitterDataTypeUnknown,
NodeEventEmitterNameDataPair,
};