Skip to content

Promise.catch should allow to widen promise type #57

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
danielrentz opened this issue Jan 10, 2025 · 8 comments · Fixed by #60
Closed

Promise.catch should allow to widen promise type #57

danielrentz opened this issue Jan 10, 2025 · 8 comments · Fixed by #60
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@danielrentz
Copy link
Contributor

Promise.catch can be used to silence rejections and to make the result optional by converting rejections to undefined (or more generally, to add any other result type to the promise chain). This is not possible anymore with better-ts-lib:

// before (TS lib)
const promise1 = Promise.resolve("a").then(() => 42, () => undefined);  // Promise<number | undefined>
const promise2 = Promise.resolve(42).catch(() => undefined);            // Promise<number | undefined>

// after (better-ts-lib)
const promise1 = Promise.resolve("a").then(() => 42, () => undefined);  // Promise<number | undefined>
const promise2 = Promise.resolve(42).catch(() => undefined);            // <=== type error

Could you add a catch<U> similar to the existing then<U>?

@danielrentz
Copy link
Contributor Author

Ok just found #32 that discusses exactly this problem. Feel free to close if this is still the way to go here. Although I think it is inconsistent compared with the "lax" definition of "Promise.then".

@danielrentz
Copy link
Contributor Author

BTW, in #32 there one can read "That being said, I wouldn't mind making the callback function of .catch mandatory." which did not happen as it seems.

@uhyo uhyo added the enhancement New feature or request label Jan 13, 2025
@uhyo
Copy link
Owner

uhyo commented Jan 13, 2025

I kind of think that making the catch definition lax is fine. There are other ways to prevent unexpectedly widened Promise value types if you want.

@uhyo uhyo added the help wanted Extra attention is needed label Jan 13, 2025
@danielrentz
Copy link
Contributor Author

@uhyo I would like to create a PR. Should I make the callback parameter required (as mentioned in #32)?

@uhyo
Copy link
Owner

uhyo commented Jan 13, 2025

Yes. catch() without callback isn't useful and can be prohibited I think.

@danielrentz
Copy link
Contributor Author

@uhyo Somehow I cannot build the project, calling npm run build:lib always ends with Error: ENOENT: no such file or directory, open '.../better-typescript-lib/TypeScript/src/lib/libs.json' (tried Linux and Windows). Is there something special to do before (despite the usual npm i)? Seems I have never worked with projects using a subproject.

@uhyo
Copy link
Owner

uhyo commented Jan 14, 2025

Thank you for working on this!

I think you need to run:

git submodule init

and:

git submodule update

@danielrentz
Copy link
Contributor Author

danielrentz commented Jan 14, 2025

Thanks I will try and if I succeed, will add this to CONTRIBUTING.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants