Clarify useRef hook usages #412
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is my suggestion follow a [comment](typescript-cheatsheets#388 (comment)) in typescript-cheatsheets#388 . The problem with the current guide is that most newcomers will use the `null!` approach, which could cause hidden bug in practice, when the developer forget to assign the ref or conditionally render the ref-ed element. I saw this a lot, because this guide is the official reading recommendation in my company, and I keep seeing that bug. My proposal is to not bypassing TypeScript, but let TypeScript does its job, by correctly tell it whether we need a RefObject and MutableRefObject. This also follows React's official guides, where there are 2 corresponding use cases (DOM access and mutable variable)
|
Hi @sw-yx ! Just wanted to say thank you for your commits. Your updates are so good. You're great at documenting stuffs. |
|
hahah too kind. thank you for contributing and hope you feel welcome to add more! this is a community effort. |
stepanenko
added a commit
to stepanenko/react-ts-best-practices
that referenced
this pull request
May 9, 2021
* Updated README on 2021-04-02T14:21:23.928Z * Fix missing parenthesis in troubleshooting types code example (typescript-cheatsheets#399) Co-authored-by: swyx <[email protected]> * format * minor improvements to "adding non-standard attributes" (typescript-cheatsheets#400) * Added event types and their descriptions (typescript-cheatsheets#402) * Added form types and their descriptions I was looking for a complete list of form types and couldn't find any anywhere. I went through the source code and extracted these with adding some description to them, either using MDN's description or some other website explanation. * Apply suggestions from code review Co-authored-by: swyx <[email protected]> * fix: Remove incorrect usage of "widening" (typescript-cheatsheets#405) Using an intersection type here is actually narrowing the type, but the intent here seems to be to introduce type assertions, not talk about the difference between wider and narrower types. * add type assertion for useState thanks @priscilaandreani closes typescript-cheatsheets#403 * format * Fixed typo (typescript-cheatsheets#406) Fixed an issue where a paragraph described three different options as two. * Add rollpkg as an option for creating libraries (typescript-cheatsheets#407) * useLocalStorage Type detection bug (typescript-cheatsheets#409) in my vs code it detects that the type of soredValue is (T | (value: T | ((val: T) => T)) => void) but it must be just (T) so it throws error with this change it solved. * Updated README on 2021-04-25T20:13:23.562Z * Clarify useRef hook usages (typescript-cheatsheets#412) Co-authored-by: swyx <[email protected]> * Update index.md * Update from-flow.md * docs: throw when ref.current is null (typescript-cheatsheets#413) * Update types.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Denis LE <[email protected]> Co-authored-by: swyx <[email protected]> Co-authored-by: Sebastian Silbermann <[email protected]> Co-authored-by: Lars <[email protected]> Co-authored-by: Gerrit Birkeland <[email protected]> Co-authored-by: Christopher Kumm <[email protected]> Co-authored-by: Rafael Pedicini <[email protected]> Co-authored-by: Mandy YP <[email protected]> Co-authored-by: Thien Do <[email protected]> Co-authored-by: Christina Grannas <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is my suggestion follow a comment in #388 .
The problem with the current guide is that most newcomers will use the
null!approach, which could cause hidden bug in practice, when the developer forget to assign the ref or conditionally render the ref-ed element. I saw this a lot, because this guide is the official reading recommendation in my company, and I keep seeing that bug 😅My proposal is to not bypassing TypeScript, but let TypeScript does its job, by correctly tell it whether we need a RefObject and MutableRefObject. This also follows React's official guides, where there are 2 corresponding use cases (DOM access and mutable variable)
Thanks for contributing!
If you are contributing to README.md, DON'T. README.md is auto-generated. Please just make edits to the source inside of
/docs/basic. Sorry, we get that it is a slight pain.