Skip to content

feat: add useStateRef hook #1113

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

Conversation

michaltarasiuk
Copy link
Contributor

@michaltarasiuk michaltarasiuk commented Jan 28, 2023

Hook description

One rudimentary way to measure the position or size of a DOM node is to use a callback ref. React will call that callback whenever the ref gets attached to a different node.

Note that we pass [] as a dependency array to useCallback. This ensures that our ref callback doesn’t change between the re-renders, and so React won’t call it unnecessarily, so in current example the callback ref will be called only when the component mounts and unmount. As in result we will not fall into infinite re-renders.

Checklist

  • Have you read the contribution guidelines?
  • If you are porting a hook from react-use, have you checked Port remaining hooks from react-use #33 and the migration guide
    to confirm that the hook has been approved for porting?
  • Does the code have comments in hard-to-understand areas?
  • Is there an existing issue for this PR?
    • link issue here
  • Have the files been linted and formatted?
  • Have the docs been updated?
  • Have you written tests for the new hook?
  • Have you run the tests locally to confirm they pass?

@codecov
Copy link

codecov bot commented Jan 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.53%. Comparing base (f6829c0) to head (f833f4a).
Report is 349 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1113      +/-   ##
==========================================
+ Coverage   98.52%   98.53%   +0.01%     
==========================================
  Files          63       64       +1     
  Lines        1082     1091       +9     
  Branches      180      180              
==========================================
+ Hits         1066     1075       +9     
  Misses          2        2              
  Partials       14       14              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xobotyi
Copy link
Contributor

xobotyi commented Jan 28, 2023

So, you've

  • added new dependency (not compatible with our testing pipeline
  • globally disabled linting rule without any reasoning
  • once again ignored PR guidelines

Last one is somehow "forgivable"

But first two are no-go since they should be separate pull requests with reasoning

@michaltarasiuk
Copy link
Contributor Author

Hi @xobotyi, in days I will create a pull request for two first points.

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

Successfully merging this pull request may close these issues.

2 participants