Typing IRejectedPromise and IPendingPromise to hold an unknown value

[jraoult]

When using fromPromise the pending and rejected outcome have a value property typed to any. Now that we have unknown type I thinks in this case it makes more sense and allows us to catch issues a compile-time.

Risks wise, it would introduce an incompatibility with the older versions of TS (<3.0) and would probably make some code that was passing type checking before, fail now.

FYI this is what I used right now as a local override:

declare module "mobx-utils" {
  type BetterPendingPromise = {
    readonly state: "pending";
    readonly value: unknown;
  };

  type BetterRejectedPromise = {
    readonly state: "rejected";
    readonly value: unknown;
  };

  type BetterPromiseObservable<T> = IBasePromiseBasedObservable<T> &
    (BetterPendingPromise | IFulfilledPromise<T> | BetterRejectedPromise);

  export function fromPromise<T>(
    origPromise: PromiseLike<T>,
    oldPromise?: PromiseLike<T>
  ): BetterPromiseObservable<T>;
}

If you're happy with the proposal, I can submit a PR.

[jraoult]

👋 @mweststrate not sure if you are still the main maintainer but you are the last person who edited the lines I'm proposing to change. WDYT?

[NaridaL]

TypeScript itself defines the error "reason" as any, both in Promise and PromiseLike. Although that might be for historical reasons. Have you had concrete problems with the any typing?

At this point I could go either way.

[jraoult]

@NaridaL regarding the error, you are right that it is for historical reason. They actually recently allowed to specified unknwon in a catch clause to enable better type safety (https://devblogs.microsoft.com/typescript/announcing-typescript-4-0/#unknown-on-catch).

Now regarding my proposal, that is actually more about the value than the error, the problem is any is that it causes no problems 😉. So you migh forgot to check the state of the promise and access the value. The compiler wouldn't tell you about it and you would have a false sense of safety. Unknown on the other end is detected by the compiler (cf https://mariusschulz.com/blog/the-unknown-type-in-typescript)

[NaridaL]

Good point about forgetting to check the state. If you create a PR I'll merge it.

[jraoult]

@NaridaL cool I"ll get that ready on Monday then.

Just a heads up that it will make the type definition incompatible with any version of TS below 3.0 and will probably "break" code that was compiling before. The first caveat is probably OK by now since 3.0 was released more than 2 years ago, but if we still want to make sure we don't break existing setup we can make use of the typesVersions field. For the the second one, while annoying, I believe is for the best since it would reveal unchecked code paths.

[jraoult]

Here we go @NaridaL #295