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

Make isBoom type definition laxer #275

Merged
merged 1 commit into from
Nov 30, 2020
Merged

Make isBoom type definition laxer #275

merged 1 commit into from
Nov 30, 2020

Conversation

nulltoken
Copy link
Contributor

@nulltoken nulltoken commented Nov 27, 2020

Especially useful in hapijs context, where request.response can be a
Boom object. This change allows the TypeScript code to leverage isBoom()
to check if request.response is a Boom object or not, without any noisy typecast.

@nulltoken
Copy link
Contributor Author

@cjihrig, @devinivy, @Nargonath, @lloydbenson, @nlf, @geek 👋 kind bump?

Copy link
Member

@Nargonath Nargonath left a comment

Choose a reason for hiding this comment

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

I'm not a regular TS user but LGTM, let's see what other have to say about it.

Copy link
Contributor

@kanongil kanongil 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 like a very sensible (and backwards compatible) change.

FYI, for TS, you could just use a obj instanceof Boom check instead.

package.json Outdated Show resolved Hide resolved
test/index.ts Show resolved Hide resolved
Especially useful in hapijs context, where request.response can be a
Boom object. This change allows the TypeScript code to leverage isBoom()
to check if request.response is a Boom object or not, without any noisy typecast.
@kanongil kanongil added the types TypeScript type definitions label Nov 30, 2020
Copy link
Member

@nlf nlf left a comment

Choose a reason for hiding this comment

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

agreed, this looks totally reasonable and how i would expect things to function 👍

@cjihrig cjihrig added this to the 9.1.1 milestone Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types TypeScript type definitions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants