From f4eddaa2a2a599ece28afb0b8f2655d4b0d10b7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 09:12:53 +0800 Subject: [PATCH 01/21] fix(fromEvent): add unit test to show the failure of infer string literal from Node.js EventEmitter --- spec/observables/fromEvent-spec.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index 9fe29b1b64..3167fabac6 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -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'; @@ -432,4 +432,19 @@ describe('fromEvent', () => { expect(nodeList[0]._removeEventListenerArgs).to.deep.equal(nodeList[0]._addEventListenerArgs); expect(nodeList[1]._removeEventListenerArgs).to.deep.equal(nodeList[1]._addEventListenerArgs); }); + + it('should successful inference the first argument from the listener of Node.js EventEmitter', (done) => { + class NodeEventeEmitterTest { + addListener(eventName: 'foo', listener: (bar: number) => void) { return this; } + removeListener(eventName: 'foo', listener: (bar: number) => void ) { return this; } + } + const test = new NodeEventeEmitterTest(); + + expect(() => { + const $ = fromEvent(test, 'foo'); + const typeTest: typeof $ extends Observable ? true : never = true; + expect(typeTest).to.be.true; + done(); + }).to.not.throw(TypeError); + }); }); From 0ac11dd5b2e9391dbb139758765268f8df1b235b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 09:29:41 +0800 Subject: [PATCH 02/21] fix(fromEvent): add code to fix the failed unit test --- spec/observables/fromEvent-spec.ts | 4 ++++ src/internal/observable/fromEvent.ts | 22 ++++++++++++++-------- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index 3167fabac6..723e3a227f 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -433,6 +433,10 @@ describe('fromEvent', () => { expect(nodeList[1]._removeEventListenerArgs).to.deep.equal(nodeList[1]._addEventListenerArgs); }); + /** + * Huan(202111): Correct typing inference for Node.js EventEmitter + * @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: (bar: number) => void) { return this; } diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 8044ff9da9..43eace208a 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -10,9 +10,9 @@ const nodeEventEmitterMethods = ['addListener', 'removeListener'] as const; 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; +export interface NodeStyleEventEmitter { + addListener(eventName: string | symbol, handler: (this: TContext, t: T, ...args: any[]) => any): this; + removeListener(eventName: string | symbol, handler: (this: TContext, t: T, ...args: any[]) => any): this; } export type NodeEventHandler = (...args: any[]) => void; @@ -78,11 +78,17 @@ export function fromEvent( resultSelector: (event: T) => R ): Observable; -export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; +export function fromEvent( + target: NodeStyleEventEmitter | ArrayLike>, + eventName: string +): Observable; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ -export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; -export function fromEvent( - target: NodeStyleEventEmitter | ArrayLike, +export function fromEvent( + target: NodeStyleEventEmitter | ArrayLike>, + eventName: string +): Observable; +export function fromEvent( + target: NodeStyleEventEmitter | ArrayLike>, eventName: string, resultSelector: (...args: any[]) => R ): Observable; @@ -306,7 +312,7 @@ function toCommonHandlerRegistry(target: any, eventName: string) { * for adding and removing event handlers. * @param target the object to check */ -function isNodeStyleEventEmitter(target: any): target is NodeStyleEventEmitter { +function isNodeStyleEventEmitter(target: any): target is NodeStyleEventEmitter { return isFunction(target.addListener) && isFunction(target.removeListener); } From bde7c4071a752fcf0cfb304644789b45d69b2a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 09:42:35 +0800 Subject: [PATCH 03/21] fix(fromEvent): make dtslint happy --- src/internal/observable/fromEvent.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 43eace208a..f53c7fdb8c 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -60,7 +60,10 @@ export interface AddEventListenerOptions extends EventListenerOptions { passive?: boolean; } -export function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string): Observable; +export function fromEvent( + target: HasEventTargetAddRemove | ArrayLike>, + eventName: string +): Observable; export function fromEvent( target: HasEventTargetAddRemove | ArrayLike>, eventName: string, @@ -70,7 +73,7 @@ export function fromEvent( target: HasEventTargetAddRemove | ArrayLike>, eventName: string, options: EventListenerOptions -): Observable; +): Observable; export function fromEvent( target: HasEventTargetAddRemove | ArrayLike>, eventName: string, From b069b1074b5852906b505295df9df1b2e57087b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 09:56:39 +0800 Subject: [PATCH 04/21] fix(fromEvent): fix code --- spec/observables/fromEvent-spec.ts | 2 +- src/internal/observable/fromEvent.ts | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index 723e3a227f..168db153bb 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -434,7 +434,7 @@ describe('fromEvent', () => { }); /** - * Huan(202111): Correct typing inference for Node.js EventEmitter + * 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) => { diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index f53c7fdb8c..186d89ca39 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -60,10 +60,7 @@ export interface AddEventListenerOptions extends EventListenerOptions { passive?: boolean; } -export function fromEvent( - target: HasEventTargetAddRemove | ArrayLike>, - eventName: string -): Observable; +export function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string): Observable; export function fromEvent( target: HasEventTargetAddRemove | ArrayLike>, eventName: string, @@ -73,7 +70,7 @@ export function fromEvent( target: HasEventTargetAddRemove | ArrayLike>, eventName: string, options: EventListenerOptions -): Observable; +): Observable; export function fromEvent( target: HasEventTargetAddRemove | ArrayLike>, eventName: string, @@ -84,7 +81,7 @@ export function fromEvent( export function fromEvent( target: NodeStyleEventEmitter | ArrayLike>, eventName: string -): Observable; +): Observable; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ export function fromEvent( target: NodeStyleEventEmitter | ArrayLike>, From 29910b98a4ed34d2be74ebb3cd443c9a29b0452e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 10:33:43 +0800 Subject: [PATCH 05/21] fix(fromEvent): add AnyToUnknown converter --- src/internal/observable/fromEvent.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 186d89ca39..91c4f54ce9 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -17,6 +17,8 @@ export interface NodeStyleEventEmitter { export type NodeEventHandler = (...args: any[]) => void; +type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; + // For APIs that implement `addListener` and `removeListener` methods that may // not use the same arguments or return EventEmitter values // such as React Native @@ -81,7 +83,7 @@ export function fromEvent( export function fromEvent( target: NodeStyleEventEmitter | ArrayLike>, eventName: string -): Observable; +): Observable>; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ export function fromEvent( target: NodeStyleEventEmitter | ArrayLike>, From 62bced1a8420d441e6bc8bb96dc5f090c35e5c6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 10:38:02 +0800 Subject: [PATCH 06/21] fix(fromEvent): fix dtslint spec --- spec-dtslint/observables/fromEvent-spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec-dtslint/observables/fromEvent-spec.ts b/spec-dtslint/observables/fromEvent-spec.ts index 8ef58e387c..3385d81d90 100644 --- a/spec-dtslint/observables/fromEvent-spec.ts +++ b/spec-dtslint/observables/fromEvent-spec.ts @@ -51,13 +51,13 @@ it('should support a document source with options and result selector', () => { declare const nodeStyleSource: Pick; it('should support a node-style source', () => { - const source: NodeStyleEventEmitter = nodeStyleSource; + const source: NodeStyleEventEmitter = nodeStyleSource; const a = fromEvent(nodeStyleSource, "exit"); // $ExpectType Observable const b = fromEvent(nodeStyleSource, "exit"); // $ExpectType Observable }); it('should deprecate explicit type parameters for a node-style source', () => { - const source: NodeStyleEventEmitter = nodeStyleSource; + const source: NodeStyleEventEmitter = nodeStyleSource; const a = fromEvent(nodeStyleSource, "exit"); // $ExpectNoDeprecation const b = fromEvent(nodeStyleSource, "exit"); // $ExpectDeprecation }); From f80414f1fb97cef57e5368c91656634f43b34856 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 10:45:19 +0800 Subject: [PATCH 07/21] fix(fromEvent): update api_guard --- api_guard/dist/types/index.d.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index abbd15a343..4494eb75d9 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -257,9 +257,9 @@ export declare function fromEvent(target: HasEventTargetAddRemove | ArrayL export declare function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string, resultSelector: (event: T) => R): Observable; export declare function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string, options: EventListenerOptions): Observable; export declare function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string, options: EventListenerOptions, resultSelector: (event: T) => R): Observable; -export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; -export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; -export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string, resultSelector: (...args: any[]) => R): Observable; +export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike>, eventName: string): Observable>; +export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike>, eventName: string): Observable; +export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike>, eventName: string, resultSelector: (...args: any[]) => R): Observable; export declare function fromEvent(target: NodeCompatibleEventEmitter | ArrayLike, eventName: string): Observable; export declare function fromEvent(target: NodeCompatibleEventEmitter | ArrayLike, eventName: string): Observable; export declare function fromEvent(target: NodeCompatibleEventEmitter | ArrayLike, eventName: string, resultSelector: (...args: any[]) => R): Observable; From f25a17478fe96534854fb2351cc07dbd9c396ae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 13:42:33 +0800 Subject: [PATCH 08/21] fix(fromEvent): add two event name for multi type testing --- spec/observables/fromEvent-spec.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index 168db153bb..c8fd9dbe93 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -439,15 +439,25 @@ describe('fromEvent', () => { */ it('should successful inference the first argument from the listener of Node.js EventEmitter', (done) => { class NodeEventeEmitterTest { - addListener(eventName: 'foo', listener: (bar: number) => void) { return this; } - removeListener(eventName: 'foo', listener: (bar: number) => void ) { return this; } + addListener(eventName: 'foo', listener: (foo: number) => void): this + addListener(eventName: 'bar', listener: (bar: string) => void): this + addListener(eventName: 'foo' | 'bar', listener: ((foo: number) => void) | ((bar: string) => void)): this { return this; } + + removeListener(eventName: 'foo', listener: (foo: number) => void ): this + removeListener(eventName: 'bar', listener: (bar: string) => void ): this + removeListener(eventName: 'foo' | 'bar', listener: ((foo: number) => void) | ((bar: string) => void)): this { return this; } } const test = new NodeEventeEmitterTest(); expect(() => { - const $ = fromEvent(test, 'foo'); - const typeTest: typeof $ extends Observable ? true : never = true; - expect(typeTest).to.be.true; + const foo$ = fromEvent(test, 'foo'); + const fooTypeTest: typeof foo$ extends Observable ? true : never = true; + expect(fooTypeTest).to.be.true; + + const bar$ = fromEvent(test, 'bar'); + const barTypeTest: typeof bar$ extends Observable ? true : never = true; + expect(barTypeTest).to.be.true; + done(); }).to.not.throw(TypeError); }); From 62c7582307887f10c0885428c4b229fd02e1c41b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 21:02:52 +0800 Subject: [PATCH 09/21] fix(fromEvent): add helper file with TypeScript overload function workaround --- spec/helpers/inference-overload-function.ts | 116 ++++++++++++++++++++ spec/observables/fromEvent-spec.ts | 16 +-- src/internal/observable/fromEvent.ts | 2 - 3 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 spec/helpers/inference-overload-function.ts diff --git a/spec/helpers/inference-overload-function.ts b/spec/helpers/inference-overload-function.ts new file mode 100644 index 0000000000..cfe9ed1cd1 --- /dev/null +++ b/spec/helpers/inference-overload-function.ts @@ -0,0 +1,116 @@ +/* eslint-disable no-use-before-define */ +/* eslint-disable max-len */ +/** + * Issue #6669 - fix(fromEvent): infer from Node.js EventEmitter with types + * @see https://github.com/ReactiveX/rxjs/pull/6669 + * + * Huan + * Nov 7, 2021 + * + * TypeScript has design limitation that prevents us from inferring from the overload functins: + * > ReturnType or the use of infer in a function parameter or return position does handle overloads. + * the last overload is used for inference as it is assumed to be the most general. + * - @mhegazy @link https://github.com/Microsoft/TypeScript/issues/24275#issuecomment-390701982 + * + * This file is created by @huan and it was inspired from the brilliant work from @Aidin on StackOverflow: + * @link https://stackoverflow.com/a/60822641/1123955 + */ + +/** + * Node.js EventEmitter Add/Remove Listener interface + */ +interface L { + (type: T, listener: (event: E, ..._: any[]) => any): any +} + +/** + * Overload function support + * + * - L: Add/Remove Listener interface + * - T: Event type + * - E: Event payload + */ +interface L1 extends L {} +interface L2 extends L, L {} +interface L3 extends L, L, L {} + +type TypeEventPair1 = AddRemoveListener extends L1 ? [T1, E1] : never +type TypeEventPair2 = AddRemoveListener extends L2 ? [T1, E1] | [T2, E2] : never +type TypeEventPair3 = AddRemoveListener extends L3 ? [T1, E1] | [T2, E2] | [T3, E3]: never + +type NodeEventEmitterAddRemoveListener = (type: T, listener: (event: E, ...args: any[]) => any) => any + +type TypeEventPair = + | TypeEventPair3 + | TypeEventPair2 + | TypeEventPair1 + // L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any; (t: infer T2, l: (e: infer E2, ..._: any[]) => any): any; } ? [T1, E1] | [T2, E2] : + // Listener extends L2 ? [T1, E1] | [T2, E2] : + // L extends { Y, Y } [T1, E1] | [T2, E2] : + // L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any } ? [T1, E1] : + // L extends Y ? [T1, E1] : + // L extends L1 ? [T1, E1] : + +/** + * Convert the `any` type to `unknown for a better safety + * TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases + */ +type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; + +interface HasEventEmitterAddRemove { + addListener( + type: T, + listener: ((evt: E, ...args: any[]) => void), + ): this; + removeListener( + type: T, + listener: ((evt: E, ...args: any[]) => void), + ): this; +} + +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; } + + /** + * JQueryStyle + */ + // 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 {} +} + +// type NE = InstanceType +type NE = NodeEventeEmitterTest + +type EVENT_NAME = TypeEventPair[0] + +type NodeEmitterEvent< + E extends HasEventEmitterAddRemove, + T +> = + Extract< + TypeEventPair, + [T, any] + >[1] + +type T1 = NodeEmitterEvent +type T2 = NodeEmitterEvent + +// type NodeEmitterEvent< +// E extends EventEmitter, +// T extends string | symbol +// > = ListenerEventPayload + +export type { + NodeEmitterEvent, +}; diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index c8fd9dbe93..95b88cd85b 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -439,23 +439,23 @@ describe('fromEvent', () => { */ it('should successful inference the first argument from the listener of Node.js EventEmitter', (done) => { class NodeEventeEmitterTest { - addListener(eventName: 'foo', listener: (foo: number) => void): this - addListener(eventName: 'bar', listener: (bar: string) => void): this - addListener(eventName: 'foo' | 'bar', listener: ((foo: number) => void) | ((bar: string) => void)): this { return this; } + 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: number) => void ): this - removeListener(eventName: 'bar', listener: (bar: string) => void ): this - removeListener(eventName: 'foo' | 'bar', listener: ((foo: number) => void) | ((bar: string) => 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(); expect(() => { const foo$ = fromEvent(test, 'foo'); - const fooTypeTest: typeof foo$ extends Observable ? true : never = true; + const fooTypeTest: typeof foo$ extends Observable ? true : never = true; expect(fooTypeTest).to.be.true; const bar$ = fromEvent(test, 'bar'); - const barTypeTest: typeof bar$ extends Observable ? true : never = true; + const barTypeTest: typeof bar$ extends Observable ? true : never = true; expect(barTypeTest).to.be.true; done(); diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 91c4f54ce9..25b7f78e24 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -17,8 +17,6 @@ export interface NodeStyleEventEmitter { export type NodeEventHandler = (...args: any[]) => void; -type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; - // For APIs that implement `addListener` and `removeListener` methods that may // not use the same arguments or return EventEmitter values // such as React Native From c145cac9c5e40d8248317c93445af95ab77f3b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Sun, 7 Nov 2021 21:09:47 +0800 Subject: [PATCH 10/21] fix(fromEvent): add helper functio unit test --- .../inference-overload-function.spec.ts | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 spec/helpers/inference-overload-function.spec.ts diff --git a/spec/helpers/inference-overload-function.spec.ts b/spec/helpers/inference-overload-function.spec.ts new file mode 100644 index 0000000000..f5131a464f --- /dev/null +++ b/spec/helpers/inference-overload-function.spec.ts @@ -0,0 +1,34 @@ +import { expect } from 'chai'; + +import { NodeEmitterEvent } from './inference-overload-function'; + +class NodeEventeEmitterFixture { + 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; } + + /** + * 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 {} +} + +describe('overload function inference types helper', () => { + it('should get emitter type & event as correctly', () => { + const foo: NodeEmitterEvent = false; + const bar: NodeEmitterEvent = true; + + expect(foo).to.be.false; + expect(bar).to.be.true; + }); +}); \ No newline at end of file From a3fa4a7f1597d62dadd4f1f8022ec57be8c99b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 00:20:08 +0800 Subject: [PATCH 11/21] fix(fromEvent): add util helper for workaround TypeScript limitation --- spec/helpers/inference-overload-function.ts | 116 --------------- .../inference-overload-function.spec.ts | 6 +- src/internal/observable/fromEvent.ts | 3 +- src/internal/util/NoddEventEmitterType.ts | 133 ++++++++++++++++++ 4 files changed, 138 insertions(+), 120 deletions(-) delete mode 100644 spec/helpers/inference-overload-function.ts rename spec/{helpers => util}/inference-overload-function.spec.ts (84%) create mode 100644 src/internal/util/NoddEventEmitterType.ts diff --git a/spec/helpers/inference-overload-function.ts b/spec/helpers/inference-overload-function.ts deleted file mode 100644 index cfe9ed1cd1..0000000000 --- a/spec/helpers/inference-overload-function.ts +++ /dev/null @@ -1,116 +0,0 @@ -/* eslint-disable no-use-before-define */ -/* eslint-disable max-len */ -/** - * Issue #6669 - fix(fromEvent): infer from Node.js EventEmitter with types - * @see https://github.com/ReactiveX/rxjs/pull/6669 - * - * Huan - * Nov 7, 2021 - * - * TypeScript has design limitation that prevents us from inferring from the overload functins: - * > ReturnType or the use of infer in a function parameter or return position does handle overloads. - * the last overload is used for inference as it is assumed to be the most general. - * - @mhegazy @link https://github.com/Microsoft/TypeScript/issues/24275#issuecomment-390701982 - * - * This file is created by @huan and it was inspired from the brilliant work from @Aidin on StackOverflow: - * @link https://stackoverflow.com/a/60822641/1123955 - */ - -/** - * Node.js EventEmitter Add/Remove Listener interface - */ -interface L { - (type: T, listener: (event: E, ..._: any[]) => any): any -} - -/** - * Overload function support - * - * - L: Add/Remove Listener interface - * - T: Event type - * - E: Event payload - */ -interface L1 extends L {} -interface L2 extends L, L {} -interface L3 extends L, L, L {} - -type TypeEventPair1 = AddRemoveListener extends L1 ? [T1, E1] : never -type TypeEventPair2 = AddRemoveListener extends L2 ? [T1, E1] | [T2, E2] : never -type TypeEventPair3 = AddRemoveListener extends L3 ? [T1, E1] | [T2, E2] | [T3, E3]: never - -type NodeEventEmitterAddRemoveListener = (type: T, listener: (event: E, ...args: any[]) => any) => any - -type TypeEventPair = - | TypeEventPair3 - | TypeEventPair2 - | TypeEventPair1 - // L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any; (t: infer T2, l: (e: infer E2, ..._: any[]) => any): any; } ? [T1, E1] | [T2, E2] : - // Listener extends L2 ? [T1, E1] | [T2, E2] : - // L extends { Y, Y } [T1, E1] | [T2, E2] : - // L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any } ? [T1, E1] : - // L extends Y ? [T1, E1] : - // L extends L1 ? [T1, E1] : - -/** - * Convert the `any` type to `unknown for a better safety - * TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases - */ -type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; - -interface HasEventEmitterAddRemove { - addListener( - type: T, - listener: ((evt: E, ...args: any[]) => void), - ): this; - removeListener( - type: T, - listener: ((evt: E, ...args: any[]) => void), - ): this; -} - -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; } - - /** - * JQueryStyle - */ - // 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 {} -} - -// type NE = InstanceType -type NE = NodeEventeEmitterTest - -type EVENT_NAME = TypeEventPair[0] - -type NodeEmitterEvent< - E extends HasEventEmitterAddRemove, - T -> = - Extract< - TypeEventPair, - [T, any] - >[1] - -type T1 = NodeEmitterEvent -type T2 = NodeEmitterEvent - -// type NodeEmitterEvent< -// E extends EventEmitter, -// T extends string | symbol -// > = ListenerEventPayload - -export type { - NodeEmitterEvent, -}; diff --git a/spec/helpers/inference-overload-function.spec.ts b/spec/util/inference-overload-function.spec.ts similarity index 84% rename from spec/helpers/inference-overload-function.spec.ts rename to spec/util/inference-overload-function.spec.ts index f5131a464f..0eaf97c29d 100644 --- a/spec/helpers/inference-overload-function.spec.ts +++ b/spec/util/inference-overload-function.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { NodeEmitterEvent } from './inference-overload-function'; +import { NodeEventEmitterType } from '../../src/internal/util/NoddEventEmitterType'; class NodeEventeEmitterFixture { addListener(eventName: 'foo', listener: (foo: false) => void): this @@ -25,8 +25,8 @@ class NodeEventeEmitterFixture { describe('overload function inference types helper', () => { it('should get emitter type & event as correctly', () => { - const foo: NodeEmitterEvent = false; - const bar: NodeEmitterEvent = true; + const foo: NodeEventEmitterType = false; + const bar: NodeEventEmitterType = true; expect(foo).to.be.false; expect(bar).to.be.true; diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 25b7f78e24..803b2acba6 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -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 { NodeEventEmitterType } from '../util/NoddEventEmitterType'; // These constants are used to create handler registry functions using array mapping below. const nodeEventEmitterMethods = ['addListener', 'removeListener'] as const; @@ -81,7 +82,7 @@ export function fromEvent( export function fromEvent( target: NodeStyleEventEmitter | ArrayLike>, eventName: string -): Observable>; +): Observable>; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ export function fromEvent( target: NodeStyleEventEmitter | ArrayLike>, diff --git a/src/internal/util/NoddEventEmitterType.ts b/src/internal/util/NoddEventEmitterType.ts new file mode 100644 index 0000000000..dfa62709fc --- /dev/null +++ b/src/internal/util/NoddEventEmitterType.ts @@ -0,0 +1,133 @@ +/** + * Issue #6669 - fix(fromEvent): infer from Node.js EventEmitter with types + * @see https://github.com/ReactiveX/rxjs/pull/6669 + * + * Author: Huan + * Date: Nov 7, 2021 + * + * TypeScript has design limitation that prevents us from inferring from the overload functins: + * > ReturnType or the use of infer in a function parameter or return position does handle overloads. + * the last overload is used for inference as it is assumed to be the most general. + * - @mhegazy @link https://github.com/Microsoft/TypeScript/issues/24275#issuecomment-390701982 + * + * This file is created by @huan and it was inspired from the brilliant work from @Aidin on StackOverflow: + * @link https://stackoverflow.com/a/60822641/1123955 + */ + +/* eslint-disable no-use-before-define */ +/* eslint-disable max-len */ +import { Observable } from '../Observable'; + +class NodeEventEmitterTest { + 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; + } + + /** + * JQueryStyle + */ + // 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 {} +} + +/** + * Node.js EventEmitter Add/Remove Listener interface + */ +interface L { + (type: T, listener: (event: E, ..._: any[]) => any): any; +} + +/** + * Overload function support + * + * - L: Add/Remove Listener interface + * - T: Event type + * - E: Event payload + */ +interface L1 extends L {} +interface L2 extends L, L {} +interface L3 extends L, L, L {} + +type TypeEventPair1 = AddRemoveListener extends L1 ? [T1, E1] : never; +type TypeEventPair2 = AddRemoveListener extends L2 ? [T1, E1] | [T2, E2] : never; +type TypeEventPair3 = AddRemoveListener extends L3 + ? [T1, E1] | [T2, E2] | [T3, E3] + : never; + +type NodeEventEmitterAddRemoveListener = (type: T, listener: (event: E, ...args: any[]) => any) => any; + +type TypeEventPair = + | TypeEventPair3 + | TypeEventPair2 + | TypeEventPair1; +// L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any; (t: infer T2, l: (e: infer E2, ..._: any[]) => any): any; } ? [T1, E1] | [T2, E2] : +// Listener extends L2 ? [T1, E1] | [T2, E2] : +// L extends { Y, Y } [T1, E1] | [T2, E2] : +// L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any } ? [T1, E1] : +// L extends Y ? [T1, E1] : +// L extends L1 ? [T1, E1] : + +/** + * Convert the `any` type to `unknown for a better safety + * TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases + */ +type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; + +interface HasNodeEventEmitterAddRemove { + addListener(type: T, listener: (evt: E, ...args: any[]) => void): this; + removeListener(type: T, listener: (evt: E, ...args: any[]) => void): this; +} + +type NodeEventEmitterPair> = TypeEventPair; + +type NodeEventEmitterName> = NodeEventEmitterPair[0]; + +// type EVENT_PAIR = TypeEventPair +type EVENT_PAIR = NodeEventEmitterPair; +type EVENT_NAME = EVENT_PAIR[0]; +type EVENT_TYPE = EVENT_PAIR[1]; + +type NodeEventEmitterType, T> = Extract, [T, any]>[1]; + +type FOO_EVENT = NodeEventEmitterType; +type BAR_EVENT = NodeEventEmitterType; + +// export interface NodeStyleEventEmitter> { +export interface NodeStyleEventEmitter { + addListener(type: T, handler: (event: NodeEventEmitterType, T>, ...args: any[]) => any): this; + removeListener(type: T, handler: (event: NodeEventEmitterType, T>, ...args: any[]) => any): this; +} + +export function fromEvent>( + target: E | ArrayLike, + eventName: T +): Observable> { + return {} as any; +} + +// type TARGET> = NodeStyleEventEmitter +// type TARGET1 = TARGET +// type TARGET1_NAME = NodeEventEmitterName +// type T = TypeEventPair +// type TARGET1_TYPE = NodeEventEmitterType + +// const t: T = 0; + +const ne = new NodeEventEmitterTest(); +const foo$ = fromEvent(ne, 'foo'); +const bar$ = fromEvent(ne, 'bar'); + +export type { NodeEventEmitterType }; From b17c418c2f662e6216a2f3d846d1966b711e1338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 09:48:45 +0800 Subject: [PATCH 12/21] fix(fromEvent): clean util function code --- spec/util/NodeEventEmitterDataType-spec.ts | 55 ++++++++ spec/util/inference-overload-function.spec.ts | 34 ----- src/internal/observable/fromEvent.ts | 2 +- src/internal/util/NoddEventEmitterType.ts | 133 ------------------ src/internal/util/NodeEventEmitterDataType.ts | 114 +++++++++++++++ 5 files changed, 170 insertions(+), 168 deletions(-) create mode 100644 spec/util/NodeEventEmitterDataType-spec.ts delete mode 100644 spec/util/inference-overload-function.spec.ts delete mode 100644 src/internal/util/NoddEventEmitterType.ts create mode 100644 src/internal/util/NodeEventEmitterDataType.ts diff --git a/spec/util/NodeEventEmitterDataType-spec.ts b/spec/util/NodeEventEmitterDataType-spec.ts new file mode 100644 index 0000000000..d16a8e5107 --- /dev/null +++ b/spec/util/NodeEventEmitterDataType-spec.ts @@ -0,0 +1,55 @@ +import { expect } from 'chai'; + +import { + NodeEventEmitterNameDataPair, + NodeEventEmitterDataType, +} from '../../src/internal/util/NodeEventEmitterDataType'; + +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 {} +} + +describe('NodeEventEmitterDataType smoke testing', () => { + it('should get emitter name & data types correctly', () => { + const foo: NodeEventEmitterDataType = fooEvent; + const bar: NodeEventEmitterDataType = barEvent; + + expect(foo).to.be.ok; + expect(bar).to.be.ok; + }); + it('should get name & data from NodeEventEmitterNameDataPair', () => { + // type EVENT_PAIR = TypeEventPair + type EVENT_PAIR = NodeEventEmitterNameDataPair; + type EVENT_NAME = EVENT_PAIR[0]; + type EVENT_DATA = EVENT_PAIR[1]; + + const nameTypeTest: EVENT_NAME extends 'foo' | 'bar' ? true : never = true; + const dataTypeTest: EVENT_DATA extends FOO_DATA | BAR_DATA ? true : never = true; + + expect(nameTypeTest).to.be.ok; + expect(dataTypeTest).to.be.ok; + }); +}); \ No newline at end of file diff --git a/spec/util/inference-overload-function.spec.ts b/spec/util/inference-overload-function.spec.ts deleted file mode 100644 index 0eaf97c29d..0000000000 --- a/spec/util/inference-overload-function.spec.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { expect } from 'chai'; - -import { NodeEventEmitterType } from '../../src/internal/util/NoddEventEmitterType'; - -class NodeEventeEmitterFixture { - 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; } - - /** - * 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 {} -} - -describe('overload function inference types helper', () => { - it('should get emitter type & event as correctly', () => { - const foo: NodeEventEmitterType = false; - const bar: NodeEventEmitterType = true; - - expect(foo).to.be.false; - expect(bar).to.be.true; - }); -}); \ No newline at end of file diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 803b2acba6..5170b52fa1 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -4,7 +4,7 @@ import { mergeMap } from '../operators/mergeMap'; import { isArrayLike } from '../util/isArrayLike'; import { isFunction } from '../util/isFunction'; import { mapOneOrManyArgs } from '../util/mapOneOrManyArgs'; -import { NodeEventEmitterType } from '../util/NoddEventEmitterType'; +import { NodeEventEmitterDataType } from '../util/NodeEventEmitterDataType'; // These constants are used to create handler registry functions using array mapping below. const nodeEventEmitterMethods = ['addListener', 'removeListener'] as const; diff --git a/src/internal/util/NoddEventEmitterType.ts b/src/internal/util/NoddEventEmitterType.ts deleted file mode 100644 index dfa62709fc..0000000000 --- a/src/internal/util/NoddEventEmitterType.ts +++ /dev/null @@ -1,133 +0,0 @@ -/** - * Issue #6669 - fix(fromEvent): infer from Node.js EventEmitter with types - * @see https://github.com/ReactiveX/rxjs/pull/6669 - * - * Author: Huan - * Date: Nov 7, 2021 - * - * TypeScript has design limitation that prevents us from inferring from the overload functins: - * > ReturnType or the use of infer in a function parameter or return position does handle overloads. - * the last overload is used for inference as it is assumed to be the most general. - * - @mhegazy @link https://github.com/Microsoft/TypeScript/issues/24275#issuecomment-390701982 - * - * This file is created by @huan and it was inspired from the brilliant work from @Aidin on StackOverflow: - * @link https://stackoverflow.com/a/60822641/1123955 - */ - -/* eslint-disable no-use-before-define */ -/* eslint-disable max-len */ -import { Observable } from '../Observable'; - -class NodeEventEmitterTest { - 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; - } - - /** - * JQueryStyle - */ - // 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 {} -} - -/** - * Node.js EventEmitter Add/Remove Listener interface - */ -interface L { - (type: T, listener: (event: E, ..._: any[]) => any): any; -} - -/** - * Overload function support - * - * - L: Add/Remove Listener interface - * - T: Event type - * - E: Event payload - */ -interface L1 extends L {} -interface L2 extends L, L {} -interface L3 extends L, L, L {} - -type TypeEventPair1 = AddRemoveListener extends L1 ? [T1, E1] : never; -type TypeEventPair2 = AddRemoveListener extends L2 ? [T1, E1] | [T2, E2] : never; -type TypeEventPair3 = AddRemoveListener extends L3 - ? [T1, E1] | [T2, E2] | [T3, E3] - : never; - -type NodeEventEmitterAddRemoveListener = (type: T, listener: (event: E, ...args: any[]) => any) => any; - -type TypeEventPair = - | TypeEventPair3 - | TypeEventPair2 - | TypeEventPair1; -// L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any; (t: infer T2, l: (e: infer E2, ..._: any[]) => any): any; } ? [T1, E1] | [T2, E2] : -// Listener extends L2 ? [T1, E1] | [T2, E2] : -// L extends { Y, Y } [T1, E1] | [T2, E2] : -// L extends { (t: infer T1, l: (e: infer E1, ..._: any[]) => any): any } ? [T1, E1] : -// L extends Y ? [T1, E1] : -// L extends L1 ? [T1, E1] : - -/** - * Convert the `any` type to `unknown for a better safety - * TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases - */ -type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; - -interface HasNodeEventEmitterAddRemove { - addListener(type: T, listener: (evt: E, ...args: any[]) => void): this; - removeListener(type: T, listener: (evt: E, ...args: any[]) => void): this; -} - -type NodeEventEmitterPair> = TypeEventPair; - -type NodeEventEmitterName> = NodeEventEmitterPair[0]; - -// type EVENT_PAIR = TypeEventPair -type EVENT_PAIR = NodeEventEmitterPair; -type EVENT_NAME = EVENT_PAIR[0]; -type EVENT_TYPE = EVENT_PAIR[1]; - -type NodeEventEmitterType, T> = Extract, [T, any]>[1]; - -type FOO_EVENT = NodeEventEmitterType; -type BAR_EVENT = NodeEventEmitterType; - -// export interface NodeStyleEventEmitter> { -export interface NodeStyleEventEmitter { - addListener(type: T, handler: (event: NodeEventEmitterType, T>, ...args: any[]) => any): this; - removeListener(type: T, handler: (event: NodeEventEmitterType, T>, ...args: any[]) => any): this; -} - -export function fromEvent>( - target: E | ArrayLike, - eventName: T -): Observable> { - return {} as any; -} - -// type TARGET> = NodeStyleEventEmitter -// type TARGET1 = TARGET -// type TARGET1_NAME = NodeEventEmitterName -// type T = TypeEventPair -// type TARGET1_TYPE = NodeEventEmitterType - -// const t: T = 0; - -const ne = new NodeEventEmitterTest(); -const foo$ = fromEvent(ne, 'foo'); -const bar$ = fromEvent(ne, 'bar'); - -export type { NodeEventEmitterType }; diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts new file mode 100644 index 0000000000..a344a75209 --- /dev/null +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -0,0 +1,114 @@ +/** + * Issue #6669 - fix(fromEvent): infer from Node.js EventEmitter with types + * @see https://github.com/ReactiveX/rxjs/pull/6669 + * + * Author: Huan + * Date: Nov 7, 2021 + * + * TypeScript has design limitation that prevents us from inferring from the overload functins: + * > ReturnType or the use of infer in a function parameter or return position does handle overloads. + * the last overload is used for inference as it is assumed to be the most general. + * - @mhegazy @link https://github.com/Microsoft/TypeScript/issues/24275#issuecomment-390701982 + * + * This file is created by @huan and it was inspired from the brilliant work from @Aidin on StackOverflow: + * @link https://stackoverflow.com/a/60822641/1123955 + */ + +/* eslint-disable no-use-before-define */ +/* eslint-disable max-len */ +import { Observable } from '../Observable'; + +/** + * Node.js EventEmitter Add/Remove Listener interface + */ +interface L { + (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 extends L {} +interface L2 extends L, L {} +interface L3 extends L, L, L {} +interface L4 extends L, L, L, L {} + +type EventNameDataPair1 = AddRemoveListener extends L1 ? [N1, D1] : never; +type EventNameDataPair2 = AddRemoveListener extends L2 + ? [N1, D1] | [N2, D2] + : never; +type EventNameDataPair3 = AddRemoveListener extends L3 + ? [N1, D1] | [N2, D2] | [N3, D3] + : never; +type EventNameDataPair4 = 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 NodeEventEmitterAddRemoveListener = (name: N, listener: (data: D, ...args: any[]) => any) => any; +interface HasNodeEventEmitterAddRemove { + 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 + * + * @example `['foo', number] | ['bar', string]` + */ +type EventNameDataPair['addListener']> = + | EventNameDataPair4 + | EventNameDataPair3 + | EventNameDataPair2 + | EventNameDataPair1; + +/** + * Convert the `any` type to `unknown for a better safety + * + * @example `AnyToUnknown === unknown` + * + * TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases + */ +type AnyToUnknown = null extends T ? (void extends T ? unknown : T) : T; + +// the [eventName, eventData] types array +type NodeEventEmitterNameDataPair> = EventNameDataPair; + +// the event names +// type NodeEventEmitterName> = NodeEventEmitterNameDataPair[0]; +// // The types of `args[0]` defined by the listeners +// type NodeEventEmitterData> = NodeEventEmitterNameDataPair[1]; + +/** + * Get event emitter data type by event name + */ +type NodeEventEmitterDataType, T> = Extract, [T, any]>[1]; + +// export interface NodeStyleEventEmitter> { +interface NodeStyleEventEmitter { + addListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; + removeListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; +} + +export function fromEvent>( + target: E | ArrayLike, + eventName: T +): Observable> { + return {} as any; +} + +export type { NodeEventEmitterDataType, NodeEventEmitterNameDataPair }; From ff95a6c503667738f49c735af121bab25c0e8e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 10:07:41 +0800 Subject: [PATCH 13/21] fix(fromEvent): clean fromEvent.ts code to make sure its clean --- src/internal/observable/fromEvent.ts | 23 +++++++------------ src/internal/util/NodeEventEmitterDataType.ts | 9 ++++---- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 5170b52fa1..8044ff9da9 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -4,16 +4,15 @@ import { mergeMap } from '../operators/mergeMap'; import { isArrayLike } from '../util/isArrayLike'; import { isFunction } from '../util/isFunction'; import { mapOneOrManyArgs } from '../util/mapOneOrManyArgs'; -import { NodeEventEmitterDataType } from '../util/NodeEventEmitterDataType'; // These constants are used to create handler registry functions using array mapping below. const nodeEventEmitterMethods = ['addListener', 'removeListener'] as const; const eventTargetMethods = ['addEventListener', 'removeEventListener'] as const; const jqueryMethods = ['on', 'off'] as const; -export interface NodeStyleEventEmitter { - addListener(eventName: string | symbol, handler: (this: TContext, t: T, ...args: any[]) => any): this; - removeListener(eventName: string | symbol, handler: (this: TContext, t: T, ...args: any[]) => any): this; +export interface NodeStyleEventEmitter { + addListener(eventName: string | symbol, handler: NodeEventHandler): this; + removeListener(eventName: string | symbol, handler: NodeEventHandler): this; } export type NodeEventHandler = (...args: any[]) => void; @@ -79,17 +78,11 @@ export function fromEvent( resultSelector: (event: T) => R ): Observable; -export function fromEvent( - target: NodeStyleEventEmitter | ArrayLike>, - eventName: string -): Observable>; +export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ -export function fromEvent( - target: NodeStyleEventEmitter | ArrayLike>, - eventName: string -): Observable; -export function fromEvent( - target: NodeStyleEventEmitter | ArrayLike>, +export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; +export function fromEvent( + target: NodeStyleEventEmitter | ArrayLike, eventName: string, resultSelector: (...args: any[]) => R ): Observable; @@ -313,7 +306,7 @@ function toCommonHandlerRegistry(target: any, eventName: string) { * for adding and removing event handlers. * @param target the object to check */ -function isNodeStyleEventEmitter(target: any): target is NodeStyleEventEmitter { +function isNodeStyleEventEmitter(target: any): target is NodeStyleEventEmitter { return isFunction(target.addListener) && isFunction(target.removeListener); } diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index a344a75209..a109a0d91a 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -94,17 +94,18 @@ type NodeEventEmitterNameDataPair> = NodeEventEmitterNameDataPair[1]; /** - * Get event emitter data type by event name + * + * Tada! Get event emitter data type by event name + * */ type NodeEventEmitterDataType, T> = Extract, [T, any]>[1]; -// export interface NodeStyleEventEmitter> { -interface NodeStyleEventEmitter { +interface NamedNodeEventEmitter { addListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; removeListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; } -export function fromEvent>( +export function fromEvent>( target: E | ArrayLike, eventName: T ): Observable> { From 740abf37928483988385c63680201353efcf0504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 10:23:36 +0800 Subject: [PATCH 14/21] fix(fromEvent): integrate code to fromEvent --- spec/observables/fromEvent-spec.ts | 1 + spec/util/NodeEventEmitterDataType-spec.ts | 7 +++++++ src/internal/observable/fromEvent.ts | 10 ++++++++++ src/internal/util/NodeEventEmitterDataType.ts | 9 +-------- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index 95b88cd85b..10e268b51f 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -447,6 +447,7 @@ describe('fromEvent', () => { 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(); expect(() => { diff --git a/spec/util/NodeEventEmitterDataType-spec.ts b/spec/util/NodeEventEmitterDataType-spec.ts index d16a8e5107..9b82e34bf3 100644 --- a/spec/util/NodeEventEmitterDataType-spec.ts +++ b/spec/util/NodeEventEmitterDataType-spec.ts @@ -3,6 +3,7 @@ import { expect } from 'chai'; import { NodeEventEmitterNameDataPair, NodeEventEmitterDataType, + NamedNodeEventEmitter, } from '../../src/internal/util/NodeEventEmitterDataType'; const fooEvent = 'fooEvent'; @@ -40,6 +41,12 @@ describe('NodeEventEmitterDataType smoke testing', () => { expect(foo).to.be.ok; expect(bar).to.be.ok; }); + + it('should extendable', () => { + const emitterTypeTest: NodeEventEmitterFixture extends NamedNodeEventEmitter<'foo'> ? true : never = true; + expect(emitterTypeTest).to.be.ok; + }); + it('should get name & data from NodeEventEmitterNameDataPair', () => { // type EVENT_PAIR = TypeEventPair type EVENT_PAIR = NodeEventEmitterNameDataPair; diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index 8044ff9da9..d8a7de818e 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -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, NodeEventEmitterDataType } from '../util/NodeEventEmitterDataType'; // These constants are used to create handler registry functions using array mapping below. const nodeEventEmitterMethods = ['addListener', 'removeListener'] as const; @@ -78,6 +79,15 @@ export function fromEvent( resultSelector: (event: T) => R ): Observable; +/** + * Automatically infer from Node.js EventEmitter with event data types + * @see https://github.com/ReactiveX/rxjs/pull/6669 + */ +export function fromEvent>( + target: E | ArrayLike, + eventName: T +): Observable>; + export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index a109a0d91a..31fb4bf38a 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -105,11 +105,4 @@ interface NamedNodeEventEmitter { removeListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; } -export function fromEvent>( - target: E | ArrayLike, - eventName: T -): Observable> { - return {} as any; -} - -export type { NodeEventEmitterDataType, NodeEventEmitterNameDataPair }; +export type { NamedNodeEventEmitter, NodeEventEmitterDataType, NodeEventEmitterNameDataPair }; From 42aee0de407c21fd8f7b0d2142fbeea6e64a42df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 10:42:49 +0800 Subject: [PATCH 15/21] fix(fromEvent): code clean & add unit test for AnyToUnknown type caster --- spec-dtslint/observables/fromEvent-spec.ts | 4 +- spec/util/NodeEventEmitterDataType-spec.ts | 71 +++++++++++++------ src/internal/observable/fromEvent.ts | 4 +- src/internal/util/NodeEventEmitterDataType.ts | 7 +- 4 files changed, 56 insertions(+), 30 deletions(-) diff --git a/spec-dtslint/observables/fromEvent-spec.ts b/spec-dtslint/observables/fromEvent-spec.ts index 3385d81d90..8ef58e387c 100644 --- a/spec-dtslint/observables/fromEvent-spec.ts +++ b/spec-dtslint/observables/fromEvent-spec.ts @@ -51,13 +51,13 @@ it('should support a document source with options and result selector', () => { declare const nodeStyleSource: Pick; it('should support a node-style source', () => { - const source: NodeStyleEventEmitter = nodeStyleSource; + const source: NodeStyleEventEmitter = nodeStyleSource; const a = fromEvent(nodeStyleSource, "exit"); // $ExpectType Observable const b = fromEvent(nodeStyleSource, "exit"); // $ExpectType Observable }); it('should deprecate explicit type parameters for a node-style source', () => { - const source: NodeStyleEventEmitter = nodeStyleSource; + const source: NodeStyleEventEmitter = nodeStyleSource; const a = fromEvent(nodeStyleSource, "exit"); // $ExpectNoDeprecation const b = fromEvent(nodeStyleSource, "exit"); // $ExpectDeprecation }); diff --git a/spec/util/NodeEventEmitterDataType-spec.ts b/spec/util/NodeEventEmitterDataType-spec.ts index 9b82e34bf3..c30cd13677 100644 --- a/spec/util/NodeEventEmitterDataType-spec.ts +++ b/spec/util/NodeEventEmitterDataType-spec.ts @@ -4,36 +4,38 @@ import { NodeEventEmitterNameDataPair, NodeEventEmitterDataType, NamedNodeEventEmitter, + AnyToUnknown, } from '../../src/internal/util/NodeEventEmitterDataType'; -const fooEvent = 'fooEvent'; -const barEvent = 'barEvent'; +describe('NodeEventEmitterDataType smoke testing', () => { -type FOO_DATA = typeof fooEvent; -type BAR_DATA = typeof barEvent; + const fooEvent = 'fooEvent'; + const barEvent = '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; } + type FOO_DATA = typeof fooEvent; + type BAR_DATA = typeof barEvent; - 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; } + 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; } - /** - * 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 {} + 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; } - // 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 {} -} + /** + * 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 {} + } -describe('NodeEventEmitterDataType smoke testing', () => { it('should get emitter name & data types correctly', () => { const foo: NodeEventEmitterDataType = fooEvent; const bar: NodeEventEmitterDataType = barEvent; @@ -59,4 +61,27 @@ describe('NodeEventEmitterDataType smoke testing', () => { expect(nameTypeTest).to.be.ok; expect(dataTypeTest).to.be.ok; }); -}); \ No newline at end of file +}); + +describe('AnyToUnknown smoke testing', () => { + it('should only convert any to unknown', () => { + type T_ANY = AnyToUnknown + type T_UNKNOWN = AnyToUnknown + + type T_BOOLEAN = AnyToUnknown + type T_NULL = AnyToUnknown + type T_OBJ = AnyToUnknown + type T_STRING = AnyToUnknown + type T_UNDEFINED = AnyToUnknown + type T_VOID = AnyToUnknown + + type UNKNOWN_TYPE = T_ANY & T_UNKNOWN + type KNOWN_TYPE = T_VOID | T_BOOLEAN | T_STRING | T_UNDEFINED | T_NULL | T_OBJ + + const unknownTypeTest: unknown extends UNKNOWN_TYPE ? true : never = true; + const knownTypeTest: unknown extends KNOWN_TYPE ? never : true = true; + + expect(unknownTypeTest).to.be.true; + expect(knownTypeTest).to.be.true; + }); +}); diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index d8a7de818e..f5e2f96de9 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -80,7 +80,9 @@ export function fromEvent( ): Observable; /** - * Automatically infer from Node.js EventEmitter with event data types + * 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>( diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index 31fb4bf38a..604c6abab8 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -16,7 +16,6 @@ /* eslint-disable no-use-before-define */ /* eslint-disable max-len */ -import { Observable } from '../Observable'; /** * Node.js EventEmitter Add/Remove Listener interface @@ -83,7 +82,7 @@ type EventNameDataPair = null extends T ? (void extends T ? unknown : T) : T; +type AnyToUnknown = unknown extends T ? unknown : T; // the [eventName, eventData] types array type NodeEventEmitterNameDataPair> = EventNameDataPair; @@ -95,7 +94,7 @@ type NodeEventEmitterNameDataPair, T> = Extract, [T, any]>[1]; @@ -105,4 +104,4 @@ interface NamedNodeEventEmitter { removeListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; } -export type { NamedNodeEventEmitter, NodeEventEmitterDataType, NodeEventEmitterNameDataPair }; +export type { AnyToUnknown, NamedNodeEventEmitter, NodeEventEmitterDataType, NodeEventEmitterNameDataPair }; From f2638fa590ccc38eb5fe699ef0ea1a984de37684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 14:46:23 +0800 Subject: [PATCH 16/21] fix(fromEvent): code clean --- spec/util/NodeEventEmitterDataType-spec.ts | 32 +++++++++++++++++-- src/internal/observable/fromEvent.ts | 4 +-- src/internal/util/NodeEventEmitterDataType.ts | 13 +++++++- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/spec/util/NodeEventEmitterDataType-spec.ts b/spec/util/NodeEventEmitterDataType-spec.ts index c30cd13677..42bc4b5e1a 100644 --- a/spec/util/NodeEventEmitterDataType-spec.ts +++ b/spec/util/NodeEventEmitterDataType-spec.ts @@ -3,12 +3,12 @@ import { expect } from 'chai'; import { NodeEventEmitterNameDataPair, NodeEventEmitterDataType, + NodeEventEmitterDataTypeUnknown, NamedNodeEventEmitter, AnyToUnknown, } from '../../src/internal/util/NodeEventEmitterDataType'; describe('NodeEventEmitterDataType smoke testing', () => { - const fooEvent = 'fooEvent'; const barEvent = 'barEvent'; @@ -61,6 +61,33 @@ describe('NodeEventEmitterDataType smoke testing', () => { expect(nameTypeTest).to.be.ok; expect(dataTypeTest).to.be.ok; }); + + it('should get `unknown` for `process` events by NodeEventEmitterDataTypeUnknown', () => { + let exit: NodeEventEmitterDataTypeUnknown< + Pick< + typeof process, + 'addListener' | 'removeListener' + >, + 'exit' + >; + exit = true; + exit = 42; + exit = 'unknown'; + + expect(exit).to.be.ok; + }); + + it('should get `never` for `process` events by NodeEventEmitterDataType', () => { + const exit: NodeEventEmitterDataType< + Pick< + typeof process, + 'addListener' | 'removeListener' + >, + 'exit' + > extends never ? true : never = true; + + expect(exit).to.be.ok; + }); }); describe('AnyToUnknown smoke testing', () => { @@ -74,9 +101,10 @@ describe('AnyToUnknown smoke testing', () => { type T_STRING = AnyToUnknown type T_UNDEFINED = AnyToUnknown type T_VOID = AnyToUnknown + type T_NEVER = AnyToUnknown type UNKNOWN_TYPE = T_ANY & T_UNKNOWN - type KNOWN_TYPE = T_VOID | T_BOOLEAN | T_STRING | T_UNDEFINED | T_NULL | T_OBJ + type KNOWN_TYPE = T_VOID | T_BOOLEAN | T_STRING | T_UNDEFINED | T_NULL | T_OBJ | T_NEVER const unknownTypeTest: unknown extends UNKNOWN_TYPE ? true : never = true; const knownTypeTest: unknown extends KNOWN_TYPE ? never : true = true; diff --git a/src/internal/observable/fromEvent.ts b/src/internal/observable/fromEvent.ts index f5e2f96de9..44a670f4a1 100644 --- a/src/internal/observable/fromEvent.ts +++ b/src/internal/observable/fromEvent.ts @@ -4,7 +4,7 @@ import { mergeMap } from '../operators/mergeMap'; import { isArrayLike } from '../util/isArrayLike'; import { isFunction } from '../util/isFunction'; import { mapOneOrManyArgs } from '../util/mapOneOrManyArgs'; -import { NamedNodeEventEmitter, NodeEventEmitterDataType } from '../util/NodeEventEmitterDataType'; +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; @@ -88,7 +88,7 @@ export function fromEvent( export function fromEvent>( target: E | ArrayLike, eventName: T -): Observable>; +): Observable>; export function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; /** @deprecated Do not specify explicit type parameters. Signatures with type parameters that cannot be inferred will be removed in v8. */ diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index 604c6abab8..acea99a724 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -99,9 +99,20 @@ type NodeEventEmitterNameDataPair, T> = Extract, [T, any]>[1]; +// Convert `never` to `unknown` +type NodeEventEmitterDataTypeUnknown, T> = NodeEventEmitterDataType extends never + ? unknown + : NodeEventEmitterDataType; + interface NamedNodeEventEmitter { addListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; removeListener(name: N, handler: (data: NodeEventEmitterDataType, N>, ...args: any[]) => any): this; } -export type { AnyToUnknown, NamedNodeEventEmitter, NodeEventEmitterDataType, NodeEventEmitterNameDataPair }; +export type { + AnyToUnknown, + NamedNodeEventEmitter, + NodeEventEmitterDataType, + NodeEventEmitterDataTypeUnknown, + NodeEventEmitterNameDataPair, +}; From fa79faf34f463ee6ee44afd3648c6e6e60c98ff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 14:52:09 +0800 Subject: [PATCH 17/21] fix(fromEvent): update api guard --- api_guard/dist/types/index.d.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index 4494eb75d9..cd6a43d8d7 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -257,9 +257,10 @@ export declare function fromEvent(target: HasEventTargetAddRemove | ArrayL export declare function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string, resultSelector: (event: T) => R): Observable; export declare function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string, options: EventListenerOptions): Observable; export declare function fromEvent(target: HasEventTargetAddRemove | ArrayLike>, eventName: string, options: EventListenerOptions, resultSelector: (event: T) => R): Observable; -export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike>, eventName: string): Observable>; -export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike>, eventName: string): Observable; -export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike>, eventName: string, resultSelector: (...args: any[]) => R): Observable; +export declare function fromEvent>(target: E | ArrayLike, eventName: T): Observable>; +export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; +export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string): Observable; +export declare function fromEvent(target: NodeStyleEventEmitter | ArrayLike, eventName: string, resultSelector: (...args: any[]) => R): Observable; export declare function fromEvent(target: NodeCompatibleEventEmitter | ArrayLike, eventName: string): Observable; export declare function fromEvent(target: NodeCompatibleEventEmitter | ArrayLike, eventName: string): Observable; export declare function fromEvent(target: NodeCompatibleEventEmitter | ArrayLike, eventName: string, resultSelector: (...args: any[]) => R): Observable; From d6da834b9e9f6ee2cbb69e65504bc773774626e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 15:02:37 +0800 Subject: [PATCH 18/21] fix(fromEvent): CI green! code clean --- src/internal/util/NodeEventEmitterDataType.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index acea99a724..f459a5b9cd 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -58,7 +58,6 @@ type EventNameDataPair4 = AddRemoveListener extends L4< ? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] : never; -// type NodeEventEmitterAddRemoveListener = (name: N, listener: (data: D, ...args: any[]) => any) => any; interface HasNodeEventEmitterAddRemove { addListener(name: N, listener: (data: D, ...args: any[]) => void): this; removeListener(name: N, listener: (data: D, ...args: any[]) => void): this; @@ -67,7 +66,7 @@ interface HasNodeEventEmitterAddRemove { /** * Get the event name/data pair types from an event emitter * - * @example `['foo', number] | ['bar', string]` + * @return `['foo', number] | ['bar', string]` */ type EventNameDataPair['addListener']> = | EventNameDataPair4 @@ -78,7 +77,7 @@ type EventNameDataPair === unknown` + * @return `AnyToUnknown -> unknown` * * TODO: huan(202111) need to be tested more and confirm it has no bug in edge cases */ @@ -87,11 +86,6 @@ type AnyToUnknown = unknown extends T ? unknown : T; // the [eventName, eventData] types array type NodeEventEmitterNameDataPair> = EventNameDataPair; -// the event names -// type NodeEventEmitterName> = NodeEventEmitterNameDataPair[0]; -// // The types of `args[0]` defined by the listeners -// type NodeEventEmitterData> = NodeEventEmitterNameDataPair[1]; - /** * * Tada! Get event emitter data type by event name 8-D From 1554c9f56580fecdd7243db6f8a6bed439a019b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 15:34:40 +0800 Subject: [PATCH 19/21] fix(fromEvent): increase overload max number to 9 --- src/internal/util/NodeEventEmitterDataType.ts | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index f459a5b9cd..bfaf80518d 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -37,6 +37,35 @@ interface L1 extends L {} interface L2 extends L, L {} interface L3 extends L, L, L {} interface L4 extends L, L, L, L {} +interface L5 extends L, L, L, L, L {} +interface L6 extends L, L, L, L, L, L {} +interface L7 + extends L, + L, + L, + L, + L, + L, + L {} +interface L8 + extends L, + L, + L, + L, + L, + L, + L, + L {} +interface L9 + extends L, + L, + L, + L, + L, + L, + L, + L, + L {} type EventNameDataPair1 = AddRemoveListener extends L1 ? [N1, D1] : never; type EventNameDataPair2 = AddRemoveListener extends L2 @@ -57,6 +86,96 @@ type EventNameDataPair4 = AddRemoveListener extends L4< > ? [N1, D1] | [N2, D2] | [N3, D3] | [N4, D4] : never; +type EventNameDataPair5 = 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 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 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 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 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 { addListener(name: N, listener: (data: D, ...args: any[]) => void): this; From 7b7739e038205f4d142b5030b35523ddeee70ab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Mon, 8 Nov 2021 16:26:31 +0800 Subject: [PATCH 20/21] fix(fromEvent): increase overload max number to 9 --- src/internal/util/NodeEventEmitterDataType.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index bfaf80518d..fde4cf092b 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -188,6 +188,11 @@ interface HasNodeEventEmitterAddRemove { * @return `['foo', number] | ['bar', string]` */ type EventNameDataPair['addListener']> = + | EventNameDataPair9 + | EventNameDataPair8 + | EventNameDataPair7 + | EventNameDataPair6 + | EventNameDataPair5 | EventNameDataPair4 | EventNameDataPair3 | EventNameDataPair2 From 88acd54fcb129bb3454125f342db9acf4d4f68ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Huan=20=28=E6=9D=8E=E5=8D=93=E6=A1=93=29?= Date: Thu, 2 Dec 2021 21:28:50 +0800 Subject: [PATCH 21/21] chore(fromEvent): move types test to dtslint & remove comments --- spec-dtslint/observables/fromEvent-spec.ts | 21 +++++++ .../util/NodeEventEmitterDataType-spec.ts | 55 ++++++------------- spec/observables/fromEvent-spec.ts | 29 ---------- src/internal/util/NodeEventEmitterDataType.ts | 16 ------ 4 files changed, 39 insertions(+), 82 deletions(-) rename {spec => spec-dtslint}/util/NodeEventEmitterDataType-spec.ts (68%) diff --git a/spec-dtslint/observables/fromEvent-spec.ts b/spec-dtslint/observables/fromEvent-spec.ts index 8ef58e387c..69b89b6265 100644 --- a/spec-dtslint/observables/fromEvent-spec.ts +++ b/spec-dtslint/observables/fromEvent-spec.ts @@ -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 }); + +/** + * 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 + const bar$ = fromEvent(test, 'bar'); // $ExpectType Observable +}); diff --git a/spec/util/NodeEventEmitterDataType-spec.ts b/spec-dtslint/util/NodeEventEmitterDataType-spec.ts similarity index 68% rename from spec/util/NodeEventEmitterDataType-spec.ts rename to spec-dtslint/util/NodeEventEmitterDataType-spec.ts index 42bc4b5e1a..b90aa0a301 100644 --- a/spec/util/NodeEventEmitterDataType-spec.ts +++ b/spec-dtslint/util/NodeEventEmitterDataType-spec.ts @@ -1,14 +1,11 @@ -import { expect } from 'chai'; - import { NodeEventEmitterNameDataPair, NodeEventEmitterDataType, NodeEventEmitterDataTypeUnknown, - NamedNodeEventEmitter, AnyToUnknown, } from '../../src/internal/util/NodeEventEmitterDataType'; -describe('NodeEventEmitterDataType smoke testing', () => { +it('NodeEventEmitterDataType smoke testing', () => { const fooEvent = 'fooEvent'; const barEvent = 'barEvent'; @@ -37,60 +34,46 @@ describe('NodeEventEmitterDataType smoke testing', () => { } it('should get emitter name & data types correctly', () => { - const foo: NodeEventEmitterDataType = fooEvent; - const bar: NodeEventEmitterDataType = barEvent; - - expect(foo).to.be.ok; - expect(bar).to.be.ok; - }); - - it('should extendable', () => { - const emitterTypeTest: NodeEventEmitterFixture extends NamedNodeEventEmitter<'foo'> ? true : never = true; - expect(emitterTypeTest).to.be.ok; + // $ExpectType "fooEvent" + type Foo = NodeEventEmitterDataType; + // $ExpectType "barEvent" + type Bar = NodeEventEmitterDataType; }); it('should get name & data from NodeEventEmitterNameDataPair', () => { // type EVENT_PAIR = TypeEventPair type EVENT_PAIR = NodeEventEmitterNameDataPair; + + // $ExpectType "foo" | "bar" type EVENT_NAME = EVENT_PAIR[0]; + // $ExpecTType FOO_DATA | BAR_DATA type EVENT_DATA = EVENT_PAIR[1]; - - const nameTypeTest: EVENT_NAME extends 'foo' | 'bar' ? true : never = true; - const dataTypeTest: EVENT_DATA extends FOO_DATA | BAR_DATA ? true : never = true; - - expect(nameTypeTest).to.be.ok; - expect(dataTypeTest).to.be.ok; }); it('should get `unknown` for `process` events by NodeEventEmitterDataTypeUnknown', () => { - let exit: NodeEventEmitterDataTypeUnknown< + // $ExpectType unknown + type Exit = NodeEventEmitterDataTypeUnknown< Pick< typeof process, 'addListener' | 'removeListener' >, 'exit' >; - exit = true; - exit = 42; - exit = 'unknown'; - - expect(exit).to.be.ok; }); it('should get `never` for `process` events by NodeEventEmitterDataType', () => { - const exit: NodeEventEmitterDataType< + // $ExpectType never + type Exit = NodeEventEmitterDataType< Pick< typeof process, 'addListener' | 'removeListener' >, 'exit' - > extends never ? true : never = true; - - expect(exit).to.be.ok; + > }); }); -describe('AnyToUnknown smoke testing', () => { +it('AnyToUnknown smoke testing', () => { it('should only convert any to unknown', () => { type T_ANY = AnyToUnknown type T_UNKNOWN = AnyToUnknown @@ -103,13 +86,11 @@ describe('AnyToUnknown smoke testing', () => { type T_VOID = AnyToUnknown type T_NEVER = AnyToUnknown + // $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 - - const unknownTypeTest: unknown extends UNKNOWN_TYPE ? true : never = true; - const knownTypeTest: unknown extends KNOWN_TYPE ? never : true = true; - expect(unknownTypeTest).to.be.true; - expect(knownTypeTest).to.be.true; + 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 }); }); diff --git a/spec/observables/fromEvent-spec.ts b/spec/observables/fromEvent-spec.ts index 10e268b51f..2b8edfdbbd 100644 --- a/spec/observables/fromEvent-spec.ts +++ b/spec/observables/fromEvent-spec.ts @@ -433,33 +433,4 @@ describe('fromEvent', () => { expect(nodeList[1]._removeEventListenerArgs).to.deep.equal(nodeList[1]._addEventListenerArgs); }); - /** - * 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(); - - expect(() => { - const foo$ = fromEvent(test, 'foo'); - const fooTypeTest: typeof foo$ extends Observable ? true : never = true; - expect(fooTypeTest).to.be.true; - - const bar$ = fromEvent(test, 'bar'); - const barTypeTest: typeof bar$ extends Observable ? true : never = true; - expect(barTypeTest).to.be.true; - - done(); - }).to.not.throw(TypeError); - }); }); diff --git a/src/internal/util/NodeEventEmitterDataType.ts b/src/internal/util/NodeEventEmitterDataType.ts index fde4cf092b..491ee62019 100644 --- a/src/internal/util/NodeEventEmitterDataType.ts +++ b/src/internal/util/NodeEventEmitterDataType.ts @@ -1,19 +1,3 @@ -/** - * Issue #6669 - fix(fromEvent): infer from Node.js EventEmitter with types - * @see https://github.com/ReactiveX/rxjs/pull/6669 - * - * Author: Huan - * Date: Nov 7, 2021 - * - * TypeScript has design limitation that prevents us from inferring from the overload functins: - * > ReturnType or the use of infer in a function parameter or return position does handle overloads. - * the last overload is used for inference as it is assumed to be the most general. - * - @mhegazy @link https://github.com/Microsoft/TypeScript/issues/24275#issuecomment-390701982 - * - * This file is created by @huan and it was inspired from the brilliant work from @Aidin on StackOverflow: - * @link https://stackoverflow.com/a/60822641/1123955 - */ - /* eslint-disable no-use-before-define */ /* eslint-disable max-len */