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 type definition on useResizeObserver #326

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

genepaul
Copy link
Contributor

Description

When using useResizeObserver, I discovered that the definition of the return type of this hook was misleading. DOMRect is initialized to undefined, and upon the first run of this hook, the return value can be undefined. In fact, the docs all show that you should check for the existence of DOMRect before doing anything with the return value. That should be reflected in the types.

Related Issue

Fixes #325

Motivation and Context

TypeScript should be used to help consumers understand the potential values a variable could be. The return type of this function being inaccurate can cause consumers to inaccurately handle it and expect it to always be defined.

How Has This Been Tested?

Linked locally into my project using this function and verified that it correctly reported that it was potentially undefined.

Screenshots (if appropriate):

When using `useResizeObserver`, I discovered that the definition of the return type of this hook was misleading. `DOMRect` is initialized to `undefined`, and upon the first run of this hook, the return value can be undefined. In fact, the docs all show that you should check for the existence of `DOMRect` before doing anything with the return value. That should be reflected in the types.
@antonioru antonioru merged commit c8e200f into antonioru:master Jan 27, 2022
@genepaul
Copy link
Contributor Author

I apologize, it looks like I didn't read the contribution guidelines closely enough. I forgot to update the version in package.json and changelog. Should I open a new PR to do that?

@genepaul genepaul deleted the patch-1 branch January 27, 2022 21:25
@antonioru
Copy link
Owner

@genepaul don't worry, it's not a problem... and if anything it's my bad :))
I'll update the version and open an issue to remind me to automate this kind of tasks

@antonioru
Copy link
Owner

@genepaul version 1.0.4 containing your contribution has been released!

Thank you very much for improving the project.
Feel free to reach me out if you spot anything else that can or should be fixed and are willing to contribute again

@genepaul
Copy link
Contributor Author

@antonioru - I'm sorry to keep bothering you but it looks like the build wasn't run so my change isn't in 1.0.4 😦

@antonioru
Copy link
Owner

@genepaul not bothering at all mate!

Are you sure about that?
Cause I can definitely see your code in version 1.04, check the screenshot attached:
Screenshot 2022-01-27 at 22 39 32

@antonioru
Copy link
Owner

antonioru commented Jan 27, 2022

I've created a brand new react project from scratch using create-react-app and installed beautiful-react-hooks... your code is there, innit?
Or am I missing something? 😥😥😥

@genepaul
Copy link
Contributor Author

You're right, my bad. My IDE wasn't updating with the TS types, but I see it. Thanks!

@antonioru
Copy link
Owner

@genepaul you're welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useResizeObserver reports incorrect return type
2 participants