Ava: ObservableLike type definition is incorrect

[micaelmbagira]

Description

As I commented in this closed issue https://github.com/avajs/ava/issues/1685#issuecomment-520885556, the ObservableLike definition in ava 2.2.0 is incorrect. It expects the property [Symbol.observable]() but this property is not in rxjs Observable type.

Test Source

import { of } from 'rxjs';

test('foo', t => {
  return of(3).pipe(tap(() => t.pass()));
});

Error Message & Stack Trace

  error TS2345: Argument of type '"foo"' is not assignable to parameter of type 'OneOrMoreMacros<[(t: ExecutionContext<{}>) => Observable<number>], {}>'.

  test('foo', t => {
         ~~~~~

Removing the [Symbol.observable](): ObservableLike; solves it though....

When trying to enforce the ObservableLike type with return of(3).pipe(tap(() => t.pass())) as ObservableLike;. TypeScript would throw

Conversion of type 'Observable' to type 'ObservableLike' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
Property '[Symbol.observable]' is missing in type 'Observable' but required in type 'ObservableLike'

So basically, [Symbol.observable]() is not in rxjs Observable type definition. Can we then reopen this issue?

Expected behaviour

The returned Observable should match the ObservableLike type and the t => ... should not infer the type OneOrMoreMacros.

[novemberborn]

t => ... should not infer the type OneOrMoreMacros.

I think this is just the type checker doing its thing.

The returned Observable should match the ObservableLike type

@sindresorhus?

[sindresorhus]

It expects the property Symbol.observable but this property is not in rxjs Observable type.

This is the RxJS ObservableLike type:

export interface Subscribable<T> {
    subscribe(observer?: PartialObserver<T>): Unsubscribable;
    /** @deprecated Use an observer instead of a complete callback */
    subscribe(next: null | undefined, error: null | undefined, complete: () => void): Unsubscribable;
    /** @deprecated Use an observer instead of an error callback */
    subscribe(next: null | undefined, error: (error: any) => void, complete?: () => void): Unsubscribable;
    /** @deprecated Use an observer instead of a complete callback */
    subscribe(next: (value: T) => void, error: null | undefined, complete: () => void): Unsubscribable;
    subscribe(next?: (value: T) => void, error?: (error: any) => void, complete?: () => void): Unsubscribable;
}
export declare type ObservableLike<T> = InteropObservable<T>;
export declare type InteropObservable<T> = {
    [Symbol.observable]: () => Subscribable<T>;
};

Notice the [Symbol.observable]: () => Subscribable<T>;.

[micaelmbagira]

@sindresorhus this definition is deprecated but most important, when you type an observable obs: Observable<T>, then the type declaration that is used is not ObservableLike but export declare class Observable<T>.... that you can find in node_modules/rxjs/internal/Observable.d.ts

[micaelmbagira]

@sindresorhus @novemberborn I would be happy to open a PR and remove the [Symbol.observable](): ObservableLike;. Based on my previous message, is that an acceptable fix?

[novemberborn]

@micaelmbagira it seems to be used with InteropObservable though? We're not looking to support just RxJS.

I don't use observables and they're not actively being standardized. Honestly I'm not sure AVA should be supporting them to begin with. But, they are at the moment, and I'd like it to work without getting bogged down in these details. So I'm OK with removing this requirement but I really need to defer to @sindresorhus on this.

[stavalfi]

is there any progress? types are completely wrong.

even a simple example have types errors:

import {Subject} from 'rxjs'
test.beforeEach(t => {
  return new Subject()
})

Error:(13, 24) TS2769: No overload matches this call.
Overload 1 of 4, '(implementation: Implementation<{ suricate: Suricate; client: Server; closeExistingConnections: () => void; }>): void', gave the following error.
Argument of type '(t: ExecutionContext<{ suricate: Suricate; client: Server; closeExistingConnections: () => void; }>) => Subject' is not assignable to parameter of type 'Implementation<{ suricate: Suricate; client: Server; closeExistingConnections: () => void; }>'.
Type 'Subject' is not assignable to type 'ImplementationResult'.
Property '[Symbol.observable]' is missing in type 'Subject' but required in type 'ObservableLike'.
Overload 2 of 4, '(macros: OneOrMoreMacros<[], { suricate: Suricate; client: Server; closeExistingConnections: () => void; }>): void', gave the following error.
Argument of type '(t: ExecutionContext<{ suricate: Suricate; client: Server; closeExistingConnections: () => void; }>) => Subject' is not assignable to parameter of type 'OneOrMoreMacros<[], { suricate: Suricate; client: Server; closeExistingConnections: () => void; }>'.
Type '(t: ExecutionContext<{ suricate: Suricate; client: Server; closeExistingConnections: () => void; }>) => Subject' is not assignable to type 'UntitledMacro<[], { suricate: Suricate; client: Server; closeExistingConnections: () => void; }>'.
Type 'Subject' is not assignable to type 'ImplementationResult'.
Type 'Subject' is not assignable to type 'ObservableLike'.

[novemberborn]

@stavalfi I don't use observables. I'm quite confused since Symbol.observable is not standardized, and then there are various observable libraries but they don't necessary agree with each other?

[stavalfi]

@benlesh, can you answer?

[micaelmbagira]

@novemberborn then again, since Symbol.observable is not standardized and not even used anymore in RxJS (which is the most used Observable library), I suggest we remove [Symbol.observable](): ObservableLike;. Would that solve your problem @stavalfi ?

If you agree with the approach, @novemberborn I can open the PR.

[stavalfi]

@micaelmbagira I don't know, but you can easly test it with this example:

import {Subject} from 'rxjs'
test.beforeEach(t => {
  return new Subject()
})

[micaelmbagira]

@stavalfi Just tried

  ✖ No tests found in index.spec.ts

  1 uncaught exception

  Uncaught exception in index.spec.ts

  index.spec.ts:4:17 - error TS2769: No overload matches this call.
    Overload 1 of 4, '(implementation: Implementation<unknown>): void', gave the following error.
      Argument of type '(t: ExecutionContext<unknown>) => Subject<unknown>' is not assignable to parameter of type 'Implementation<unknown>'.
        Type 'Subject<unknown>' is not assignable to type 'ImplementationResult'.
          Property '[Symbol.observable]' is missing in type 'Subject<unknown>' but required in type 'ObservableLike'.
    Overload 2 of 4, '(macros: OneOrMoreMacros<[], unknown>): void', gave the following error.
      Argument of type '(t: ExecutionContext<unknown>) => Subject<unknown>' is not assignable to parameter of type 'OneOrMoreMacros<[], unknown>'.
        Type '(t: ExecutionContext<unknown>) => Subject<unknown>' is not assignable to type 'UntitledMacro<[], unknown>'.
          Type 'Subject<unknown>' is not assignable to type 'ImplementationResult'.
            Type 'Subject<unknown>' is not assignable to type 'ObservableLike'.

  4 test.beforeEach(t => {
                    ~~~~~~

    node_modules/ava/index.d.ts:5:2
      5  [Symbol.observable](): ObservableLike;
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      '[Symbol.observable]' is declared here.

this is the error, and indeed getting rid of [Symbol.observable](): ObservableLike; fixes it.

[stavalfi]

@micaelmbagira cool! then a fix would be great! Thanks!!

[novemberborn]

Type definitions aside, right now, AVA supports "observable" test implementations. This is built on the following detection logic:

https://github.com/sindresorhus/is-observable/blob/ca9746cc390d7f941afe5af8a6d71ac114392ebe/index.js#L9:L15

if (value[Symbol.observable] && value === value[Symbol.observable]()) {
    return true;
}


if (value['@@observable'] && value === value['@@observable']()) {
    return true;
}

Only when the object returned by the test implementation has either of these properties will we subscribe to it.

So, sure, we could remove Symbol.observable from the type definition. That does not give any guarantees though as to what return value is recognized as an observable at runtime. This, I imagine, is why Symbol.observable was added to the type definition to begin with.

@micaelmbagira, regarding this:

Symbol.observable is [...] not even used anymore in RxJS (which is the most used Observable library)

How does RxJS "tag" observables then? The '@@observable' string? I'm happy to change the type to recognize that instead, though we may want to push updates to AVA's observable dependencies as well to remove the Symbol.observable checks.

Technically this is also a breaking change in our type definition although the subset of folks who would be affected probably wouldn't mind it 😄

[micaelmbagira]

Actually, my bad @novemberborn I checked again and I am wrong regarding how I think it should be fixed.

• ObservableLike has the [Symbol.observable]() tag https://github.com/ReactiveX/rxjs/blob/56cbd220dac11ec58751e15a23a56d6747964df2/src/internal/types.ts#L55-L57
• However if I do const s = from([1, 2, 3]);, the inferred type is Observable and because it doesn't have [Symbol.observable](), it breaks.

So ideally, ImplementationResult type should be SubscribableOrPromise. Not sure what's the best way to achieve that, maybe add a Subscribable or SubscribableLike type?

[novemberborn]

@micaelmbagira I think if we say any implementation result that has a subscribe() method, which takes error and complete callbacks, then we can treat it as an observable and convert it to a promise?

[micaelmbagira]

@novemberborn How would you do that?

What about this?

interface SubscribableLike {
    subscribe(observer?: (value: unknown) => void): void;
    subscribe(next?: (value: any) => void, error?: (error: any) => void, complete?: () => void): void;
}

export type ImplementationResult = PromiseLike<void> | ObservableLike | SubscribableLike | void;

Inspired from these lines

[novemberborn]

@micaelmbagira yes that's my thinking, however the type definition has to match the way we call the subscribe function, in order to observe errors / completion.

Which means we can't support two signatures. We wouldn't know which to call.

[micaelmbagira]

@novemberborn Ok I will open a PR with this way