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

Preview: Introduce reactNativeComponent #616

Closed
wants to merge 1 commit into from

Conversation

sahrens
Copy link
Contributor

@sahrens sahrens commented Apr 2, 2015

Summary:

Creating a new native component is a pain - you have to write native
code, manually export the properties you want to use in JS, then copy those
props in JS in validAttributes and make sure you specify differs for
non-scalar props, otherwise changes won't be sent to native.

This diff adds native code to automatically export nativePropTypes, and
consumes those in a new helper requireNativeComponent. Right now the actual
types aren't used in JS, but I changed the logic to default to using
deepDiffer for objects without an explicit differ specified (this should be
much safer in general - we've had a few bugs related to bad diff configuration).
The only disadvantage to always using deepDiffer for Objects is perf, so
requireNativeComponent has an option to provide optimized differ functions,
which is demonstrated in ScrollView.js.

The native export is done similarly to the way we figure out property setters,
but with class methods (prefixed with getViewPropDef_) that simply return
dictionary literals with the type info.

This also sets the stage for potentially doing JS-side type checking, which
would give us better errors with more context and useful stack traces.

Finally, this gives us the ability to warn or throw when requiring unsupported
native views.

cc @ide - this is a preview of an internal diff that's under review.

Test Plan:

SliderIOS still works, sticky headers stick properly (they break if
diffing breaks), other apps/interactions seems fine.

Summary:

Creating a new native component is a pain - you have to write native
code, manually export the properties you want to use in JS, then copy those
props in JS in `validAttributes` and make sure you specify differs for
non-scalar props, otherwise changes won't be sent to native.

This diff adds native code to automatically export `nativePropTypes`, and
consumes those in a new helper `requireNativeComponent`.  Right now the actual
types aren't used in JS, but I changed the logic to default to using
`deepDiffer` for objects without an explicit differ specified (this should be
much safer in general - we've had a few bugs related to bad diff configuration).
The only disadvantage to always using `deepDiffer` for `Objects` is perf, so
`requireNativeComponent` has an option to provide optimized differ functions,
which is demonstrated in `ScrollView.js`.

The native export is done similarly to the way we figure out property setters,
but with class methods (prefixed with `getViewPropDef_`) that simply return
dictionary literals with the type info.

This also sets the stage for potentially doing JS-side type checking, which
would give us better errors with more context and useful stack traces.

Finally, this gives us the ability to warn or throw when requiring unsupported
native views.

cc @ide

Test Plan:

SliderIOS still works, sticky headers stick properly (they break if
diffing breaks), other apps/interactions seems fine.
@ide
Copy link
Contributor

ide commented Apr 2, 2015

This looks great and I think it makes the right tradeoffs. You might want to prefix the codegen'd methods with rct_ which is Apple's suggested convention for private methods, although in practice I don't see name collisions happening.

A longer-term thing to think about is Swift support... since Swift can use the Objective-C runtime API it should be possible to dynamically add the getViewPropDef_* methods but there isn't a way to do it with a macro. Just something to keep in mind, but it feels like there's a path forward to accommodate Swift one day.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2015
@sophiebits
Copy link
Contributor

This happened, I think.

@sophiebits sophiebits closed this Apr 24, 2015
jfrolich pushed a commit to jfrolich/react-native that referenced this pull request Apr 22, 2020
ayushjainrksh referenced this pull request in MLH-Fellowship/react-native Jul 2, 2020
Correcting grammar for clarity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants