-
Notifications
You must be signed in to change notification settings - Fork 45.6k
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
ReactTestInstance to expose toJSON() method for focused snapshots #25329
base: main
Are you sure you want to change the base?
ReactTestInstance to expose toJSON() method for focused snapshots #25329
Conversation
Hi @mdjastrzebski! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Comparing: ea04a48...6deb332 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Pinging so this PR doesn't get closed. I'm looking forward for some feedback on it. The change is small, it's tested, and would be really useful for Test Renderer users, as indicated in #14539 :-) |
A test that mimics how you would use, would help. Looks like none of the existing tests really showcase a need for this feature since it's just used on the root. |
On the web, we've moved away from recommending the test renderer, and instead recommended to query the native tree. I wonder if instead of expanding the API surface of the Test Renderer, whether we've considered to do something similar on RN. I don't mean literally running Android/iOS, but more like whether we can expose something on the RN tree level and use that as the way forward. We've been adding some test selector APIs (#18607) which aren't used anywhere yet but I think they should also (?) work on RN. |
95c52c5
to
43f3333
Compare
@eps1lon I've added tests for more complex tree structure with mixed host & composite elements, re-using existing arrangement from traversal tests. I used @gaearon sounds like an interesting idea to consider, yet quite complex one, as its appears to work on lower level of abstraction than Test Renderer. One aspect that bothers me though is the "experimental" status of these queries. Will research more on that, and discuss with other RNTL maintainers. Meanwhile, I would argue that merging this PR would still benefit the existing RN projects using RNTL/Test Renderer with immediate benefit of focused snapshot testing. Using Test Renderer is currently the main solution for writing component & TL-style integration tests in RN world, with a lot of projects being heavily invested in creating their automated tests. While the cost is relatively minor addition to Test Renderer package. |
It's implemented. The only reason they're called experimental is because we never deployed or exposed them anywhere so they're just... kind of there. |
@gaearon is there any recommendation of moving from react-test-renderer to test selector API? Some plans to deprecate it maybe? If you could confirm whether this approach works with RN or not, that would be great 👍🏼 |
@eps1lon I've added more tests for |
It's me again, I wanted to check with the React team, if this PR has chances to move forward. To give you some more context, React Test Renderer is the primary solution used for testing in the React Native ecosystem, so while it might seem a bit odd to use in on the web, there are a lot of RN apps that use it directly or through React Native Testing Library, which I maintain, to provide solid integration & component test coverage. This PR would really bring value to RN space. I believe that having |
Curious if there has been any movement on this. I'm a React Native developer, and these changes would be a huge benefit to my testing workflow if this were merged and React Native Testing Library were able to take advantage of this. Thanks all! |
would also like to see this implemented |
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
bump |
Summary
Allow calling
toJSON
on anyReactTestInstance
in order to be able to make focused snapshots when writing tests using React Test Renderer or React Native Testing Library.Currently Test Renderer only exposes parameterless
toJSON()
as a result toReactTestRenderer.create()
function call but that function can only output whole component tree, which frequently result in very long snapshots that are hard to work with.Additionally, we plan to use this new function to improve implementation of various internal elements from React Native Testing Library, like
getByText
query algorithm, focuseddebug()
function, etc.This PR adds
toJSON()
method toReactTestInstance
class, so that users can generate JSON tree for any node. Implementation works by calling internaltoJSON
method if given node is host component, otherwise it recursively callstoJSON()
on its children and returns collected JSON trees as array.Resolves #14539
How did you test this change?
I've added additional assertions to all relevant
ReactTestRenderer-test.internal.js
tests. The new assertions verify thatrenderer.root.toJSON()
output is the same asrenderer.toJSON()
output. These tests cover different component tree structures and lifecycle changes.