Adds Option To Generate ReactNative Example project #57

thorbenprimke · 2018-02-28T07:05:42Z

Adds the option --generate-example that generate an example project and links the newly created to it.

This addresses #28.

Due to react-native init project_name not executing in a folder that already contains a package.json file, the example is generated first, then all the files are created and last the library is linked to the example.

frostney

Amazing work 🎉
I really love this feature, I just have a few small questions.

frostney · 2018-03-12T21:47:30Z

lib.js

+      // Adds and links the created library project
+      const pathExampleApp = './' + name + '/example';
+      const options = { cwd: pathExampleApp, stdio:'inherit'};
+      execSync('yarn add file:../', options);


Would it be possible to check first if yarn is available here?

Sure. I guess fallback would be npm or shall it just throw an error?

I think npm you may not be able to link locally (or I could be totally wrong - maybe it was the point at a commit that didn't work with npm).

I added a try/catch around yarn and added fallback support for npm.

frostney · 2018-03-12T21:49:44Z

lib.js

+        return;
+      }
+      // Adds and links the created library project
+      const pathExampleApp = './' + name + '/example';


Nitpick: What do you think about using template literals here? (It's just a personal preference on my part.)

Sure. Will look at updating this.

updated to use template literals.

frostney · 2018-03-12T21:52:14Z

templates/android.js


    dependencies {
-        classpath 'com.android.tools.build:gradle:1.3.1'
+        classpath 'com.android.tools.build:gradle:2.2.3'


I'm a bit concerned that this change doesn't have a lot of to do with the rest of pull request. Am I missing something or could this be delivered separately?

yes, resolved this in #59

… not avaialble

thorbenprimke · 2018-03-17T06:02:34Z

@maicki - could you give this a review? It's good to go.

maicki · 2018-03-17T16:56:18Z

lib.js

-      authorEmail,
-      license,
-    };
+  return Promise


I think we don't need the Promise.resolve here, just return the createFolder

Furthermore I think in all of the early returns for the !generateExample checks we should return a Promise.reject if we consume the promise that is returned from this funciton or a Promise.resolve in case we see it as success.

Yea, that's a good improvement. I move the createFolder out.

thorbenprimke

I'll make these change and then merge it.

thorbenprimke · 2018-03-18T17:55:58Z

lib.js

-      authorEmail,
-      license,
-    };
+  return Promise


Yea, that's a good improvement. I move the createFolder out.

thorbenprimke · 2018-03-18T17:56:19Z

lib.js

+    .resolve()
+    .then(() => {
+      if (!generateExample) {
+        return


Yes, this should be a Promise.resolve()

thorbenprimke · 2018-03-18T17:56:31Z

lib.js

+    })
+    .then(() => {
+      if (!generateExample) {
+        return;


same here Promise.resolve()

thorbenprimke · 2018-03-18T17:59:30Z

Verified that both with and without generate parameter it works as expected.

pvh · 2018-03-20T18:47:16Z

This is a really great feature, but on a totally fresh install it causes Metro to get terribly confused.

I see an "Ambiguous Module Resolution Error" caused by metro picking up both the package.json in the example/package.json directory but ALSO the same package.json a second time when it's installed in node_modules/$MODULE_NAME/example!

I've worked around this by moving the example app out of the tree, but I'm not really sure what a good way to solve this would be at the tool level.

thorbenprimke · 2018-03-20T19:46:40Z

@pvh - Hmm. I did not run into this and we always have an example folder inside a lib project (for testing purposes and to provide usage examples of a lib).

What are the commands that you are running?

Perhaps we could make the path for the example app configurable. The reason it's inside the project is because one would ideally push both the lib and example to a single repo on github.

maxme · 2018-03-23T14:45:25Z

This is a really great feature, but on a totally fresh install it causes Metro to get terribly confused.

I see an "Ambiguous Module Resolution Error" caused by metro picking up both the package.json in the example/package.json directory but ALSO the same package.json a second time when it's installed in node_modules/$MODULE_NAME/example!

I get the same problem, steps to reproduce the issue:

$ react-native-create-library --generate-example mytest
$ cd mytest/
$ yarn install
$ cd example
$ react-native run-android

Metro bundler fails with the error:

Loading dependency graph, done.
error: bundling failed: ambiguous resolution: module `/private/tmp/mytest/example/index.js` tries to require `react-native`, but there are several files providing this module. You can delete or fix them:

  * `/private/tmp/mytest/example/node_modules/react-native-mytest/example/node_modules/react-native/package.json`
  * `/private/tmp/mytest/example/node_modules/react-native/package.json`

maxme · 2018-03-23T15:56:11Z

I tried to add a metro bundler configuration file to exclude the directory mytest/example/node_modules/ from the resolution map but I ran into other issues after that. I finally moved the library in its own directory and now I have something like:

.
├── example
│   ├── android
│   ├── ios
│   ├── node_modules
│   ├── App.js
│   ├── app.json
│   ├── index.js
│   └── package.json
├── library
│   ├── android
│   ├── ios
│   ├── node_modules
│   ├── windows
│   ├── RNMytest.podspec
│   ├── index.js
│   └── package.json
└── README.md

thorbenprimke · 2018-03-23T16:41:00Z

@maxme - thanks for the steps. I'll try to repro this later.

I see where my workflow differs. Instead of your last command react-native run-android, I would normally just do

yarn install
yarn start

which theoretically should do the same.

And I built out of AS or XCode.

maxme · 2018-03-23T17:02:33Z

FWIW, I also tried:

react-native-create-library --generate-example mytest \
 && cd mytest/ \
 && yarn install \
 && cd example \
 && pushd android \
 && ./gradlew installDebug \
 && popd \
 && adb shell am start -n com.example/.MainActivity \
 && yarn start

And it fails with the same error.

I'm far from being an RN veteran so I'm not sure what's the best way to setup the library and example directories. Having the example directory as a library sub directory seems to be a source of issues.

iamcxa · 2018-05-21T15:14:06Z

that's great! it will save lot's time when making a native module at the beginning. when will this merged version will be sent to npm? because now still has no this feature in npm version yet.

frostney reviewed Mar 12, 2018

View reviewed changes

thorbenprimke added 2 commits March 16, 2018 22:15

Adds Option To Generate ReactNative Example project

dd54b47

updated readme

e19a7ba

thorbenprimke force-pushed the generate_example branch from fd7ab4f to e19a7ba Compare March 17, 2018 05:15

Changed path to use template, Added try/catch to use npm when yarn is…

12cfde4

… not avaialble

maicki approved these changes Mar 17, 2018

View reviewed changes

thorbenprimke commented Mar 18, 2018

View reviewed changes

Moved createFolder out, changed empty return to Promise.resolve()

45b4b4a

thorbenprimke merged commit 2b04cfc into frostney:master Mar 18, 2018

thorbenprimke deleted the generate_example branch March 18, 2018 17:59

maicki mentioned this pull request Jun 11, 2018

Add example project #28

Closed

Adds Option To Generate ReactNative Example project #57

Adds Option To Generate ReactNative Example project #57

Conversation

thorbenprimke commented Feb 28, 2018

Uh oh!

frostney left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorbenprimke commented Mar 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorbenprimke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thorbenprimke commented Mar 18, 2018

Uh oh!

pvh commented Mar 20, 2018

Uh oh!

thorbenprimke commented Mar 20, 2018

Uh oh!

maxme commented Mar 23, 2018

Uh oh!

maxme commented Mar 23, 2018

Uh oh!

thorbenprimke commented Mar 23, 2018

Uh oh!

maxme commented Mar 23, 2018

Uh oh!

iamcxa commented May 21, 2018

Uh oh!

Uh oh!