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

Noisy output in compress.js #33

Open
PAkerstrand opened this issue Mar 5, 2019 · 4 comments · May be fixed by #36
Open

Noisy output in compress.js #33

PAkerstrand opened this issue Mar 5, 2019 · 4 comments · May be fixed by #36

Comments

@PAkerstrand
Copy link

The adapter()-function is very noisy when it comes to outputting what it does. It might make sense to use something other then console.log in order to notify the user about what's going on. One approach would be to use the debug-package, which lets the user decide whether he wants to see the warning or keep it suppressed: https://www.npmjs.com/package/debug

1____project_spt_hyperion__bash_

function adapter() {
  // ...
    try {
        console.log('warning: couldn\'t find native brotli support in zlib library. trying to fall back to iltorb.');
        var iltorb = require('iltorb');
        return iltorb.compress;
    } catch (err) {
        console.log('warning: couldn\'t load iltorb library. trying to fall back to brotli.js.');
      // ...
    }
}
+ const debug = require('debug');
+ const warn = debug('brotli-webpack-plugin:warning');
+ const err = debug('brotli-webpack-plugin:error');

- console.log('warning: couldn\'t find native brotli support in zlib library. trying to fall back to iltorb.');
+ warn('couldn\'t find native brotli support in zlib library. trying to fall back to iltorb.');
@ViktorZhurbin
Copy link

ViktorZhurbin commented Mar 13, 2019

Seems to be related:

Using plugin version 1.1.0 and Nodejs 10 LTS, run webpack --profile --json > stats.json (webpack 4.29.6). A warning is injected before the actual json starts:

warning: couldn't find native brotli support in zlib library. trying to fall back to iltorb.

{
  "errors": [],
  "warnings": []
   ...
}

Had to remove the line manually before I could run webpack-bundle-analyzer.
Switched to v 1.0.0 and it's fine now.

@keeganstreet
Copy link

Webpack loaders and plugins should use this.emitWarning, not console.log or other npm packages to log warnings. The current version of brotli-webpack-plugin is using console.log which is polluting stdout.

@gex
Copy link

gex commented May 8, 2019

the console.logs also break the webpack-bundle-analyzer plugin when it's used in cli mode.

nh2 added a commit to nh2/brotli-webpack-plugin that referenced this issue Jun 28, 2019
nh2 added a commit to nh2/brotli-webpack-plugin that referenced this issue Jun 28, 2019
nh2 added a commit to nh2/brotli-webpack-plugin that referenced this issue Jun 28, 2019
@nh2 nh2 linked a pull request Jun 28, 2019 that will close this issue
@nh2
Copy link

nh2 commented Jun 28, 2019

Webpack loaders and plugins should use this.emitWarning, not console.log

I went for console.warn instead in PR #36 (which goes to stderr), because the this isn't in scope where the logs are done, and (I think) also can't be passed in because that's available only here but we need it here.

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 a pull request may close this issue.

5 participants