Skip to content
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

Clean up ReactTestRenderer #7716

Merged
merged 4 commits into from
Sep 14, 2016
Merged

Conversation

aweary
Copy link
Contributor

@aweary aweary commented Sep 13, 2016

This breaks out ReactTestTextComponent and ReactTestEmptyComponent into their own files so the main ReactTestRenderer.js file is a little cleaner.

I also implemented ReactTestRenderer with an ES6 class + flow types.

cc @gaearon @spicyj

@@ -7,6 +7,7 @@
* of patent rights can be found in the PATENTS file in the same directory.
*
* @providesModule ReactTestRenderer
* @flow
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaearon should I add flow as a separate PR? I know you mentioned keeping style changes separate from feature changes, but I wasn't sure where flow landed on that spectrum.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flow is fine now that we have a better idea how to use it thanks to PRs from @vjeux!

@ghost ghost added the CLA Signed label Sep 13, 2016
mountComponent(
transaction: ReactTestReconcileTransaction,
nativeParent: null | ReactTestComponent,
nativeContainerInfo: ?null,
Copy link
Contributor Author

@aweary aweary Sep 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't too sure if I typed nativeParent and nativeContainerInfo correctly. From what I could tell, nativeParent is either null or another instance of ReactTestComponent, and nativeContainerInfo was either null or undefined.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nativeContainerInfo was either null or undefined

haha, sounds about right.

@sophiebits
Copy link
Collaborator

Seems all right, thanks.

@ghost ghost added the CLA Signed label Sep 13, 2016
@aweary aweary merged commit 5a3abab into facebook:master Sep 14, 2016
@aweary aweary added this to the 15-next milestone Sep 14, 2016
@zpao zpao modified the milestones: 15-next, 15.4.0 Oct 4, 2016
zpao pushed a commit that referenced this pull request Oct 4, 2016
* create ReactTestTextComponent fil

* create ReactTestEmptyComponent

* Use class for ReactTestRenderer

* Add flow to ReactTestRenderer

(cherry picked from commit 5a3abab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants