-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Set Error.prototype.cause type to unknown (#70) #49639
Conversation
Signed-off-by: Abdul-Azeez Lawal <abdul-azeez.lawal@outlook.com>
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good change. Would fix voxpelli/pony-cause#35
@iamharbie @mkubilayk @dragomirtitian What's needed to get the CLA signed? My guess is that Bloomberg already has a signed CLA, just that @iamharbie hasn't been added to it and thus that the bot doesn't pick it up? |
Oh, apparently the Bloomberg CLA thing is being worked on by @RyanCavanaugh: #49675 (comment) |
…error` on TS `4.8` by @voxpelli This commit fixes the following error, which appears when running TS `^4.8.0-dev.20220707` with eg. ` npx tsc --lib es2020,es2022.error,es2021.promise` or ` npx tsc --lib esnext` > error TS2425: Class 'Error' defines instance member property 'cause', but extended class 'VError' defines it as instance member function. Discovered in voxpelli/pony-cause#41 when looking to extend the tests for voxpelli/pony-cause#35 to detect whether the nightly of `4.8.0` now correctly fixes that, following the merge of microsoft/TypeScript#49639
Describe your changes
This PR fixes #45167 which changes the type of
Error.prototype.cause
fromError
toUnknown
Testing performed
I added new tests to ensure that unknown type is recognised
Additional context
Could also potentially resolve #48098. @DanielRosenwasser's suggested being more restrictive with the type of the
cause
property of the options param. I am open to discussions on this.Fixes #45167
Credits to Bloomberg colleagues who helped with reviews @mkubilayk @dragomirtitian