-
Notifications
You must be signed in to change notification settings - Fork 127
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
Lean Core #6
Comments
I don't know that this will necessarily be the case. If code ends up being spread throughout many modules and repos, it's going to be hard to find exactly the code we're looking for. Having the React Native renderer in the React repo is already an example of this. It's trivial to search the repo now for the specific component you're looking for and find all the code related to it.
I believe we can accomplish this in ways that don't include separating code in the repo. Have we explored using Lerna or Yarn workspaces to help alleviate some of this? |
👋 @eliperkins I see you points, just want to underline how this is a temp post while the proposal is being written up :) You should discuss those points there once it's up (which should be soon, @axe-fb is writing it AFAIK). |
I think components like
Metro should support tree-shaking. Then RN can use ES exports which will reduce the bundle size for JavaScript code that's not used. For native code, on iOS, devs have to include the modules they are going to use, not sure why this isn't the case on Android. But it would help to reduce the native code size. |
@kelset - I started working on a proposal, but I think that each Native Module and View Manager may be slightly different. Instead of writing one blanket proposal for all components, I think it will be more actionable to write up proposal for sets of components. I am thinking that the work itself could be done in two phases. Here is a set of Native Modules that I have looked at. Phase 1Remove all components that we think can be deprecated. This is the rows in yellow. Phase 2Remove maintained components out of the repo. Some of the other reasons I think we should do this
|
@eliperkins - Thanks for taking a look at the idea.
I would like to believe that in most cases, the code of a component or native module should live at one single place. Today, JS and Native code is separated, which makes it harder. But putting them all in one repo, it would be much easier to develop/debug.
I think you are right. We would like to eventually like to move to a world where React is the top level repo, and REact-dom is a renderer, just like React-native, or React-VR. I think that by separating core from components and native modules would help with that.
Also be making core components like If you think these explanations don't make sense, please let me know - I can try to elaborate !! |
@satya164 |
Tree-shaking is when the bundler marks unused imports as dead code which is then removed by the uglifier. Since ES imports are statically analyzable, it's easy to detect which modules are unused and mark them as dead code. All the major bundlers on the web support it, such as Webpack, Rollup, and Parcel. Rollup and Parcel also support tree-shaking on CommonJS modules. Metro doesn't support this feature. So when you start a React Native iOS project, not all of the native modules and components are linked by default. If you want to use a native module that's not linked by default, you need to link it manually (for example the blob module on iOS is not linked by default). Whereas on Android all of the available native modules and components are already linked by default. |
We used Lerna on a project (https://github.com/infinitered/gluegun) and, while it's a very cool system, I found it wasn't really worth the extra confusion it generated among new contributors. |
I remember a year ago or so, someone came up with a list of the components that every renderer needed to support if they were going to be able to able to render an app. It was things like All of that to say, it seems like the React Native repo should have the minimal list of components/APIs/code required for every app and then everything else should be in a separate repo |
At React Native EU today there was a conversation with some of the core contributors around why we haven't removed NavigatorIOS yet. The answer was that it is part of the slimmening effort but just nobody had done it yet. @fkgozali just put up an internal commit removing it from React Native. That will be landing shortly. |
Summary: Legacy navigator impl. There are other alternatives that should be used instead. Part of the slimmening effort as described here: react-native-community/discussions-and-proposals#6 Reviewed By: TheSavior Differential Revision: D9677824 fbshipit-source-id: 24ae500751d2a8c398f246d36604a58f0b3c113b
here’s the commit: facebook/react-native@0df92af |
@kelset is this still planned? |
No, actually 😅 I'm planning on reworking the top post a bit (based on a couple things discussed during RNEU) but haven't had time yet 🙈 Will do it this week |
@turnrye - I think we don't need a top level PR. We could have PRs for individual components that we want to remove and move out - like what we did for webview. |
I've updated the top post, let me know / let's discuss about how to make the slimmening better. |
Thank you for updating the original issue @kelset with some recent changes we have discussed together. Generally speaking, I really like the direction we are moving towards. I have one comment on the roadmap for deprecating and eventually, removing modules from the core. As a part of the first step (listed below):
I would also think about removing the module that is the subject of a particular extraction and requiring it from the new module right away. That way, we would automatically opt-in everyone for the latest version of a particular module and make it easy to verify that the newly created API and established release process works without issues. I would imagine we could print a deprecation message along the following lines:
|
@kelset ,NavigatorIOS will be removed in rn-0.58,would you put it in facebookarchive/react-native-custom-components |
Summary: Legacy navigator impl. There are other alternatives that should be used instead. Part of the slimmening effort as described here: react-native-community/discussions-and-proposals#6 Reviewed By: TheSavior Differential Revision: D9677824 fbshipit-source-id: 24ae500751d2a8c398f246d36604a58f0b3c113b
@linjson I suggest you do it yourself if you still need that component. Or maintain your own version of it. |
Does anybody have a rough estimate for impact on bundle size & startup time when removing NavigatorIOS + WebView? |
@ricardobeat This tweet by @fkgozali suggests that with NavigatorIOS removed, the savings should be 20kb+. I guess @jamonholmgren would have more info on the webview savings once he has extracted it from core and deleted the necessary code. |
That said, since those two extractions will happen as separate times (the full extraction of WebView won't happen in 0.58) I don't think it's too relevant to get a combined "benchmark". |
I don't currently have an estimate on savings, but that would be interesting to know. |
@ericketts - as someone currently experiencing a migraine, albeit for unrelated reasons, i feel your pain. also as a maintainer of one of the navigation solutions, i see where you're coming from - asyncstorage and maskedviewios are now two more libraries that we'll need users to install, or we need to instead remove the features that they provide and require users to inject their own tools to do their jobs. that said, i can't see how it would be any better if navigation itself was internal to the react-native repository. it used to be, and it just slowed down iteration speed and made other navigators seem like secondary options because they weren't "blessed" by being part of core, regardless of their merit. further, i think we need to accept the reality that facebook has no interest in maintaining a large react-native core that includes code that they don't use in their apps. i work on expo and we provide a large set of apis out of the box, and we're also including the packages that are being split out of core (a few will go into our next release in a few weeks or so). in my opinion expo can help a lot with approachability of react-native. other tools like ignite provide opinionated boilerplates and are another reasonable alternative. in general there's an opportunity for the community to build end-to-end frameworks around react-native. react-native will end up as the rendering/plugin layer, and the pieces that are built on top of it will be up to individuals and companies in the community. my only fear is that the instability from core that we have seen since the start will continue, and folks working with react-native will become fatigued with keeping up to breaking changes and regressions. i imagine it'll get worse before it gets better, with turbomodules and fabric coming and representing a significant change to core, but if this helps us get to a point where the core can be relatively stable it'll be worth it. |
yeah this is really the truth, despite sometimes in the past my trying to pretend its not haha. it also scares the hell out of me for the future of RN as a whole if facebook ever decided that native only is their path forward. I hope the community could and would step up, and I guess this will be something of a test of the potential for that (eventuality of that?) (also you guys at expo are doing some great work for lowering barrier to entry on RN, and I thank you for it, despite having decided to go with a more piecemeal solution for various reasons (some of which contain a bit of irony given the position of my previous post, so I'll edit them... for brevity)). as you @brentvatne say and I agree, there will likely be growing pains here, and I still have concerns about this project (both on the tech and human sides of the undertaking and results thereof); but there's also a lot of growth going on with RN that looks quite promising, still much to be excited about on the horizon. I just wanted to get my few cents in, I know others had expressed some of my concerns more succinctly, but the conversation still seemed a bit more in the "yea" camp, so I'll throw my cautious "ehhh..?" into the books. edit: also hoping your head feels better soon mate, those things are the worst. |
Let's have this discussion somewhere else, this is not part of the Lean Core process. I understand your frustration but @brentvatne made a great reply.
I agree it may sound counterintuitive but if you look at webview, they (cc @Titozzz) actually managed to merge more than 100 PRs since it's extraction: https://github.com/react-native-community/react-native-webview Other components like react-native-cli and react-native-netinfo are also success stories: useful modules will find awesome dedicated maintainers who can spend more time on solving a well-scoped problem than we could by mixing everything together. For some of these modules, this is the first time they are receiving real attention. Similarly, this effort also allows us to do a much better job at managing the React Native repo itself. As you can see, the number of open issues and pull requests is as low as it was when React Native was initially open sourced! I realize though as you point out that there my be a small period of pain until we get everything into order. The new autolinking of native dependencies that will likely be part of 0.60 will help ease the pain though. We won't remove any actual components until then (except WebView). All that being said: if something is extremely important to people and the separation is causing pain to the community we will consider bringing the module back into core. That reversal would also be a much simpler process :) |
(this is slightly off topic so skip it if you're trying to follow the thread)
we're working towards making expo both a cohesive end-to-end tool while also letting folks who don't want that opt-in in a piecemeal style - more on that in https://blog.expo.io/you-can-now-use-expo-apis-in-any-react-native-app-7c3a93041331. |
@michaelknoch what did you guys end up doing re: |
Summary: Legacy navigator impl. There are other alternatives that should be used instead. Part of the slimmening effort as described here: react-native-community/discussions-and-proposals#6 Reviewed By: TheSavior Differential Revision: D9677824 fbshipit-source-id: 24ae500751d2a8c398f246d36604a58f0b3c113b
How is the removal of Webview going to affect those who use RN with Expo. |
@Shpadoinkle Presumably Expo will include the webview plugin by default like it includes various other native plugins. |
indeed expo/expo#3748 |
This comment has been minimized.
This comment has been minimized.
Summary: Legacy navigator impl. There are other alternatives that should be used instead. Part of the slimmening effort as described here: react-native-community/discussions-and-proposals#6 Reviewed By: TheSavior Differential Revision: D9677824 fbshipit-source-id: 24ae500751d2a8c398f246d36604a58f0b3c113b
support this explain, but i'm very confuse about
and it's really removed from js code, but still remain in native code camera, that's means i can use |
I expect the native code to be removed for anything where the JS code was removed. If it hasn’t been already then I expect it to in the future. |
@TheSavior is correct. We are gradually removing things and you may see some leftovers in some releases. I highly recommend against accessing them as they'll go away soon. |
@cpojer |
Summary: Changelog: [**General**][**Removed**] - Removed AsyncStorage usage from RNTester As part of the "Lean Core" efforts (see react-native-community/discussions-and-proposals#6) to remove outdated and/or unused components (status: https://gist.github.com/Simek/88a9f1a014a47c37f4fce3738864d2e1), this diff removes usage of the deprecated AsyncStorage API from RNTester. RNTester is intended as a reference to showcase various components and APIs. The implications of the replacement of AsyncStorage in RNTester with in-memory management of state is a tradeoff of persistance to a lighter weight implementation and user predictable behavior. 1. Removed AsyncStorage from rn-tester - removed Navigation and bookmark persisting from reducer - moved JS Stalls and tracking to application state with context and reducer 2. Fixed InternalSettings Example bugs Reviewed By: lunaleaps Differential Revision: D35435562 fbshipit-source-id: a879787d8683a1c452e5b6b75a9e01f3ceadfe5d
Summary: Changelog: [**General**][**Removed**] - Removed AsyncStorage usage from RNTester As part of the "Lean Core" efforts (see react-native-community/discussions-and-proposals#6) to remove outdated and/or unused components (status: https://gist.github.com/Simek/88a9f1a014a47c37f4fce3738864d2e1), this diff removes usage of the deprecated AsyncStorage API from RNTester. RNTester is intended as a reference to showcase various components and APIs. The implications of the replacement of AsyncStorage in RNTester with in-memory management of state is a tradeoff of persistance to a lighter weight implementation and user predictable behavior. 1. Removed AsyncStorage from rn-tester - removed Navigation and bookmark persisting from reducer - moved JS Stalls and tracking to application state with context and reducer 2. Fixed InternalSettings Example bugs Reviewed By: lunaleaps Differential Revision: D35435562 fbshipit-source-id: a879787d8683a1c452e5b6b75a9e01f3ceadfe5d
Intro
With this issue I'd like to try and create a "one stop" for all the information available around the discussion that has been going on for a long time within the Core team about this project - and explain the label.
The Gist of it
React Native is currently a huge repo. Would it make sense to move UI components (ScrollView, Switches, WebView) and Native Modules like PushNotifications, etc into a separate repos?
The basic answer is yes, and in the past few months this has started becoming a reality via a few proposals and some commits from the FB team.
You can check the full list of native components that are being considered for extraction/deprecation here, but proposals can also be about JS-only components.
Advantages
Reduce the app size effect of adding React Native to an app
Doubts (with "answers" to discuss about)
A) Should we define a roadmap that each "separation" will have to follow?
The rough rule, for now, is to follow this set of steps:
For "pure removal" of old code, this may happen in a "2 version" window instead (ex. the old
NavigatorIOS
, removal announced in 57 and will happen in 58).This is still a WIP so feedback on how to make it better is appreciated. Ideally we'd have a dedicated document with the details on how the flow of an extraction works, so that everyone on the community can understand all the steps involved and can take ownership of a proposal for an extraction.
B) How would this affect platforms like React-native-windows?
Owners of those out-of-tree platforms should be involved in the conversations to ensure a smooth transition.
B1) What's the best way to ensure that Expo is not left behind when the components are moved outside?
C) Who would retain ownership (and "responsibility to maintain") the separated component?
For "extractions", the new repositories will live under the
react-native-community
umbrella. Details around npm deployment are still being discussed, expect a dedicated issue about that.For "deprecations/removals" they will be probably be just removed from the main repo, and eventually "ported" to this repository for deprecated-modules by FB by developers that still need it.
EDIT: this has been massively edited to provide more informations about the slimmening and reflect the ongoing discussion. And since things are still moving, please keep an eye on this post because it will evolve more.
The text was updated successfully, but these errors were encountered: