Skip to content

The catch method of Promise seems unreasonable #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
uinz opened this issue Jun 15, 2023 · 2 comments
Closed

The catch method of Promise seems unreasonable #32

uinz opened this issue Jun 15, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@uinz
Copy link

uinz commented Jun 15, 2023

The original Promise method will union the result of resolve with the result of catch

async function main() {
    const value = await Promise.resolve("1").catch(() => undefined);
    //     ^? const value: string | undefiend
}

But now the catch signature must return the same type as resolve.

interface Promise<T> {
  // ...
  catch(
    onrejected?: ((reason: unknown) => T | PromiseLike<T>) | null | undefined
  ): Promise<T>;
}

In my opinion await task().catch(() => defaultValue) is a good development experience

Similar to rust's unwrap_or

const value = await task().catch(() => defaultValue)

In addition, I think onrejected as optional will also bring some ambiguity.

The .catch() of the following code is actually useless

const value = awiat task().catch()

Therefore, I suggest modifying it as follows

interface Promise<T> {
  // ...
  catch<R>(
    onrejected: ((reason: unknown) => R | PromiseLike<R>)
  ): Promise<T | R>;
}
@uhyo uhyo added the bug Something isn't working label Jun 16, 2023
@aaditmshah
Copy link
Contributor

The original Promise method will union the result of resolve with the result of catch

async function main() {
    const value = await Promise.resolve("1").catch(() => undefined);
    //     ^? const value: string | undefiend
}

But now the catch signature must return the same type as resolve.

interface Promise<T> {
  // ...
  catch(
    onrejected?: ((reason: unknown) => T | PromiseLike<T>) | null | undefined
  ): Promise<T>;
}

Yes, this was intentional. It matches the type signature of the catch function in Haskell.

catch :: Exception e => IO a -> (e -> IO a) -> IO a

In my opinion await task().catch(() => defaultValue) is a good development experience

You can still do that. You just need to be explicit about the type of the awaited value.

@@ -1,4 +1,4 @@
 async function main() {
-  const value = await Promise.resolve('1').catch(() => undefined);
+  const value = await Promise.resolve<string | undefined>('1').catch(() => undefined);
   //     ^? const value: string | undefiend
 }

Similar to rust's unwrap_or

const value = await task().catch(() => defaultValue)

The unwrap_or method in Rust also requires that the type of the default value matches the type of the value contained within the Option. It does not use a different type for default, and return the union of the two types.

impl<T> Option<T> {
  pub fn unwrap_or(self, default: T) -> T {}
}

In addition, I think onrejected as optional will also bring some ambiguity.

The .catch() of the following code is actually useless

const value = awiat task().catch()

Agreed. You should always provide a callback function to .catch. However, I made it optional because omitting the callback function, or providing null or undefined instead of the callback function, is valid. This is because internally .catch(f) calls .then(undefined, f). Besides, this error can be caught by eslint by enabling the promise/valid-params rule.

That being said, I wouldn't mind making the callback function of .catch mandatory.

Therefore, I suggest modifying it as follows

interface Promise<T> {
  // ...
  catch<R>(
    onrejected: ((reason: unknown) => R | PromiseLike<R>)
  ): Promise<T | R>;
}

In my humble opinion, using a different type for the result of .catch is bad because it's too lax. It allows you to return absolutely anything from the .catch handler. Thus, it will implicitly change the type of the awaited value to something that you might not expect. And, as The Zen of Python states, "Explicit is better than implicit." Hence, I think it's better that the result of .catch has the same type as the awaited value. It forces developers to explicitly provide the type of the awaited value.

@aaditmshah
Copy link
Contributor

@uinz Please consider closing this issue if my response solved your problem or was helpful in finding a solution to your problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants