Skip to content
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

Flag to type Promise.catch variables as unknown #45602

Open
5 tasks done
yoav-lavi opened this issue Aug 27, 2021 · 9 comments
Open
5 tasks done

Flag to type Promise.catch variables as unknown #45602

yoav-lavi opened this issue Aug 27, 2021 · 9 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@yoav-lavi
Copy link

yoav-lavi commented Aug 27, 2021

Suggestion

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Following #41016 a flag was added to consider catch variables as unknown.
For consistency and since they're similar cases, I suggest also adding a flag that does this for .catch callbacks on promises.

See also: eslint-plugin-etc/no-implicit-any-catch

@andrewbranch
Copy link
Member

The type for .catch(...) callback parameters is defined in lib files, so we’d have to expose a new type that acts like unknown or any depending on compilation settings... and I’m not sure we’d want that type escaping into the wild 😬. This is a compelling use case, though.

@andrewbranch andrewbranch added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Aug 31, 2021
@Cellule
Copy link

Cellule commented Oct 5, 2021

You can easily achieve this by overloading the signature of Promise.catch in your project

// Promise.d.ts
interface Promise<T> {
  /**
   * Attaches a callback for only the rejection of the Promise.
   * @param onrejected The callback to execute when the Promise is rejected.
   * @returns A Promise for the completion of the callback.
   */
  catch<TResult = never>(
    onrejected?: ((reason: unknown) => TResult | PromiseLike<TResult>) | undefined | null,
  ): Promise<T | TResult>;
}

@yoav-lavi
Copy link
Author

@Cellule Thanks for the snippet, yes, you can do this yourself, but the goal of this suggestion is to add this to TS itself

@fregante
Copy link

fregante commented Dec 6, 2021

@Cellule thanks for that! Unfortunately it doesn't cover allSettled though and I wasn't able to override that:

interface PromiseRejectedResult {
  status: "rejected";
  // @ts-expect-error TS2717 even if overridden, it doesn't work
  reason: unknown;
}

https://www.typescriptlang.org/play?target=7#code/IYZwngdgxgBAZgV2gFwJYHsIwG4FMBOYehEqUuAFAJQwDeAUDEzFJiMjAA77oC2qIXCBgBeGMADuwVBwAKPfoIB0wADaqAyrmTJVuACYUA2gEYAulUbM46fDAqsI7LgoG4Y6OC75uQNBszMqF4U3D7K7MDICMIicTAARPi4AFa4UMgGCf5WgUxhirhKyaCYucwAvuVVFUA

Screen Shot 9

@emartinfigma
Copy link

You can easily achieve this by overloading the signature of Promise.catch in your project

I tried this, but it seems like TS just selects the 'any' overload if the signatures don't match. Playground

@thw0rted
Copy link

thw0rted commented Apr 6, 2022

I'd really like to see this implemented, and I think it's a shame it wasn't considered for release alongside useUnknownInCatchVariables. Before the flag existed, you could always freely convert between

  • p.then(good, bad)
  • p.then(good).catch(bad), and
  • try { result = await p; good(result) } catch (err) { bad(err); } (in async context)

Now, if bad doesn't expect unknown it will fail, but only in the last of these forms. I want a flag that says "use unknown type for all errors", including try/catch and Promise rejection -- which are the same thing, under async/await!

@andrewbranch andrewbranch added this to Next Meeting in Suggestion Backlog Triage Apr 6, 2022
@ExE-Boss
Copy link
Contributor

@thw0rted

  • p.then(good, bad)
  • p.then(good).catch(bad)

Are not the same, since if good throws an error, only the latter form will catch it.


p => p.then(good, bad) is actually equivalent to:

async p => {
	let result;
	try {
		result = await p;
	} catch (err) {
		return bad(err);
	}

	return good(result);
}

@thw0rted
Copy link

That's correct, but I didn't mean that those forms were identical, I was only referring to the parameter type of the bad function. The point is that the then-callback gets any while the catch-block invocation gets unknown.

@NotWoods
Copy link
Member

Could the approach used to fix iterator types in #58243 be used to fix promise.catch as well?

Similiar to the BuiltinIteratorReturn type, perhaps a PromiseCatchErrorType can be used that would be either any or unknown depending on a compiler flag.

interface Promise<T> {
    catch<TResult = never>(onrejected?: ((reason: PromiseCatchErrorType) => TResult | PromiseLike<TResult>) | undefined | null): Promise<T | TResult>;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
Development

No branches or pull requests

8 participants