-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Tests: Use container
instead of container.firstChild
for snapshots
#45278
Conversation
Size Change: 0 B Total Size: 1.28 MB ℹ️ View Unchanged
|
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.
The changes look good to me 👍
P.S. Any reason why we're not allowing allowContainerFirstChild
but marking other node traversing as an error?
Great question! I gave this a bit of thought before filing the PR - basically, because in the rule docs we can specifically read:
And I believe that it's actually pretty easy to avoid - having the wrapper as part of the snapshot doesn't hurt at all. |
What?
This PR updates snapshot tests that use RTL's
container.firstChild
to usecontainer
instead. This changes the snapshots by adding the wrapper as well.Why?
Because it allows us to get rid of a bunch of
no-node-access
ESLint violations for a very cheap price.Also, it's better according to Kent C. Dodds, if we don't compare snapshots before and after, see https://twitter.com/kentcdodds/status/1272186092779761669
How?
We're just replacing
container.firstChild
withcontainer
and updating the tests.Testing Instructions
Verify all tests still pass.