csepulv / electron-with-create-react-app

Sample Electron application using create-react-app
MIT License
831 stars 217 forks source link

Use npm start instead of npm run dev #2

Closed vcarl closed 7 years ago

vcarl commented 7 years ago
csepulv commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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