drapanjanas / re-natal

Bootstrap ClojureScript React Native apps
MIT License
1.22k stars 100 forks source link

Template runs iOS and Android code on startup #133

Closed jeaye closed 7 years ago

jeaye commented 7 years ago

As defined by the :source-paths here: https://github.com/drapanjanas/re-natal/blob/master/resources/project.clj#L20 there is an issue where prod builds will run env.android.prod and env.ios.prod on startup. Similarly, dev builds will run env.android.dev and env.ios.dev on startup. This is because both Android and iOS sources are grouped under env/dev or env/prod.

Here's an example, as per the README:

┌─(✓)[jeaye@oryx]─[~/projects/tmp][18:02:39]
└──╼ re-natal init FutureApp
Creating FutureApp
☕  Grab a coffee! Downloading deps might take a while...
Creating Leiningen project
Updating Leiningen project
Creating React Native skeleton.
Creating Re-Natal config
Compiling ClojureScript

To get started with your new app, first cd into its directory:
cd future-app

Run iOS app:
react-native run-ios > /dev/null

To use figwheel type:
re-natal use-figwheel
lein figwheel ios

Reload the app in simulator (⌘ + R)

At the REPL prompt type this:
(in-ns 'future-app.ios.core)

Changes you make via the REPL or by changing your .cljs files should appear live.

Try this command as an example:
(dispatch [:set-greeting "Hello Native World!"])

✔ Done

┌─(✓)[jeaye@oryx]─[~/projects/tmp][18:03:56]
└──╼ cd future-app/

┌─(✓)[jeaye@oryx]─[~/projects/tmp/future-app][18:04:02]
└──╼ tree env/
env/
├── dev
│   ├── env
│   │   ├── android
│   │   │   └── main.cljs
│   │   └── ios
│   │       └── main.cljs
│   └── user.clj
└── prod
    └── env
        ├── android
        │   └── main.cljs
        └── ios
            └── main.cljs

8 directories, 5 files
jeaye commented 7 years ago

Unfortunately, after refactoring to work around this issue, I've run into a design problem deeper than just the template: .re-natal also assumes the sources are laid out this way.

Give an adapted structure like this (following typical Leiningen profile practices):

$ env/
├── android
│   ├── dev
│   │   └── env
│   │       └── android
│   │           └── main.cljs
│   └── prod
│       └── env
│           └── android
│               └── main.cljs
├── dev
│   └── user.clj
└── ios
    ├── dev
    │   └── env
    │       └── ios
    │           └── main.cljs
    └── prod
        └── env
            └── ios
                └── main.cljs

15 directories, 5 files

When I try to run my normal build cycle:

$ re-natal use-figwheel
Cleaning...
index.ios.js was regenerated
index.android.js was regenerated
Host in RCTWebSocketExecutor.m was updated
ENOENT: no such file or directory, open 'env/dev/env/ios/main.cljs'

So it looks like something more fundamental will need to change for this. How people are running non-trivial apps with custom initialization per-platform, or without double initializing each startup is still unclear. It seems to me that everyone is affected.

drapanjanas commented 7 years ago

True, unfortunately at the moment project structure is not very flexible. Some of of functions of re-natal needs to access env/dev env/prod files. Maybe it is possoble to refactor and introduce more fine-grained configuration for the paths in .re-natal. Have no good ideas at the moment.

jeaye commented 7 years ago

Forcing this layout is not the problem; it's a reasonable layout that separates platform-dependent code from the shared code under src/. The only issue is that the layout is forced, but the code isn't actually separated. Android is still going to run the iOS code at startup, and vice versa. That's a big issue, which is much more important than allowing the project layout to be flexible (since it means most people are initializing their apps twice and it also means there's not actually a way to have platform-dependent code using re-natal).

I'm hoping you'll address this, since it's the focus of this ticket. Me trying to rework the layout was just an attempted fix.

carocad commented 7 years ago

@jeaye not sure if I understand this correctly but did you prove that both iOS and Android apps are running on startup?

AFAIK they are both being compiled but only the selected target is run due to the use of :main "env.android.main". Everything else should be strip away by the compiler since it is not reachable from the main namespace.

jeaye commented 7 years ago

Yeah, I proved it by commenting out the iOS code that was running on startup and seeing my double initialization go away. The same should be possible by following the barest re-natal template, which I can do to further minimize the problem space.

jeaye commented 7 years ago

After having tried this on a bare template, I'm able to reproduce the problem in production builds only IFF I make some changes to the project.clj. As it is, I'm wrong that the re-natal template is broken out of the box. Alas, it seems to rely on some behavior which the ClojureScript docs say doesn't exists, so I think we need to tread very carefully.

@carocad's reply was very helpful, reminding me that :main is used to determine the entry point. Guess what happens when :main does exist for production builds with re-natal: both iOS and Android mains get executed. So, the simple fix is to add :main back into our production builds. Why was it removed?

  1. The generated project.clj has quite a bit of error-prone duplication, so we refactored it, following the cljsbuild docs and ClojureScript docs, so that the common bits are shared
  2. The ClojureScript docs imply that :main isn't used when optimizations are enabled, so it was removed from the production profiles

I think, going forward, re-natal could likely verify that a :main exists, even in production, if it's requiring the key in order to behave as expected.

Thanks for prompting me to look deeper, @carocad.