Add BuildProgressPlugin #1011
Add BuildProgressPlugin #1011
Conversation
Are you saying that I should remove the peer dependency of webpack and trust that it is there at runtime? As of now, I used the semver matching from 4cd9fd9. |
Yes. (It's not as bad as it sounds, this plugin is useless without webpack anyway.) |
| @@ -257,7 +258,8 @@ module.exports = { | |||
| // having to parse `index.html`. | |||
| new ManifestPlugin({ | |||
| fileName: 'asset-manifest.json' | |||
| }) | |||
| }), | |||
| new BuildProgressPlugin() | |||
There was a problem hiding this comment.
Can we add a small comment here explaining what it does. "Displays a progress bar during the build" is a fine way to put it.
|
(It's really nice btw, great work) |
Timer
force-pushed
the
build-progress
branch
4 times, most recently
from
cf4b001 to
e4085c2
Compare
|
Removed the peer dep, is this ok behavior though for when someone installs this for their own use? |
|
|
||
| let ProgressPlugin; | ||
| if (__dirname.indexOf(path.join('packages', 'react-dev-utils')) !== -1) { | ||
| ProgressPlugin = require('../react-scripts/node_modules/webpack').ProgressPlugin; |
There was a problem hiding this comment.
Why do we need this? Seems fishy.
There was a problem hiding this comment.
Webpack doesn't exist in any parent directory during development, only a sister directory (which node's module resolution doesn't traverse).
For development in the monorepo to work, we need a hack. I chose to do it this way, though we could accomplish it however.
tl;dr without it: Error: Cannot find module 'webpack'
There was a problem hiding this comment.
Can we add it to devDependencies?
Yes it’s fine. Peer deps are often more trouble than they’re worth, and until Webpack 2 is out of beta, they are definitely a pain since semver authors refuse to have a way to refer to non-stable versions via a range. |
|
Ping, would you like to make the requested changes so we can get it in? |
|
Sure |
Timer
force-pushed
the
build-progress
branch
2 times, most recently
from
68e1eb7 to
a9e6465
Compare
|
@gaearon pending CI, should be good -- though I'm not sure if I'm happy with the quality of progress the Maybe we should shut off the progress bar when not in an interactive terminal? |
👍 |
|
@gaearon turns out travis is |
|
|
||
| function BuildProgressPlugin() { | ||
| if (process.env.CI) return new ProgressPlugin(function handler(percentage, msg) { | ||
| // noop |
There was a problem hiding this comment.
Style nit: please use curly form of If statement.
| width: 25 | ||
| }); | ||
| return new ProgressPlugin(function handler(percent, msg) { | ||
| if (percent === 1) msg = 'completed'; |
There was a problem hiding this comment.
Same here, please always use multiline if.
|
Let's hold off on this until 0.10.0 and hope webpack's progress reporting got better. If not we can adjust it to just show current step / steps remaining. |
|
Closing as stale. Maybe worth revisiting if the tracking is accurate. |
Reincarnation of #950. Please see it for details.