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

[Dependencies] Module system (fantastic project for interested contributors!) #235

Closed
ide opened this issue Mar 26, 2015 · 33 comments
Closed

[Dependencies] Module system (fantastic project for interested contributors!) #235

ide opened this issue Mar 26, 2015 · 33 comments

Comments

@ide
Copy link
Contributor

@ide ide commented Mar 26, 2015

Currently React Native ships with a set of modules, some of which are ubiquitous (basic View support) and some that are used only in a handful of apps (e.g. Vibration support). As the set of modules grows, the size of the 'react-native' JS will also increase and slow down the boot time for apps. There's already overhead on the order of a couple hundred milliseconds spinning up a JSContext and loading every single Obj-C class (#69) so reducing the startup cost is valuable. Also there needs to be a module system in place to support third-party components, especially those with a native implementation.

Features:

My current thinking is a good module system would let you:

  • Pay for only what you use. Ex: if you don't use a database module then your app shouldn't include the JS for it (string loading + parsing + evaluation cost) nor the native binary code (app size cost).
  • Dedupe native dependencies. This is a requirement since you cannot have two Obj-C classes with the same name. Ex: two modules may both use CocoaLumberjack, a very popular logging library. The module system should determine which version of CocoaLumberjack is compatible with both modules and download and link just one copy. Good news is that the CocoaPods system already does this with a constraint solver!
  • Dedupe JS depedendencies. Many JS components will use the same npm modules. Unlike Node on the server (boot time is hidden from the user + powerful hardware available), extra JS on the client (esp. mobile clients) adds a user-facing cost. So . This is not a hard requirement unlike with the native dependencies, but still important.

Design:

The gist is that the modules should be hosted in npm and will include both a package.json and a podspec (similar in spirit to package.json but for Obj-C and Swift). The package manager client (that is, what this project is about) looks inside node_modules for React Native components and updates a Podfile to reference these modules. The rest of the work is delegated to CocoaPods:

File hierarchy:

Podfile
Autogenerated-npm-Podfile  # created by the tool
node_modules/
  react-native-page-view-controller/
    package.json
    index.js
    ios/
      RCTPageViewController.h
      RCTPageViewController.m
      react-native-page-view-controller.podspec

Podfile

# all your normal dependencies, here are some examples
pod 'AWSiOSSDKv2', '~> 2.1'
pod 'CocoaLumberjack', '~> 2.0'
pod 'FMDB', '~> 2.5'
pod 'GoogleAnalytics-iOS-SDK', '~> 3.0'
pod 'Reachability', '~> 3.2'

require 'Autogenerated-npm-Podfile'

Autogenerated-npm-Podfile

# @generated by hypothetical tool
# Reference the local pods that live in node_modules
pod 'react-native-page-view-controller', 'node_modules/react-native-page-view-controller/ios'

What the tool does:

  1. Scans node_modules for the podspecs
  2. Adds references to them in an autogenerated file that is loaded from the Podfile (Podfiles and Podspecs are just Ruby)
  3. Runs pod install or pod update, whatever is appropriate

The workflow

npm install react-native-page-view-controller
cool_tool
# open App.xcworkspace

There are details to be worked through but I believe this is directionally correct.

Skill set:

Strong familiarity with npm and iOS development is helpful, as well as some familarity with CocoaPods. You could learn some of these as you go though you'll be more comfortable with an advance understanding of how the pieces fit together and what npm and CocoaPods can and can't do.

I'm happy to provide feedback, direction, or mentorship if someone or a pair of people want to take up this endeavor - it should be a worthwhile project that lots of people will use.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Mar 31, 2015

I'll definitely take a stab at this. Something to think about is whether there should be a value for react-module in its package.json to let the manager know that it is a react module. That way, if a node module includes a podfile for other reasons, the react package manager won't try to add it as a dependency.

@ide
Copy link
Contributor Author

@ide ide commented Mar 31, 2015

@tommchugh Great to hear you're interested. I agree it makes sense for package.json to denote that the package is a React Native module for the reason you mentioned and also it could be useful to store extra metadata (ex: which platforms are supported).

@corymsmith
Copy link

@corymsmith corymsmith commented Mar 31, 2015

I'm working on taking a crack at this right now, agree on a entry in package.json, was thinking about that as well.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Mar 31, 2015

You can take a look at what I have right now: https://github.com/TomMcHugh/react-native/tree/extension-manager/react-extension-manager

Pretty rough and just threw it together in the past couple of hours, but so far it has the basic functionality.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Mar 31, 2015

One thing that I found was that Podfiles aren't very friendly to using require. Might have to look into that. Because of that, I just inject the dependencies straight into the users Podfile.

@corymsmith
Copy link

@corymsmith corymsmith commented Mar 31, 2015

Looks great, pretty much what I was just writing! Good call on injecting right into the podfile.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Mar 31, 2015

Yeah, it's important it doesn't mess up a users Podfile. Thats why I have the rem init function create a commented out section where the package manager will look to inject the depenedency. It's a little hacky, but until we solidify whether we can do require on podfiles, it works.

@ide
Copy link
Contributor Author

@ide ide commented Mar 31, 2015

Perhaps eval instead of require is the right thing to do since the autogenerated file should have access to all of the functions that the main Podfile does. I believe this matches how CocoaPods loads the main Podfile in fact: https://github.com/CocoaPods/Core/blob/master/lib/cocoapods-core/podfile.rb#L254

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Mar 31, 2015

Eval is what we're looking for. Just pushed it with some code restructuring changes to my forked branch. We now have a separate Podfile that is managed by the manager.

@ide
Copy link
Contributor Author

@ide ide commented Mar 31, 2015

@tommchugh Is your code in working condition? If so, do you want to create a new GitHub organization and repo with your work, and then we can publish it to npm and start trying it out? I have a couple of native components that are coming together like a camera and more advanced URL handling that I could try linking with your script.

@corymsmith
Copy link

@corymsmith corymsmith commented Mar 31, 2015

I was wondering the same thing, want to publish my react-native-icons project and also add support for using icons in a tab bar item :)

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Apr 1, 2015

@ide @corymsmith Yep, just pushed the package manager source with an example that includes Vibrations and Text modules. Here it is: https://github.com/ReactExtensionManager/ReactExtensionManager

You can read about using the example project in the instructions.md. Basically just using npm link, installing dependencies and then using this new package manager, but its always good to have in case anyone finds anything confusing.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Apr 1, 2015

Just beefed up the documentation and showed how to get the package manager installed threw npm install.

@joewood
Copy link

@joewood joewood commented Apr 1, 2015

That looks great. Any reason why it can't be npm -g installed, rather than cloned?

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Apr 1, 2015

@joewood just published it, updating the readme now

@vjeux
Copy link
Contributor

@vjeux vjeux commented Apr 1, 2015

Beware about npm install -g, it is extremely hard to get people to upgrade and will be challenging if there are two projects that need different versions

@joewood
Copy link

@joewood joewood commented Apr 1, 2015

It may be tough to use without it being on the path. Although, now npm accepts script parameters I've seen more people use npm run using a locally installed package.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Apr 1, 2015

@vjeux good thinking. For now installing global is hidden from docs, but supported. Instead I have created bash script that links to the cli and recommend using that as a local module.

@frantic
Copy link
Contributor

@frantic frantic commented Apr 2, 2015

As for cli - check out react-native-cli, it's basically a very thin wrapper that delegates all the work to local react-native.

@frantic
Copy link
Contributor

@frantic frantic commented Apr 2, 2015

Regarding Podfile - in any case we will need to patch user-generated Podfile to include the require/eval line. Why not just include stuff inline:

pod 'AWSiOSSDKv2', '~> 2.1'
pod 'CocoaLumberjack', '~> 2.0'
pod 'FMDB', '~> 2.5'
pod 'GoogleAnalytics-iOS-SDK', '~> 3.0'
pod 'Reachability', '~> 3.2'

# <react-native>
pod 'react-native-page-view-controller', 'node_modules/react-native-page-view-controller/ios'
# </react-native>
@neonichu
Copy link

@neonichu neonichu commented Apr 2, 2015

I'm thinking it might be nicer to have a corresponding CocoaPods plugin which automatically sources all Pods from node_modules

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Apr 2, 2015

@frantic I agree bringing it into the react-native-cli would be best. In my original project before using eval I had it directly injecting the dependencies. Whether we should have it inline or through an eval method is just up to whether we want just one podfile and everything is editable in one place, or whether the system should not interfere at all with the user's podfile and is managed in a single file separate from the regular podfile.

@ide
Copy link
Contributor Author

@ide ide commented Apr 3, 2015

@neonichu could you shed some more light on what a CocoaPods plugin could get us?

@neonichu
Copy link

@neonichu neonichu commented Apr 3, 2015

@ide I was thinking that it could replace the modifications to the user's Podfile.

@ligaz
Copy link

@ligaz ligaz commented Apr 3, 2015

It is good to consider how this might work for Android as well.

@tommymchugh
Copy link
Contributor

@tommymchugh tommymchugh commented Apr 3, 2015

Just finished implementing a little bit skimmed down version of ReactExtensionManager and brought it into the react-native-cli. Now react-native install command searches for React Components to install from package.json and installs them through Podfile.

https://github.com/TomMcHugh/react-native

Would love comments and feedback.

@frantic
Copy link
Contributor

@frantic frantic commented Apr 3, 2015

Would love comments and feedback.

Doing a quick pass, I'll leave my comments on the commits. Could you please squash the commit history - it's sometimes hard to follow, because some changes are undone in following commits. Also please use ESLint - we have a config checked in.

@sahrens
Copy link
Contributor

@sahrens sahrens commented Apr 22, 2015

Any more progress here?

@johanneslumpe
Copy link
Contributor

@johanneslumpe johanneslumpe commented Apr 23, 2015

Instead of having two commands to call - e.g. npm install x and cool_tool - wouldn't it be more user friendly if it were possible to just run cool_tool install x? Under the hood it would then install x and after a successful install it would do its own additional thing.

And if somebody doesn't like that, they could just go with npm install x and cool_tool separately. (cool_tool would just resolve the pod files, when called without arguments`

@ide
Copy link
Contributor Author

@ide ide commented Apr 23, 2015

@johanneslumpe Yeah, that seems reasonable and really good that you can use npm directly if you're used to it or have other tools that call into it.

@ide
Copy link
Contributor Author

@ide ide commented Apr 27, 2015

@sahrens we have the pieces of a module manager based on REM coming together. The hard part is supporting transitive dependencies (e.g. an "audio" module with some native code might be the dependency of both a "podcast" module and an "arcade" module -- what code should be included in the app? lots of choices here when taking versioning into account).

@brentvatne brentvatne changed the title Module system (fantastic project for interested contributors!) [Dependencies] Module system (fantastic project for interested contributors!) May 31, 2015
@ghost
Copy link

@ghost ghost commented Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

@ide
Copy link
Contributor Author

@ide ide commented Aug 4, 2015

Closing this out on the main repo - discussion can continue over at https://github.com/ReactExtensionManager/rem.

@ide ide closed this Aug 4, 2015
rozele added a commit to rozele/react-native that referenced this issue Apr 11, 2016
Fixes facebook#158 - Adds Chrome debugging to ReactNative for UWP
@facebook facebook locked as resolved and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.