-
Notifications
You must be signed in to change notification settings - Fork 113
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
Improved the browser build #73
Conversation
Made three improvements to the browser build: 1. It now builds _two_ browser bundles, one minified (49kb) and one unminified (129kb) 2. It now builds source maps, which makes debugging in a browser easier 3. `npm start` now runs with `--watch`, so it automatically does _fast_ differential rebuilds when any source files change
"build:readme": "rm -rf README.md && node ./support/readme", | ||
"start": "npm run build && http-server -c-1", | ||
"start": "npm run build -- --watch & http-server -o -c-1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will background the build process which could be undesirable. It might be better to use something like npm-run-all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The build process is being run with --watch
, so it'll be a long-running process, hence the need to run it in the background (via the single &
there). I can switch it to use npm-run-all --parallel
instead. AFAIK, that's essentially the same thing, except that it also works on Windows. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&
will put the process in the background, which means Ctrl+C will only kill the http-server
bit, also if npm run build
dies you won't really hear about it. npm-run-all
will kill all processes if one dies (which is a good thing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, ok. Very good point. 👍
I'll make this change today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, add a comment when you do please - as github doesn't notify for commits on PRs sadly 😞
Added a new commit to use |
Made three improvements to the browser build:
npm start
now runs with--watch
, so it automatically does fast differential rebuilds when any source files change