From ce0a2fa975e7c08de2bbf893010f2c25c090b1ca Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Thu, 15 Apr 2021 10:30:57 -0500 Subject: [PATCH] fix: forkJoin/combineLatest return Observable if passed any (#6227) * fix: forkJoin/combineLatest return Observable if passed any Resolves #6226 * chore: make shenanigans readonly Co-authored-by: Daniel Rosenwasser * chore: Ensure there is no way the arg should work If the user passed that exact shape of object in AnyCatcher, technically it should work. This ensures there's no way it would work properly so `Observable` is accurate. Also adds some documentation to the areas * refactor: Move any catcher signatures up Co-authored-by: Daniel Rosenwasser --- api_guard/dist/types/index.d.ts | 2 ++ spec-dtslint/observables/combineLatest-spec.ts | 5 +++++ spec-dtslint/observables/forkJoin-spec.ts | 5 +++++ spec-dtslint/observables/of-spec.ts | 6 ++++++ src/internal/AnyCatcher.ts | 10 ++++++++++ src/internal/observable/combineLatest.ts | 14 ++++++++++++++ src/internal/observable/forkJoin.ts | 15 +++++++++++++++ 7 files changed, 57 insertions(+) create mode 100644 src/internal/AnyCatcher.ts diff --git a/api_guard/dist/types/index.d.ts b/api_guard/dist/types/index.d.ts index 71d8c83c8d..9667748ba4 100644 --- a/api_guard/dist/types/index.d.ts +++ b/api_guard/dist/types/index.d.ts @@ -40,6 +40,7 @@ export declare function bindCallback void, resultSelector: (...args: any[]) => any, scheduler?: SchedulerLike): (...args: any[]) => Observable; export declare function bindNodeCallback(callbackFunc: (...args: [...A, (err: any, ...res: R) => void]) => void, schedulerLike?: SchedulerLike): (...arg: A) => Observable; +export declare function combineLatest(arg: T): Observable; export declare function combineLatest(sources: []): Observable; export declare function combineLatest(sources: readonly [...ObservableInputTuple]): Observable; export declare function combineLatest(sources: readonly [...ObservableInputTuple], resultSelector: (...values: A) => R, scheduler: SchedulerLike): Observable; @@ -125,6 +126,7 @@ export declare type Falsy = null | undefined | false | 0 | -0 | 0n | ''; export declare function firstValueFrom(source: Observable): Promise; +export declare function forkJoin(arg: T): Observable; export declare function forkJoin(scheduler: null | undefined): Observable; export declare function forkJoin(sources: readonly []): Observable; export declare function forkJoin(sources: readonly [...ObservableInputTuple]): Observable; diff --git a/spec-dtslint/observables/combineLatest-spec.ts b/spec-dtslint/observables/combineLatest-spec.ts index e3462f9c1a..2ebb408c98 100644 --- a/spec-dtslint/observables/combineLatest-spec.ts +++ b/spec-dtslint/observables/combineLatest-spec.ts @@ -147,3 +147,8 @@ describe('combineLatest({})', () => { const res = combineLatest(obj); // $ExpectError }); }); + +it('should take in any and return Observable because we do not know if it is an array or object', () => { + const arg: any = null; + const res = combineLatest(arg); // $ExpectType Observable +}); \ No newline at end of file diff --git a/spec-dtslint/observables/forkJoin-spec.ts b/spec-dtslint/observables/forkJoin-spec.ts index 226a731e8a..6d2f2e0f9d 100644 --- a/spec-dtslint/observables/forkJoin-spec.ts +++ b/spec-dtslint/observables/forkJoin-spec.ts @@ -117,4 +117,9 @@ describe('forkJoin([])', () => { it('should force user cast for array of 6+ observables', () => { const res = forkJoin([of(1, 2, 3), of('a', 'b', 'c'), of(1, 2, 3), of(1, 2, 3), of(1, 2, 3), of(1, 2, 3), of(1, 2, 3)]); // $ExpectType Observable<[number, string, number, number, number, number, number]> }); + + it('should return unknown for argument of any', () => { + const arg: any = null; + const res = forkJoin(arg); // $ExpectType Observable + }) }); diff --git a/spec-dtslint/observables/of-spec.ts b/spec-dtslint/observables/of-spec.ts index c2fad5f300..58a6133586 100644 --- a/spec-dtslint/observables/of-spec.ts +++ b/spec-dtslint/observables/of-spec.ts @@ -136,4 +136,10 @@ it('should deprecate correctly', () => { of(a, b); // $ExpectNoDeprecation of(a, b, c); // $ExpectNoDeprecation of(a, b, c, d); // $ExpectNoDeprecation +}); + +it('should handle null and undefined properly', () => { + const a = of(undefined); // $ExpectType Observable + const b = of(null); // $ExpectType Observable + const c = [of(1), of(2), of(undefined), of(3)] as const; }); \ No newline at end of file diff --git a/src/internal/AnyCatcher.ts b/src/internal/AnyCatcher.ts new file mode 100644 index 0000000000..b42ff7a7b8 --- /dev/null +++ b/src/internal/AnyCatcher.ts @@ -0,0 +1,10 @@ +/** + * This is just a type that we're using to identify `any` being passed to + * function overloads. This is used because of situations like {@link forkJoin}, + * where it could return an `Observable` or an `Observable<{ [key: K]: T }>`, + * so `forkJoin(any)` would mean we need to return `Observable`. + * + * @internal + */ +declare const anyCatcherSymbol: unique symbol; +export type AnyCatcher = typeof anyCatcherSymbol; diff --git a/src/internal/observable/combineLatest.ts b/src/internal/observable/combineLatest.ts index df116b24a2..b915b4192f 100644 --- a/src/internal/observable/combineLatest.ts +++ b/src/internal/observable/combineLatest.ts @@ -9,6 +9,20 @@ import { mapOneOrManyArgs } from '../util/mapOneOrManyArgs'; import { popResultSelector, popScheduler } from '../util/args'; import { createObject } from '../util/createObject'; import { OperatorSubscriber } from '../operators/OperatorSubscriber'; +import { AnyCatcher } from '../AnyCatcher'; + +// combineLatest(any) +// We put this first because we need to catch cases where the user has supplied +// _exactly `any`_ as the argument. Since `any` literally matches _anything_, +// we don't want it to randomly hit one of the other type signatures below, +// as we have no idea at build-time what type we should be returning when given an any. + +/** + * You have passed `any` here, we can't figure out if it is + * an array or an object, so you're getting `unknown`. Use better types. + * @param arg Something typed as `any` + */ +export function combineLatest(arg: T): Observable; // combineLatest([a, b, c]) export function combineLatest(sources: []): Observable; diff --git a/src/internal/observable/forkJoin.ts b/src/internal/observable/forkJoin.ts index 6b771ce938..d8d0e56242 100644 --- a/src/internal/observable/forkJoin.ts +++ b/src/internal/observable/forkJoin.ts @@ -6,7 +6,22 @@ import { popResultSelector } from '../util/args'; import { OperatorSubscriber } from '../operators/OperatorSubscriber'; import { mapOneOrManyArgs } from '../util/mapOneOrManyArgs'; import { createObject } from '../util/createObject'; +import { AnyCatcher } from '../AnyCatcher'; +// forkJoin(any) +// We put this first because we need to catch cases where the user has supplied +// _exactly `any`_ as the argument. Since `any` literally matches _anything_, +// we don't want it to randomly hit one of the other type signatures below, +// as we have no idea at build-time what type we should be returning when given an any. + +/** + * You have passed `any` here, we can't figure out if it is + * an array or an object, so you're getting `unknown`. Use better types. + * @param arg Something typed as `any` + */ +export function forkJoin(arg: T): Observable; + +// forkJoin(null | undefined) export function forkJoin(scheduler: null | undefined): Observable; // forkJoin([a, b, c])