-
Notifications
You must be signed in to change notification settings - Fork 728
Update docs with reactStrictMode in render
#1479
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
Conversation
✅ Deploy Preview for testing-library ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link was wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this redundant? Also don't forget to update the anchor if you accept the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming it only reactStrictMode would conflict with reactStrictMode from configure because they would share the same hash. If you prefer another name, let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nested below "render Options" already so there shouldn't be a conflict. Or do you mean the anchors conflict? Can we decouple the anchor ID from the heading name?
This just seems noisy at the moment:
- render Options
- container
- baseElement
- [...]
- render Options reactStrictMode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you mean the anchors conflict?
Exactly.
Can we decouple the anchor ID from the heading name?
It seems auto-generated. How would you suggest we do that?
This just seems noisy at the moment:
Yeah, I agree, but I was following the same pattern as renderHook Options wrapper.
Happy to change, but I'm not sure if there's an easy solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's keep it for now. Could even argue the repeated context is good on nested list with lots of siblings since it reduces cognitive overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also cross reference this option from configure#reactStrictMode to help discoverability.
I'd also cross reference this option from
configure#reactStrictModeto help discoverability.
Done. Let me know if wording is fine or if there's any suggestion for improvement.
Update docs with
reactStrictModeinrender, introduced in testing-library/react-testing-library#1390.