Run application using package.json scripts #12004

Kureev · 2017-01-20T13:34:21Z

Motivation:
According to the meeting notes published by @mkonicek yesterday, RN should support yarn run ios and yarn run android commands.

Test plan (required)

Generate a new project and start scaffolded app using yarn(npm) run ios(android)

…e.json scripts

mkonicek · 2017-01-20T13:44:02Z

Awesome, thank you! 👍

mkonicek · 2017-01-20T13:44:33Z

@facebook-github-bot shipit

facebook-github-bot · 2017-01-20T13:45:14Z

@mkonicek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

grabbou · 2017-01-22T09:28:32Z

Why this should be supported? Is there any benefit of adding arbitrary commands to users package.json, like ios and android if there's already react-native run-ios available? What's the benefit?

Ah, now I see (after reading Slack) that this is to make it compatible after ejecting from crna. But, wouldn't it make more sense to make crna add these commands during eject phase instead? All in all, we want ejected app to be consistent, not the fresh one.

grabbou · 2017-01-22T09:30:04Z

react-native-cli/index.js

@@ -240,7 +240,9 @@ function createProject(name, options) {
    version: '0.0.1',
    private: true,
    scripts: {
-      start: 'node node_modules/react-native/local-cli/cli.js start'
+      start: 'node node_modules/react-native/local-cli/cli.js start',
+      ios: 'react-native run-ios',


Are you sure this is actually going to work? In this context, react-native points to the executable in node_modules/.bin/react-native, which is this: https://github.com/facebook/react-native/blob/master/package.json#L114 and is used to warn users that react-native cannot be installed globally. There's imo no way to refer to react-native-cli from npm scripts context as far as I remember.

If you want to refer to the local-cli, access it like in start.

I added these ios and android scripts to the scaffolded application and tried both of them - everything works.

Maybe that is because you have the cli installed globally? What @grabbou is saying makes sense

Maybe there was no react native in node modules and the script referred to local cli that found React Native up the path?

*referred to global cli

ericvicenti · 2017-01-22T13:35:25Z

Ideally I would like to see all of the npm-local commands to have complete parity with the commands in react native local-cli. Then ideally we would release a new global react-native-cli that simply defers to the npm run behavior, and only falls back directly on the local cli when the script is not set up in the package.

The main advantage I can think of with this behavior is the ability for people to customize the actual implementation of start and run for their project, by adding flags or pointing to another command. Also people can add commands in their projects locally, and still have a similar interface. For example, windows users could enable react-native run-windows for their projects.

Kureev · 2017-01-23T08:42:07Z

Thanks for your feedback, guys!

Yeah, I have react-native-cli installed globally, but that's according to the docs. Am I wrong / What do I miss here?

grabbou · 2017-01-23T09:55:35Z

@ericvicenti

people can add commands in their projects locally, and still have a similar interface. For example, windows users could enable react-native run-windows for their projects.

This is already possible thanks to plugin system that ships out of the box (not yet documented), but things like create-react-native-app could benefit from it by adding own commands, see: https://github.com/ReactWindows/react-native-windows/blob/master/package.json#L45. That adds run-windows when react-native-windows is in project dependencies. I am aware that rnpm object on package.json is a wrong thing and I recently shipped a unified interface that removes that (now, all commands are defined inside rn-cli.config.js)

is the ability for people to customize the actual implementation of start and run for their project, by adding flags or pointing to another command

The above plugin system also allows you to override predefined commands and enhance their functionality by doing any kind of custom functionality you want. I am currently working on shipping that feature which is going to be used in Exponent mainly.

All of these are currently possible with the cli implementation that we have, but I am aware that it's because of lack of proper documentation that some of these are not that obvious at first sight.

Ideally I would like to see all of the npm-local commands to have complete parity with the commands in react native local-cli. Then ideally we would release a new global react-native-cli that simply defers to the npm run behavior, and only falls back directly on the local cli when the script is not set up in the package.

That's interesting idea, however it makes it complicated to add new commands or modify the existing pre-defined functionalities. I think given that all the benefits of doing so can be already achieved with the current implementation, there's no need in changing the way it's structured at this point.

ericvicenti · 2017-01-23T18:14:58Z

The reason I prefer to use the npm scripts is because it is highly standard across the community. This is how create-react-app works, which has become insanely popular. I don't see a good reason for rn-cli.config.js to declare local scripts, when there is already a standard way to handle local scripts.

however it makes it complicated to add new commands or modify the existing pre-defined functionalities

I don't see how it is complicated. If react-native-cli calls the RN's local-cli as a fallback when the npm script is not defined, then it works in exactly the same way as it does today, right?

grabbou · 2017-01-23T21:01:00Z

In that case I think it makes sense, given that it uses a fallback to local cli (missed that point). Makes sense now. I'll make sure to follow up on that one tomorrow as it's related to my current work in that area

Summary: **Motivation:** According to the meeting notes published by mkonicek yesterday, RN should support `yarn run ios` and `yarn run android` commands. **Test plan (required)** - [x] Generate a new project and start scaffolded app using `yarn(npm) run ios(android)` Closes facebook#12004 Differential Revision: D4441837 Pulled By: mkonicek fbshipit-source-id: 250f7a9e1efc59e0caa5c2c071b59b97e14e939b

Summary: **Motivation:** According to the meeting notes published by mkonicek yesterday, RN should support `yarn run ios` and `yarn run android` commands. **Test plan (required)** - [x] Generate a new project and start scaffolded app using `yarn(npm) run ios(android)` Closes facebook/react-native#12004 Differential Revision: D4441837 Pulled By: mkonicek fbshipit-source-id: 250f7a9e1efc59e0caa5c2c071b59b97e14e939b

Add ios/android commands to make it possible to run apps using packag…

ff249aa

…e.json scripts

Kureev requested a review from mkonicek January 20, 2017 13:34

Kureev self-assigned this Jan 20, 2017

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. GH Review: review-needed labels Jan 20, 2017

mkonicek approved these changes Jan 20, 2017

View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jan 20, 2017

facebook-github-bot closed this in bc8b23a Jan 20, 2017

grabbou reviewed Jan 22, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run application using package.json scripts #12004

Run application using package.json scripts #12004

Kureev commented Jan 20, 2017

mkonicek commented Jan 20, 2017

mkonicek commented Jan 20, 2017

facebook-github-bot commented Jan 20, 2017

grabbou commented Jan 22, 2017 •

edited

Loading

grabbou Jan 22, 2017

Kureev Jan 22, 2017

ericvicenti Jan 22, 2017

grabbou Jan 23, 2017

grabbou Jan 23, 2017

ericvicenti commented Jan 22, 2017

Kureev commented Jan 23, 2017 •

edited

Loading

grabbou commented Jan 23, 2017 •

edited

Loading

ericvicenti commented Jan 23, 2017

grabbou commented Jan 23, 2017

Run application using package.json scripts #12004

Run application using package.json scripts #12004

Conversation

Kureev commented Jan 20, 2017

mkonicek commented Jan 20, 2017

mkonicek commented Jan 20, 2017

facebook-github-bot commented Jan 20, 2017

grabbou commented Jan 22, 2017 • edited Loading

grabbou Jan 22, 2017

Choose a reason for hiding this comment

Kureev Jan 22, 2017

Choose a reason for hiding this comment

ericvicenti Jan 22, 2017

Choose a reason for hiding this comment

grabbou Jan 23, 2017

Choose a reason for hiding this comment

grabbou Jan 23, 2017

Choose a reason for hiding this comment

ericvicenti commented Jan 22, 2017

Kureev commented Jan 23, 2017 • edited Loading

grabbou commented Jan 23, 2017 • edited Loading

ericvicenti commented Jan 23, 2017

grabbou commented Jan 23, 2017

grabbou commented Jan 22, 2017 •

edited

Loading

Kureev commented Jan 23, 2017 •

edited

Loading

grabbou commented Jan 23, 2017 •

edited

Loading