writeToDisk crashes if chalk isn't installed

[bebraw]

• Operating System: n/a
• Node Version: n/a
• NPM Version: n/a
• webpack version: n/a
• webpack-dev-middleware Version: n/a

• This is a feature request
• This is a bug

Code

// please provide your webpack.config.js for bug reports

// additional code

Expected Behavior

Crashy McCrashster

Actual Behavior

For Bugs; How can we reproduce the behavior?

If writeToDisk is enabled, the middleware tries to use chalk. The middleware doesn't specify it in its dependencies, though, and if the project where you are using it doesn't have it installed, it will crash the middleware.

At the very least the middleware should specify a suitable version of chalk as a peer dependency so people know to install it.

For Features; What is the motivation and/or use-case for the feature?

[shellscape]

@bebraw the module depends on webpack-log which depends on chalk, so it should be there for free. You're right in that there should be a peerDep but I'm not sure this can throw an error in the current setup. Could it be another cause?

[bebraw]

@shellscape I can see webpack-log. Since I'm using npm5, it pulls chalk inside webpack-log directory, not node_modules root. Maybe that's why webpack-dev-middleware doesn't find it.

I can likely set up a tiny demo tomorrow.

[shellscape]

Interesting. Something else must be requiring a newer/older version. No need for the demo. I'll remedy this in the package.json and ping you for review.

[davidspiess]

I don't like this change.
Now all users have to manually install a color library as peer dependency, because of one cyan console statement?

log.debug(chalk`{cyan Asset written to disk}: ${relativePath}`);

Can't you just remove chalk altogether or pull it in as normal dependency?

[shellscape]

@davidspiess chalk is already present due to the dependency on webpack-log. Sorry, but that won't be changed. Please feel free to fork and remove that for your needs.

[davidspiess]

@shellscape sorry but i can't understand your reasoning?

Adding it as peer dependency requires all your users to install and update a library, which they don't even know about. If you add it as normal dependency as you did in webpack-log, you solve @bebraw's error, and we user don't need to worry about mantaining chalk in our package.json.

[thomaswelton]

This package imports chalk here

webpack-dev-middleware/lib/fs.js

Line 5 in 8a0bb32

const chalk = require('chalk'); // eslint-disable-line import/no-extraneous-dependencies

This isn't really any different from your other dependencies and shouldn't be a peer dep.
Also the inline es lint rule should not have been added, the linter was warning you about this bug.

[alexander-akait]

@shellscape i agree with above opinions, we should install chalk as deps, not peerDeps. We use library inside code and should install this library.

[bebraw]

I would drop chalk entirely given it doesn't add much value. If someone wants to add some color to the output, they can overwrite logger already. That would solve the concerns above.

[alexander-akait]

@bebraw It's even better, less dependencies - fewer problems

[shellscape]

Resolved in v3.1.2. Please open a new issue if you'd like to continue discussing this, as the original issue as reported by @bebraw has been resolved.