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

Use npm start instead of npm run dev #2

Closed
wants to merge 1 commit into from
Closed

Use npm start instead of npm run dev #2

wants to merge 1 commit into from

Conversation

vcarl
Copy link

@vcarl vcarl commented Feb 2, 2017

  • Change npm start's behavior to be run by npm run react-start.
  • Add a new npm script, npm run electron-start
  • Change Procfile to run those two npm scripts, so all script behavior is defined within package.json.
  • Change npm run dev's behavior to be run by npm start, to be a little more consistent with other projects. Running npm install and npm start will start up the project for development.

@csepulv
Copy link
Owner

csepulv commented Feb 2, 2017

Thanks for your interest in the project. Can you explain a little more about what you want to achieve with the changes? Since this repo is part of a Medium post, I was a little surprised by the pull request.

@vcarl
Copy link
Author

vcarl commented Feb 2, 2017

I followed the Medium post (thanks for putting it together!) but I made this change for myself while going along.

It's "best practice" for npm start to start up a project for development, and especially given that the current npm start will run the CRA build, but won't spin up Electron, I thought it would be helpful to others if I contributed it back.

Basically the current post-clone process is

npm install
npm run dev

and this PR changes it to

npm install
npm start

@vcarl
Copy link
Author

vcarl commented Feb 2, 2017

Sorry, I should have opened this PR better :)

I found your Medium post incredibly helpful for getting up and going with Electron and CRA, but while following along I wanted to use npm start instead of npm run dev, if for no other reason that it's what I use on my other projects. Every time I use something else like dev or watch I end up forgetting what I named it and having to look at package.json to remind myself.

It also occurs to me that because there's an npm start defined, when I inevitably forget what the correct script is, I'll end up trying npm start by default and only getting half of the build running.

While doing that change, I figured for the sake of symmetry and doing things in one place, I'd add another npm script for starting electron. That way the Procfile is only referencing npm scripts, and all the only place commands are defined is in package.json.

Nothing revolutionary, just some small tweaks I made for my own sanity that I thought others might find useful!

@csepulv
Copy link
Owner

csepulv commented Feb 2, 2017

That makes sense. I had debated about changing npm start; I left it to minimize the changes from create-react-app defaults.

I'd like to minimize changes to the existing article, so I will create a branch for your changes. I'll update the README in the repo and add a note to the end of the article about the branch.

@csepulv
Copy link
Owner

csepulv commented Feb 2, 2017

I've committed the branch, updated the README and added a note to the end of the article.

@csepulv csepulv closed this Feb 2, 2017
Copy link

@p3nGu1nZz p3nGu1nZz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm. much cleaner then npm run dev. +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants