[Public Interface] Create whitelist internal modules that need to be exposed publicly #1821

brentvatne · 2015-06-30T23:24:17Z

As pointed out in #1808, on 0.7.0-rc+ requiring modules that are internal to React Native (eg: require('NativeModules') rather than var { NativeModules } = require('react-native')) is not supported going forward. The new behavior is correct, so we need to help people transition away from the old way. A good example of using this incorrect behavior to access internals can be seen on my react-native-video project.

The purpose of this issue is to ensure that we publicly expose every module that is necessary to build libraries and apps - to do this we simply have to make a whitelist of those modules and add them to react-native.js)

Please add to this issue with internal modules that you use on your projects that are not already listed here. Thanks!

cc @vjeux @amasad

The text was updated successfully, but these errors were encountered:

ide · 2015-06-30T23:43:08Z

Dimensions is a big one. Many of the prop types are either exported or you can write View.propTypes, Text.propTypes, etc.

orktes · 2015-07-01T06:52:43Z

precomputeStyle is one that I think people (me included) use frequently

brentvatne · 2015-07-04T00:37:50Z

For your consideration, @ide @ccheever @orktes:

Likely candidates for inclusion

ReactNativeART (ART)
~~Dimensions~~
~~Platform~~
flattenStyle (StyleSheet.flatten)
~~ViewStylePropTypes~~
~~TextStylePropTypes~~
~~LayoutPropTypes~~
~~StyleSheetPropType~~
~~ReactNativeViewAttributes~~
NativeMethodsMixin (look into this one more)
~~deepDiffer (and other differs)~~
~~precomputeStyle~~
~~NavigatorNavigationBarStyles~~
~~StyleSheetRegistry~~
~~StyleSheetValidation~~
TextInputState (TextInput.State)

Not sure what we should do with these

~~ReactInstanceMap (via react-tools) (look into this one further)~~
Touchable (via react-contrib, used in several projects)
~~TimerMixin (via react-timer-mixin)~~
~~onlyChild (via react-tools, already available via require('react-native').Children.only)~~
~~invariant (via react-tools)~~
~~rebound~~
~~StaticContainer (via react-contrib)~~
~~logError~~
~~warning (exposed globally, probably not necessary but require'd here: https://github.com/rt2zz/react-native-addressbook/blob/master/index.js)~~
~~merge~~
~~keyMirror~~

Documentation

Dimensions
Go through others to make sure that public APIs have this

(Mostly) complete list of repos that will be affected

Notes

ReactPropTypes is often required, can just be used via React.PropTypes
Tons of people just do require('NativeModules') (probably my fault:
http://brentvatne.ca/facebook-login-with-react-native/)
ReactChildren is sometimes directly required, could just do React.Children

ide · 2015-07-04T00:58:43Z

Very comprehensive @brentvatne. Looking over your candidates for inclusion here are some of my thoughts:

Dimensions - already exported on Thursday I believe
Platform - already exported
flattenStyle - not sure about this one. exposing the Registry opens up a lot of doors instead.
ViewStylePropTypes - not sure at the moment. leaning towards holding back for now since PropTypes aren't required for a component
TextStylePropTypes
LayoutPropTypes - leaning towards yes
StyleSheetPropType
ReactNativeViewAttributes - is this necessary anymore?
NativeMethodsMixin - not sure it's good to expose such an implementation detail right now
deepDiffer (and other differs) - probably doesn't make sense to export this unless there is some use case where a module needs to use the exact same algorithm as RN
precomputeStyle - probably, but maybe namespaced under StyleSheet
NavigatorNavigationBarStyles - this is such "leaf node" code that is subject to change rapidly. people should copy & paste
StyleSheetRegistry - yes, perhaps StyleSheet.Registry
StyleSheetValidation - maybe

- Future-proofs for later RN versions (via: facebook/react-native#1821)

dsibiski · 2015-07-06T21:54:16Z

@brentvatne Thanks again for pointing this out. 👍

mattmo · 2015-07-07T00:20:53Z

Yes, this is a great list! This is a minor issue but it seems that cssVar should be removed from this example, given it's no longer exported (and probably shouldn't be).

react-native/Examples/UIExplorer/Navigator/NavigationBarSample.js

Line 29 in e81e29f

var cssVar = require('cssVar');

I'm going to file a bug for it to be removed from react-native-navbar here:

https://github.com/Kureev/react-native-navbar/blob/master/index.js#L15

In general, are there any tests in place that ensure the files in the example directory perform properly? I noticed the travis config but didn't dive into the details for how it's used.

brentvatne · 2015-07-07T01:41:33Z

Great point @mattmo - it seems like Libraries/Utilities/CSSVarConfig.js and Libraries/Utilities/cssVar.js probably don't belong in OSS, what do you think @ide?

Good spot there with react-native-navbar too! 😄

brentvatne · 2015-07-07T01:49:24Z

In general, are there any tests in place that ensure the files in the example directory perform properly? I noticed the travis config but didn't dive into the details for how it's used.

There are, but they don't cover every example. The UIExplorer examples that are specifically tested via snapshots are Layout, Slider, Switch, TabBar, Text, and View. Other functionality (but not the specific screens in UIExplorer) are tested via unit tests and JS integration tests.

brentvatne · 2015-07-07T17:46:26Z

As per #1896 (comment) I think that Subscribable should also be added to the exports

ide · 2015-07-07T21:48:02Z

it seems like Libraries/Utilities/CSSVarConfig.js and Libraries/Utilities/cssVar.js probably don't belong in OSS, what do you think @ide?

Yep they look unused.

brentvatne · 2015-07-09T04:11:07Z

ReactNativeART appears to be missing as well

orktes · 2015-07-09T06:10:47Z

What about TextInputState?

I often use

TextInputState.blurTextInput(
  TextInputState.currentlyFocusedField()
);

to blur input & dismiss keyboard if I don't have ref to what is currently focused. This is handy for example when pushing a new route to Navigator or in the Navigator onWillFocus event.

brentvatne · 2015-07-11T01:32:50Z

That's pretty nifty @orktes, cc @vjeux ^

chirag04 · 2015-07-18T00:39:03Z

I could never understand TextInputState. Thanks for the example @orktes. Seems really useful. @brentvatne I think Input State should be documented. I don't fully understand it to PR.

sahrens · 2015-07-18T11:37:50Z

We should definitely try to minimize the number of top-level things exported off of react-native.

We can hang TextInputState off of TextInput, and similar for other stuff like StyleSheet utilities.

What do people use precomputeStyle for?

Stuff like ViewStylePropTypes shouldn't be necessary because you can usually just use View.propTypes.style.

brentvatne · 2015-07-18T16:33:07Z

@sahrens - if you need to use setNativeProps with transforms, you'll need to use precomputeStyle before passing them in to setNativeProps. We could instead modify setNativeProps to run any transforms through that filter automatically.

Agreed re: View.propTypes.style and hanging TextInputState and such off of their parent components.

ide · 2015-07-18T16:37:23Z

Let's expose StyleSheet.Registry then. Any pitfalls to doing so, keeping future StyleSheet optimizations in mind?

About ViewStylePropTypes -- I imagine people extend it when their components accept more styles than plain Views, similar to what Text does.

vjeux · 2015-07-18T17:08:29Z

@brentvatne that's a bug, setNativeProps does call precomputeStyle: https://github.com/facebook/react-native/blob/master/Libraries/ReactIOS/NativeMethodsMixin.js#L105

sahrens · 2015-07-18T20:31:06Z

What are folks using StyleSheet.Registry for?

On Jul 18, 2015, at 7:08 PM, Christopher Chedeau notifications@github.com wrote:

@brentvatne that's a bug, setNativeProps does call precomputeStyle: https://github.com/facebook/react-native/blob/master/Libraries/ReactIOS/NativeMethodsMixin.js#L105

—
Reply to this email directly or view it on GitHub.

jeanregisser · 2015-07-29T14:29:17Z

Would be great to also expose Easing from react-native/Libraries/Animation/Animated/Easing.js.

skevy · 2015-07-29T15:13:48Z

@sahrens https://github.com/pjjanak/react-native-nested-stylesheets is one example

brentvatne · 2015-08-02T23:08:19Z

I'm going to get a pull request in for an updated list of modules a week from today, please comment on here if a module that you are using has not been discussed above!

brentvatne · 2015-09-25T16:18:41Z

This still seems like something that is important to fix. Sorry for not getting that PR in, other things got in the way. @vjeux - would you like a PR for this still?

vjeux · 2015-09-25T16:28:50Z

Yeah, we need to include it properly. Let's schedule a meeting on monday so we can go over all them in person and figure out a plan.

brentvatne · 2015-10-02T22:38:05Z

@vjeux - sounds good, I have a few meetings scheduled for Monday already but if 10am or 1pm work for you I can call in

brentvatne · 2015-10-09T16:21:25Z

Moving discussion to PR

Summary: - TextInputState as TextInput.State - Touchable - flattenStyle as StyleSheet.flatten - ReactNativeART as ART Original discussion in #1821 Closes #3308 Reviewed By: sebmarkbage Differential Revision: D2527152 Pulled By: javache fb-gh-sync-id: 19d4ef9d4c0e6587b9f0793e1ca624aebb034f3b

Summary: - TextInputState as TextInput.State - Touchable - flattenStyle as StyleSheet.flatten - ReactNativeART as ART Original discussion in facebook#1821 Closes facebook#3308 Reviewed By: sebmarkbage Differential Revision: D2527152 Pulled By: javache fb-gh-sync-id: 19d4ef9d4c0e6587b9f0793e1ca624aebb034f3b

TuangWang · 2016-05-25T02:45:44Z

after i update the react-native. I got this error, "undefined is not an object (evaluating 'React.PropTypes.bool')".

brentvatne self-assigned this Jun 30, 2015

brentvatne changed the title ~~[Packager] Prevent requiring internals of React~~ [Public Interface] Create whitelist internal modules that need to be exposed publicly Jun 30, 2015

brentvatne mentioned this issue Jul 1, 2015

Access requireNativeComponent through public module interface nitaliano/react-native-mapbox-gl#54

Merged

ide mentioned this issue Jul 3, 2015

0.8.0-rc #1864

Closed

4 tasks

dsibiski added a commit to dsibiski/react-native-userdefaults-ios that referenced this issue Jul 6, 2015

Require 'NativeModules' from the public interface

e598c84

- Future-proofs for later RN versions (via: facebook/react-native#1821)

mattmo mentioned this issue Jul 7, 2015

cssVar not exported in react-native 0.7 Kureev/react-native-navbar#48

Closed

JonathanWi mentioned this issue Sep 15, 2015

[Bug] require('NativeModules').SpeechSynthesizer is undefined naoufal/react-native-speech#1

Closed

brentvatne mentioned this issue Sep 25, 2015

DrawerLayoutAndroid:- undefined is not an object (evaluating DrawerLayoutAndroid.positions ) #3031

Closed

oblador mentioned this issue Sep 29, 2015

Use correct react native packager require paths oblador/react-native-vector-icons#26

Closed

brentvatne mentioned this issue Oct 9, 2015

Expose additional private modules #3308

Closed

brentvatne closed this as completed Oct 9, 2015

brianinator mentioned this issue Apr 27, 2016

I installed the module and when I try to use it I get "Unable to resolve module NativeModules" eduedix/react-native-networking#11

Closed

facebook locked as resolved and limited conversation to collaborators Jul 22, 2018

react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Public Interface] Create whitelist internal modules that need to be exposed publicly #1821

[Public Interface] Create whitelist internal modules that need to be exposed publicly #1821

brentvatne commented Jun 30, 2015

ide commented Jun 30, 2015

orktes commented Jul 1, 2015

brentvatne commented Jul 4, 2015

ide commented Jul 4, 2015

dsibiski commented Jul 6, 2015

mattmo commented Jul 7, 2015

brentvatne commented Jul 7, 2015

brentvatne commented Jul 7, 2015

brentvatne commented Jul 7, 2015

ide commented Jul 7, 2015

brentvatne commented Jul 9, 2015

orktes commented Jul 9, 2015

brentvatne commented Jul 11, 2015

chirag04 commented Jul 18, 2015

sahrens commented Jul 18, 2015

brentvatne commented Jul 18, 2015

ide commented Jul 18, 2015

vjeux commented Jul 18, 2015

sahrens commented Jul 18, 2015

jeanregisser commented Jul 29, 2015

skevy commented Jul 29, 2015

brentvatne commented Aug 2, 2015

brentvatne commented Sep 25, 2015

vjeux commented Sep 25, 2015

brentvatne commented Oct 2, 2015

brentvatne commented Oct 9, 2015

TuangWang commented May 25, 2016

[Public Interface] Create whitelist internal modules that need to be exposed publicly #1821

[Public Interface] Create whitelist internal modules that need to be exposed publicly #1821

Comments

brentvatne commented Jun 30, 2015

ide commented Jun 30, 2015

orktes commented Jul 1, 2015

brentvatne commented Jul 4, 2015

Likely candidates for inclusion

Not sure what we should do with these

Documentation

(Mostly) complete list of repos that will be affected

Notes

ide commented Jul 4, 2015

dsibiski commented Jul 6, 2015

mattmo commented Jul 7, 2015

brentvatne commented Jul 7, 2015

brentvatne commented Jul 7, 2015

brentvatne commented Jul 7, 2015

ide commented Jul 7, 2015

brentvatne commented Jul 9, 2015

orktes commented Jul 9, 2015

brentvatne commented Jul 11, 2015

chirag04 commented Jul 18, 2015

sahrens commented Jul 18, 2015

brentvatne commented Jul 18, 2015

ide commented Jul 18, 2015

vjeux commented Jul 18, 2015

sahrens commented Jul 18, 2015

jeanregisser commented Jul 29, 2015

skevy commented Jul 29, 2015

brentvatne commented Aug 2, 2015

brentvatne commented Sep 25, 2015

vjeux commented Sep 25, 2015

brentvatne commented Oct 2, 2015

brentvatne commented Oct 9, 2015

TuangWang commented May 25, 2016