-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
Creating Standalone JS File For Each Native Component #22990
Comments
I'll take |
I'd like to contribute! I'll work on |
Hello! |
Would love to help with |
I'll take RCTTabBar |
I'll take the AndroidProgressBar. |
I will try to help with |
Hello! I will take the |
Hello! I’ll work on RCTSlider |
I'll take the |
I'll take the |
I'll take the |
I'll take the |
I'll take |
I'll work on |
Summary: PR Related to: #22990 Changelog: [Android][Changed] - All the imports connected to requireNativeComponent in ActivityIndicator was moved to a seperate file. Pull Request resolved: #23104 Differential Revision: D13781451 Pulled By: cpojer fbshipit-source-id: 7204976d59a96abdaa81cdd7fd54fd001f7d1ee9
i think all is taken right ? :) |
@reemtariqq Unfortunately all are taken. However you can search for issues with "Good first issue" label. There are many of them |
Summary: Part of facebook#22990 Changelog: ---------- [iOS] [Changed] - Create individual ```RCTSnapshotNativeComponent``` JS file from ```SnapshotViewIOS``` Pull Request resolved: facebook#23012 Differential Revision: D13683566 Pulled By: PeteTheHeat fbshipit-source-id: d7a030dd59e9c5dd88f7ddd92734486063c5f450
Summary: This PR is related to facebook#22990 Changelog: ---------- [Android][Changed] - move the call to requireNativeComponent from `Checkbox.android.js` to `AndroidCheckBoxNativeComponent.js` Pull Request resolved: facebook#23003 Reviewed By: TheSavior Differential Revision: D13690827 Pulled By: rickhanlonii fbshipit-source-id: 08bc83a7f097414b5c833a3b43715e5aec277d65
Summary: Part of facebook#22990 Changelog: ---------- [iOS] [Changed] - Create individual `RCTSafeAreaViewNativeComponent` JS file from `SafeAreaView` with flow typing. Pull Request resolved: facebook#23006 Reviewed By: TheSavior Differential Revision: D13690683 Pulled By: rickhanlonii fbshipit-source-id: 7928fabf0264a6269a41148432f604e82d9b3920
Summary: This pull request is part of facebook#22990. Changelog: ---------- [iOS] [Changed] - Split RCTTabBarItem into RCTTabBarItemNativeComponent. Pull Request resolved: facebook#23004 Reviewed By: TheSavior Differential Revision: D13690820 Pulled By: rickhanlonii fbshipit-source-id: 522e55ee74a797a3930edf6821c6d8f72511cd2b
Summary: Changelog: ---------- [Android] [Changed] - As mentioned in facebook#22990, I have moved native components required by PickerAndroid.android.js into separate files and added Flow Typing. Pull Request resolved: facebook#22999 Differential Revision: D13697130 Pulled By: TheSavior fbshipit-source-id: 42da73d82cca45fefa066871eed5a637811643c8
Summary: This pull request is part of facebook#22990. Changelog: ---------- [iOS] [Changed] - Split RCTMaskedView into RCTMaskedViewNativeComponent. Pull Request resolved: facebook#23001 Differential Revision: D13697245 Pulled By: TheSavior fbshipit-source-id: 16af0b394ae32cd3c4992c2cd5ea2d3c140755b3
Summary: Thank you for sending the PR! We appreciate you spending the time to work on these changes. Help us understand your motivation by explaining why you decided to make this change: Changelog: ---------- [iOS] [Changed] - As mentioned in facebook#22990, I have moved native components required by DatePickerIOS.ios.js into separate files and added Flow Typing. Pull Request resolved: facebook#23013 Differential Revision: D13697591 Pulled By: TheSavior fbshipit-source-id: 5aec5a2270cbfc708f3e3a67662abd8071f1333f
Summary: [Android] [Changed] - create standalone JS file for ToolbarAndroid, as part of facebook#22990 I have a question. `ToolbarAndroid` component use 2 types from other files. They are not included files mentioned by facebook#22990 (comment). One type `ColorValue` is very simple and I'll copy it. But `ImageSource` type is a bit complicated I think it wolud be better to add to the codegen. How do you think about it? TheSavior Update: -> solved 👍 ---------- - [x] yarn lint - [x] yarn flow-check-android - [x] yarn flow - [x] yarn test Pull Request resolved: facebook#23009 Differential Revision: D13697524 Pulled By: TheSavior fbshipit-source-id: 72ce43e1510be0986a3798d795f07ab176bd2a92
Summary: Changelog: ---------- [iOS] [Changed] - follow facebook#22990, move the call to requireNativeComponent from `SegmentedControlIOS.ios.js` to `RCTSegmentedControlNativeComponent.js` Pull Request resolved: facebook#23000 Differential Revision: D13697168 Pulled By: TheSavior fbshipit-source-id: 83a66279d2fcd5a29d2ebc9cf5b5273947d96281
Summary: This PR is related to facebook#22990 Changelog: ---------- [iOS] [Changed] - move the call to requireNativeComponent from Modal.js to RCTModalHostViewNativeComponent.js Pull Request resolved: facebook#23030 Differential Revision: D13710032 Pulled By: cpojer fbshipit-source-id: 822284a639f38721442c67ceff98fc99495c31b9
Summary: Changelog: ---------- [iOS] [Changed] - moved RCTRefreshControl from RefreshControl as a separate component, as mentioned in facebook#22990 [Android] [Changed] - moved AndroidSwipeRefreshLayout from RefreshControl as a separate component, as mentioned in facebook#22990 Pull Request resolved: facebook#23039 Reviewed By: rickhanlonii Differential Revision: D13710076 Pulled By: cpojer fbshipit-source-id: 332520b74d6fc73e50dbe511dae22f82015c2d3a
Summary: Changelog: ---------- [Android] [Changed] - As mentioned in facebook#22990, I have moved native components required by DrawerLayoutAndroid.android.js into separate files and added Flow Typing. Question ---------- I have two questions. It is not included in the files mentioned by facebook#22990 [comment](facebook#22990 (comment)). Perhaps we should be adding knowledge of that type to the codegen since it is quite complicated, what do you think TheSavior? The `Props` type include `renderNavigationView: () => React.Element<any>,` and `children?: React.Node,`. Therefore I added `const React = require('React');` to the file, is it ok? Pull Request resolved: facebook#23036 Differential Revision: D13710035 Pulled By: cpojer fbshipit-source-id: 671423b76c3b443d85e4b63d05d6253dbd33b29c
Summary: Created Standalone JS file for RCTInputAccessoryView native component facebook#22990 Changelog: ---------- [iOS] [Changed] - Created Standalone JS file for RCTInputAccessoryView native component Pull Request resolved: facebook#23050 Differential Revision: D13735644 Pulled By: TheSavior fbshipit-source-id: 64b091957b38cb11d804582f185d5cb0c6754af3
Summary: [iOS] [Changed] - As facebook#22990 said, move requireNativeComponent to a separate file. I am not familiar with flow, I try to follow the https://pastebin.com/RFpdT76V example but I am not sure I have done it right. Pull Request resolved: facebook#22996 Differential Revision: D13697082 Pulled By: cpojer fbshipit-source-id: c0c87a8e1a7f0553da994aba230f69b496140200
Summary: his PR is related to facebook#22990 Changelog: ---------- [Android][Changed] - move the call to requireNativeComponent from ProgressBarAndroid.android.js to ProgressBarAndroidNativeComponent.js Pull Request resolved: facebook#23068 Differential Revision: D13760445 Pulled By: cpojer fbshipit-source-id: b74ff42c6f207803de70be549a13487d59125a66
Summary: Changelog: [iOS] [Changed] - Deal with facebook#22990. Move `requireNativeComponent` to a separate file. Pull Request resolved: facebook#23048 Differential Revision: D13760373 Pulled By: cpojer fbshipit-source-id: ff8cc9d468dc3bac55fc3d4156ad695dcdd10ab8
Summary: Created a standalone JS file for the `RCTProgressView` native component. This PR is part of facebook#22990. Changelog: ---------- [iOS] [Changed] - Created a standalone JS file for the `RCTProgressView` native component Pull Request resolved: facebook#23077 Differential Revision: D13760036 Pulled By: cpojer fbshipit-source-id: 0f449528b28fde089d9c6b0eb9b752dee3a85af6
Summary: [Android][Changed] - All the imports connected to `requireNativeComponent` in `ViewPager` was moved to a separate file. Issue in focus: facebook#22990 Pull Request resolved: facebook#22995 Differential Revision: D13760459 Pulled By: cpojer fbshipit-source-id: fca1633ce933ea4909ef81d7bbe8123d654e24fb
Summary: His PR is related to facebook#22990 Changelog: ---------- [IOS][Changed] - move the call to requireNativeComponent from TabBarIOS.ios.js to RCTTabBarNativeComponent.js Pull Request resolved: facebook#23118 Differential Revision: D13781428 Pulled By: cpojer fbshipit-source-id: 3034c7db127a992c5757d70e22d98ee9acf4847b
Summary: Changelog: ---------- [iOS][Changed] - added types to RCTSnapshotNativeComponent.js, as mentioned in facebook#22990 Pull Request resolved: facebook#23111 Differential Revision: D13781429 Pulled By: cpojer fbshipit-source-id: 7efffe150fd29cbfbb3a6b8f13e38295f83acb3c
Summary: PR Related to: facebook#22990 Changelog: [Android][Changed] - All the imports connected to requireNativeComponent in ActivityIndicator was moved to a seperate file. Pull Request resolved: facebook#23104 Differential Revision: D13781451 Pulled By: cpojer fbshipit-source-id: 7204976d59a96abdaa81cdd7fd54fd001f7d1ee9
Thank you guys for all the work here! We have another opportunity to contribute to an RN project: TurboModules. Please check out #24875. |
We are going to be generating native code from flow types in JS for our native components. In order to do this we need to have an individual JS file for each native component with complete flow types.
Step 1, Step 2, Step 3, and Step 4 have all been steps towards this goal.
In a bunch of our JS files we choose a component based on platform, typically looking something like this:
we want to change this code so that each
requireNativeComponent
call lives in its own file, thus changing the above code to look like this:We need your help to make this change!
How to help
Pick one of the native components from the list below. Comment that you are working on that file and try to choose one that nobody else has commented about already.
Search the codebase for .js files that include that name. We are specifically looking for
requireNativeComponent
calls. So if you pick the componentAndroidCheckBox
we'd be looking for this callsite.Create a new js file suffixed with 'NativeComponent' right next to the file that you found. In this case we'd create a
AndroidCheckBoxNativeComponent.js
file inLibraries/Components/CheckBox
.We should move the call to requireNativeComponent from
Checkbox.android.js
to this file, including the flow type if one exists, or defining one if it doesn't.A pastebin of what I think the changes look like for this component are here: https://pastebin.com/RFpdT76V
One thing to note is that the majority of types necessary for these files shouldn't be imported from other files. Our codegen can't yet handle importing types from other files so in most cases I'd prefer to duplicate the type in this new file if it is needed. There are some exceptions to this that the codegen can handle that I'd expect most of these files will need. Specifically:
If you come across other types that you think are sufficiently complex that you think shouldn't be duplicated tag me on your PR. Perhaps we should be adding knowledge of that type to the codegen. 😄
Testing Your Changes
For the highest confidence in your change, we recommend testing your changes using the RNTester App. Instructions to install and test the app are in the readme in the RNTester directory. If your component is an Android component you should test it on Android. If it is iOS, then test on iOS. 😄
The Files
The text was updated successfully, but these errors were encountered: