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

Fix: jsonErrorReplacer to use explicit Error type #125

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

h3ssan
Copy link
Contributor

@h3ssan h3ssan commented Aug 24, 2024

Description

This PR refactors the jsonErrorReplacer function to use explicit Error casting, improving type safety and preventing potential errors.

Changes

In the src/handler.ts file, the jsonErrorReplacer function contains an argument val with a type of any. However, the function also includes a type check val instanceof Error, which allows for a safe cast to the Error type. To leverage this type information, we can introduce a type cast using the as keyword, assigning the casted value to a new variable error. This refinement enables the TypeScript compiler to accurately infer the type of error and eliminates any potential type errors.

Copy link
Member

@enisdenjo enisdenjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Am wondering why TS didnt infer this type cast due to the narrowing. 🤔

@enisdenjo enisdenjo merged commit c51cc5d into graphql:main Aug 26, 2024
22 of 27 checks passed
@h3ssan
Copy link
Contributor Author

h3ssan commented Aug 26, 2024

This looks good! Am wondering why TS didnt infer this type cast due to the narrowing. 🤔

Thank you! Yes, sometimes you can't depend on a machine, lol

@h3ssan h3ssan deleted the fix/json-error-replacer-type-safety branch August 26, 2024 17:22
@benjie
Copy link
Member

benjie commented Sep 17, 2024

Maybe if val was unknown rather than any it would work?

@enisdenjo
Copy link
Member

🎉 This PR is included in version 1.22.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@enisdenjo enisdenjo added the released Has been released and published label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Has been released and published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants