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

Make DOM wrapper component using lower-level primitives #3976

Merged
merged 1 commit into from
Jun 2, 2015

Conversation

sophiebits
Copy link
Collaborator

Introducing: a really lame version of composite components, right inside of ReactDOMComponent!

Now ReactDOMInput isn't an actual component. This brings us closer to exposing DOM nodes as refs.

@sophiebits
Copy link
Collaborator Author

@sebmarkbage Is this what you were thinking?

value: value != null ? value : this._wrapperState.initialValue,
checked: checked != null ? checked : this._wrapperState.initialChecked,
onChange: this._wrapperState.onChange
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This copying should be avoidable now that we don't have to adhere to a common pretty API but fine as a first step.

@sophiebits sophiebits force-pushed the no-wrapper branch 2 times, most recently from f802daa to 503422b Compare June 2, 2015 00:13
@sophiebits
Copy link
Collaborator Author

@sebmarkbage r? If this is good then I'll convert the others too (in a separate PR).

@sebmarkbage
Copy link
Collaborator

thx

Introducing: a really lame version of composite components, right inside of ReactDOMComponent!

Now ReactDOMInput isn't an actual component. This brings us closer to exposing DOM nodes as refs.
@sophiebits
Copy link
Collaborator Author

Travis is erroring (due to nvm problems?) but unit tests and lint pass locally for me.

sophiebits added a commit that referenced this pull request Jun 2, 2015
Make DOM wrapper component using lower-level primitives
@sophiebits sophiebits merged commit 8c2a499 into facebook:master Jun 2, 2015
@sebmarkbage
Copy link
Collaborator

I'm so excited about this diff and the future ones like it!

@mridgway
Copy link
Contributor

mridgway commented Jun 3, 2015

Just noticed this PR after working on ES3 native components as we discussed in #2895 (comment). Will you be converting all native components to this pattern?

@sebmarkbage
Copy link
Collaborator

Yes

@sophiebits
Copy link
Collaborator Author

Ah, yes. Sorry for the mixup, but it sounds like this will solve your problem too.

@mridgway
Copy link
Contributor

mridgway commented Jun 3, 2015

Ok, great.

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.

5 participants