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

New Project Structure Proposal #6292

Closed
foghina opened this issue Mar 4, 2016 · 13 comments
Closed

New Project Structure Proposal #6292

foghina opened this issue Mar 4, 2016 · 13 comments
Labels
Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.

Comments

@foghina
Copy link
Contributor

foghina commented Mar 4, 2016

The problem

We generate a lot of boilerplate code (the ios/ and android/ folders containing the native app code) into people's projects / repositories. This is not uncommon, a lot of frameworks do this, but they generally do it for other reasons. They do it to impose a certain structure that they then expect when running the app. We only expect an index.js file, the rest of the stuff that we generate is just boilerplate that most people never change and/or have a hard time maintaining.

The solution

Generate the Android / iOS apps in a hidden, vcs-ignored folder (e.g. .reactnative/). This will remove the boilerplate from people's projects and give us greater control over the app templates. It also gives us the freedom to re-generate the templates whenever we please (e.g. when you switch to a newer version of React Native), since they will never be touched by developers. This also means no more upgrade command, making switching to a new React Native version much easier.

Essentially, the new structure would look something like this:

.
├── .node_modules
│   └── react-native
│       └── ...
├── .react-native
│   ├── android
│   │   └── [generated]
│   └── ios
│       └── [generated]
├── index.js
└── package.json

Challenges

  1. Project meta-data (e.g. name/label, package name etc.): we would need to store these in package.json (can we, or do we have to invent another file?) and read them at generation time.
  2. Managing dependencies: this actually makes it easier. We can scan for rnpm dependencies and add them at generation time. This means we would regenerate every time you add a dependency or even when you execute e.g. run-android.
  3. Allowing people to write native code: since the developer doesn't have access to the native projects anymore, this is significantly harder. We can let people supply a “main” rnpm module that lives in the same project and is simply linked at generation time. However, this limits developers to writing native modules and view managers (on Android it would be possible to write secondary Activities and other components like Services, too; not sure about iOS). If they need to change the main entry point (MainActivity / AppDelegate (?)) it would be kinda impossible. However, if you need to do that, your project starts to fall into the “embedding React Native into existing apps” category.
  4. Embedding React Native into existing apps: we have to continue supporting this. It shouldn't be hard: simply don't use any of the generation stuff and link to the libraries bundled in the node module, write your own MainActivity / AppDelegate using the samples provided.
  5. We'd have to solve all of these problems on both platforms and in one go, and provide a clear migration path come release time.

Going forward

First, I'm curious what people's thoughts are. Would this be valuable? Are there any huge blockers that I'm not seeing? For example, I'm worried about all rnpm modules having to match a project's react-native dependency version making upgrading hard. But I'm guessing this is already an issue.

Second, I will not have time to work on this anytime soon. I might in the future, but no guarantees. I'd be thrilled to have someone in the community own this though and more than happy to review PRs.

@grabbou
Copy link
Contributor

grabbou commented Mar 4, 2016

Honestly, to me, the only real issue was these two extra folders living at the root. As projects get bigger and new tools are added, I like to keep the structure sleek w/o two extra folders on top of that. In all my projects I've been successfully changing the structure and moving them to native folder, so it looks just like in the snippet above.

See for example this project (universal):

Having said so, that would've been my main motivation. From rnpm side (since I see it mentioned), it does not matter what's the structure since we infer folders during runtime (in fact, lots of native android module authors come up with different structures, not always mirroring the preferred one).

I am mainly concerned about limitations this means for native code there's usually laying in the app. Sometimes you have to perform extra actions in AppDelegate (well, we could wrap that in a yet another package, called Main), but having entire .xcodeproj being automatically generated sounds a little hard to get into production stage. Most importantly, stuff like Cocoapods or even changing the application splash xib with your custom (not always an image) view is going to be definitely harder. I understand we might want to enforce some set of common settings for all React Native project, but still I have that feeling xcodeproj settings might be quite different (provisioning profile, certs or app entitlements) for instance.

On the other hand, the current state of things is far from perfect, mainly when it comes down to running upgrade command. This is by the way an issue I was going to work on next week - e.g. when you have different linker_flags than the default specified, upgrade command will completely overwrite them. In practice, that means if you are using Cocoapods - you get lots of build errors because we strip $(inherited) flag.

Regarding point 5, I think it could be just a matter of a flag in rn-cli.config.js, e.g. autoGenerated: false to turn this off (which means you can safely operate on initial content of react-native folder that's been generated with react-native init). That also means we can at least bring this partially to production and add auto_generation later on (or even in smaller chunks)

@foghina
Copy link
Contributor Author

foghina commented Mar 4, 2016

@grabbou thanks for your comments. I don't have much context on iOS (I mostly work on Android) -- it sounds like we've got some unique challenges there but it also sounds like they could be solved by adding more configuration to the generation process?

Regarding point 5, I think it could be just a matter of a flag in rn-cli.config.js, e.g. autoGenerated: false to turn this off

That's pretty smart, I like it.

@corbt
Copy link
Contributor

corbt commented Mar 5, 2016

I'm writing a React Native project focusing on iOS to start. I consider this a "pure" RN project as opposed to just "RN embedded" because the vast majority of it has been implemented in React Native, but even so I've had to make changes to my AppDelegate.m to support the following behaviors:

  1. Add exception monitoring/logging with Sentry.
  2. Customized the location of the JS bundle in development for debugging on-device.
  3. Add CodePush support for upgrading the bundle in production.
  4. Add code to initialize the RN-controlled RootView with my LaunchScreen to avoid a white flash at startup (detailed in [Initialization] Launch screen white flash #1402).
  5. Implement handleEventsForBackgroundURLSession to perform actions when an upload I've started in the background finishes.

I've also made some minor customization to my build process, specifically allowing for local/staging/prod builds with different endpoints, that I wouldn't want to be overwritten.

All of these (maybe with the exception of 5) are pretty common needs even in "pure RN" apps, and I think it would be good for a solution to keep those things easy to do.

Anecdotally, I haven't had too much trouble with the current react-native upgrade process, I just manually review the diff for every conflicting file and figure out whether I should accept the change or not. Of course it would be nice not to have to do that if we can figure out a clean way to make that happen.

@satya164
Copy link
Contributor

satya164 commented Mar 5, 2016

Not entirely sure what's the best approach. But I'll share my thoughts.

I think changing the Native files is very common for lots of people. For example, on Android, say you want to add push notifications, analytics etc. you cannot do that without touching native files. Often third party plugins require changes to native files to work. And many other cases @corbt and @grabbou mentioned. So people often actually need to change certain files.

Another use case is to change package name of the project. Currently we generate the package name from the package.json's name field, which may or may not be appropriate. And I think that's one thing which causes problem while react-native upgrade, at least for me. (Related #6238).

If we solve the package name problem, now comes the auto-generated bolierplate issue. I think the first thing we should do is try to move as many files into the library itself, for example, what we did with the ReactActivity. Another such file is the react.gradle. Can it be inside node_modules/react-native instead of the project? (I dunno the answer)

Once we do that, it should be pretty trivial to upgrade I think. We could still move the android and ios folders into a react-native folder though.

@rclai
Copy link
Contributor

rclai commented Mar 5, 2016

It would be great to have an easier upgrade process. Writing custom stuff is my main concern in this. Using rnpm to do this seems fine. Regarding the app delegate and root view controller, perhaps there can be a programmatic way to hook into it from a custom rnpm module middleware style to help attach launch screens, change the dev server ip, etc?

@brentvatne
Copy link
Collaborator

Echoing the comments above, it's very common to change your AppDelegate/Info.plist and MainActivity/AndroidManifest.xml. I'm afraid that if we try to hide these through a leaky abstraction we will add more complexity than we are shaving off by simplifying the upgrade process.

@grabbou
Copy link
Contributor

grabbou commented Mar 6, 2016

@brentvatne +1 that, for instance - we add custom fonts with rnpm to Info.plist since that's easy, but there might be some other cases we simply don't think about at the moment which will lead users to fill in an issue/feature request later.

@jaygarcia
Copy link
Contributor

Hiding core react-native project stuff in .react-native will make it harder for folks to edit code in that space, some of which is required.

I like the idea, but maybe just rename it to native or react-native.

@cosmith
Copy link
Contributor

cosmith commented Mar 9, 2016

@facebook-github-bot label For Discussion

@facebook-github-bot facebook-github-bot added Type: Discussion Long running discussion. Ran Commands One of our bots successfully processed a command. labels Mar 9, 2016
@foghina
Copy link
Contributor Author

foghina commented Mar 21, 2016

For AndroidManifest.xml / Info.plist we could add a system where rnpm modules can provide custom data to write into those files. They have a pretty standard format so it shouldn't be super hard. We can also let people configure these files via package.json (e.g. adding <meta-data> to AndroidManifest.xml for Google Maps API keys).

MainActivity / AppDelegate - we should just make these super easy to hook into. E.g. we can have MainActivity look for extra ReactPackages on the classpath and automatically register them with the react instance (using reflection or annotation processing at build time, or just having rnpm modules specify the FQNs of their package classes in their package.json and using that at generation time). I figure iOS should have something similar.

@grabbou
Copy link
Contributor

grabbou commented Mar 22, 2016

For AndroidManifest.xml / Info.plist we could add a system where rnpm modules can provide custom data to write into those files. They have a pretty standard format so it shouldn't be super hard. We can also let people configure these files via package.json (e.g. adding to AndroidManifest.xml for Google Maps API keys).

We are already working on that with @Kureev so plugins can specify custom config they ask users for (e.g. Google Analytics token) and we will use prompt module to display a nicely formatted interactive question during rnpm link so they can answer and we will update their constructor on android e.g. where you define your package to be new GoogleAnalytics(<read_config_from_android_manifest>) for instance.

MainActivity / AppDelegate - we should just make these super easy to hook into.

That's also in the scope of ^ - we are planning to add some places in these files where plugins can hook with their custom code, e.g. to register for push notifications or to register Sentry (iOS) in e.g. applicationDidFinishLaunching:

ghost pushed a commit that referenced this issue Mar 29, 2016
Summary:The goal is to minimize the number of files we need to bootstrap. This allows us to make the upgrade process smoother for everyone.

If someone needs to customize the file, we already provide some config options. The ability to copy the file and modify it is always there for those few who need it.

**Test plan**

Generate a new project with the updated template.  The app should build and run fine both in debug and production mode.

Related #6292
Closes #6610

Differential Revision: D3109099

Pulled By: foghina

fb-gh-sync-id: 13fc89e60daed30bf6349e532a140c1b6f8f053a
fbshipit-source-id: 13fc89e60daed30bf6349e532a140c1b6f8f053a
@scriptjs
Copy link

scriptjs commented May 19, 2016

I appreciate the discussion about project structure but the proposal is very narrow. Projects structures are what they need to be for a variety of reasons and increasingly due to the fact they must also fit into devops scenarios with continuous integrations and deployment.

In general I think you will find developers don't want:

  1. react-native to alter the way in which node_modules are managed in projects since react native is often combined for simultaneous development for web, platforms and tooling that relies on conventional node module loading.
  2. to wall access with dotted files that would otherwise provide insight into our code.
  3. to abstract away our ability to modify a project in any language.
  4. to impair our ability to examine and get at anything that allows us to perform our work. Keep in mind boilerplates as such are good starting points - but it ends there. Please do not assume everyone needs training wheels or should be guarded from the underlying software, or from developing a clear understanding of the software driving a project (because it has been obfuscated under layers of abstraction).

What I believe we'd like to see generally:

  1. More flexibility in the packager and bundling process to provide for more path configuration to allow for alternate project structures without breaking builds.
  2. Efforts to extract the packager from react native allow for alternate building and bundling scenarios and implementations to compete with the experience we are currently receiving due to the facebook's proprietary modules. Think webpack, browserify, rollup, etc.
  3. Efforts to normalize modules and move away from Haste. It was communicated earlier that there would be efforts to move away from the proprietary haste module system and expose the modules in JavaScript so they could be more easily consumed by any JavaScript developer and systems. I don't see movement in this direction. Rather, there is perpetuation that is working contrary to this. Haste modules are not standard and this is really the elephant in the room that makes the existing packager a necessity.

zebulgar pushed a commit to nightingale/react-native that referenced this issue Jun 18, 2016
Summary:The goal is to minimize the number of files we need to bootstrap. This allows us to make the upgrade process smoother for everyone.

If someone needs to customize the file, we already provide some config options. The ability to copy the file and modify it is always there for those few who need it.

**Test plan**

Generate a new project with the updated template.  The app should build and run fine both in debug and production mode.

Related facebook#6292
Closes facebook#6610

Differential Revision: D3109099

Pulled By: foghina

fb-gh-sync-id: 13fc89e60daed30bf6349e532a140c1b6f8f053a
fbshipit-source-id: 13fc89e60daed30bf6349e532a140c1b6f8f053a
@lacker
Copy link
Contributor

lacker commented Oct 21, 2016

Since this is "for discussion" and the discussion seems to have petered out I am closing this issue.

@lacker lacker closed this as completed Oct 21, 2016
@facebook facebook locked as resolved and limited conversation to collaborators Jul 20, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.
Projects
None yet
Development

No branches or pull requests