-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Basic shallow rendering support #2497
Conversation
assign( | ||
ShallowCompositeComponentWrapper.prototype, | ||
ReactCompositeComponent.ShallowMixin | ||
); |
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.
You're not really using any logic in this file. It's mostly just validation logic and figuring out what to do about native components. Since you won't test native components using this strategy, you won't shallow render native components.
Once you remove all the stuff you're not using, this will simply just be constructing a new instance of the ShallowMixin. You can probably just move all that stuff into the ReactTestUtils so we don't have to deal with this intermediate.
Good point about not needing |
Had a little more time to work on this and got updating working. Also managed to simplify the code somewhat. I went back to using transactions since it was adding more duplication/complexity not to and they don't seem to hurt anything. Interested in any thoughts, @sebmarkbage. |
This looks good but this is probably going to conflict with some of the other PRs. So let's wait to rebase on top of them so we don't have to thrash them so much. |
That sounds fine. Any PRs in particular to look out for? |
I think they're all in now so please rebase and land asap to avoid more conflicts. |
Now handles updating. Haven't looked at refs yet.
Basic shallow rendering support (#2393)
WIP to fix #2393.
Now redone on top of the new ReactCompositeComponent/ReactClass separation (#2445). Still only handles initial rendering not updating, but this is a simpler implementation that should be easier to add the missing parts to.
Sorry about closing the old PR (#2459), I'll just force-push next time I do a new version of a PR.
As a bonus, per @sebmarkbage's recommendation, this defines the validated flag to be a non-enumerable property, and avoids setting owner on the shallowly-rendered component, so we can now do a straight equality test on the expected and actual results and avoid helper functions like
areElementsEquivalent
.