Skip to content

Commit

Permalink
fix: forkJoin/combineLatest return Observable<unknown> if passed any (#…
Browse files Browse the repository at this point in the history
…6227)

* fix: forkJoin/combineLatest return Observable<unknown> if passed any

Resolves #6226

* chore: make shenanigans readonly

Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>

* 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<unknown>` is accurate.

Also adds some documentation to the areas

* refactor: Move any catcher signatures up

Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
  • Loading branch information
benlesh and DanielRosenwasser committed Apr 15, 2021
1 parent 6fa819b commit ce0a2fa
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 0 deletions.
2 changes: 2 additions & 0 deletions api_guard/dist/types/index.d.ts
Expand Up @@ -40,6 +40,7 @@ export declare function bindCallback<A extends readonly unknown[], R extends rea
export declare function bindNodeCallback(callbackFunc: (...args: any[]) => void, resultSelector: (...args: any[]) => any, scheduler?: SchedulerLike): (...args: any[]) => Observable<any>;
export declare function bindNodeCallback<A extends readonly unknown[], R extends readonly unknown[]>(callbackFunc: (...args: [...A, (err: any, ...res: R) => void]) => void, schedulerLike?: SchedulerLike): (...arg: A) => Observable<R extends [] ? void : R extends [any] ? R[0] : R>;

export declare function combineLatest<T extends AnyCatcher>(arg: T): Observable<unknown>;
export declare function combineLatest(sources: []): Observable<never>;
export declare function combineLatest<A extends readonly unknown[]>(sources: readonly [...ObservableInputTuple<A>]): Observable<A>;
export declare function combineLatest<A extends readonly unknown[], R>(sources: readonly [...ObservableInputTuple<A>], resultSelector: (...values: A) => R, scheduler: SchedulerLike): Observable<R>;
Expand Down Expand Up @@ -125,6 +126,7 @@ export declare type Falsy = null | undefined | false | 0 | -0 | 0n | '';

export declare function firstValueFrom<T>(source: Observable<T>): Promise<T>;

export declare function forkJoin<T extends AnyCatcher>(arg: T): Observable<unknown>;
export declare function forkJoin(scheduler: null | undefined): Observable<never>;
export declare function forkJoin(sources: readonly []): Observable<never>;
export declare function forkJoin<A extends readonly unknown[]>(sources: readonly [...ObservableInputTuple<A>]): Observable<A>;
Expand Down
5 changes: 5 additions & 0 deletions spec-dtslint/observables/combineLatest-spec.ts
Expand Up @@ -147,3 +147,8 @@ describe('combineLatest({})', () => {
const res = combineLatest(obj); // $ExpectError
});
});

it('should take in any and return Observable<unknown> because we do not know if it is an array or object', () => {
const arg: any = null;
const res = combineLatest(arg); // $ExpectType Observable<unknown>
});
5 changes: 5 additions & 0 deletions spec-dtslint/observables/forkJoin-spec.ts
Expand Up @@ -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<unknown>
})
});
6 changes: 6 additions & 0 deletions spec-dtslint/observables/of-spec.ts
Expand Up @@ -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<undefined>
const b = of(null); // $ExpectType Observable<null>
const c = [of(1), of(2), of(undefined), of(3)] as const;
});
10 changes: 10 additions & 0 deletions 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<T[]>` or an `Observable<{ [key: K]: T }>`,
* so `forkJoin(any)` would mean we need to return `Observable<unknown>`.
*
* @internal
*/
declare const anyCatcherSymbol: unique symbol;
export type AnyCatcher = typeof anyCatcherSymbol;
14 changes: 14 additions & 0 deletions src/internal/observable/combineLatest.ts
Expand Up @@ -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<T extends AnyCatcher>(arg: T): Observable<unknown>;

// combineLatest([a, b, c])
export function combineLatest(sources: []): Observable<never>;
Expand Down
15 changes: 15 additions & 0 deletions src/internal/observable/forkJoin.ts
Expand Up @@ -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<T extends AnyCatcher>(arg: T): Observable<unknown>;

// forkJoin(null | undefined)
export function forkJoin(scheduler: null | undefined): Observable<never>;

// forkJoin([a, b, c])
Expand Down

0 comments on commit ce0a2fa

Please sign in to comment.