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.

Sign up for GitHub

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

Jump to bottom

feat: add status in quiet log level #2235

Merged
merged 11 commits into from Sep 12, 2019
Merged

feat: add status in quiet log level #2235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e867064
feat: add status log when quiet
rishabh3112 Aug 31, 2019
58d8a9a
chore: update function call
rishabh3112 Aug 31, 2019
75dc99e
chore: add comment for the behaviour
rishabh3112 Aug 31, 2019
3119a12
chore: fix lint
rishabh3112 Aug 31, 2019
80a7fa4
chore: update level
rishabh3112 Aug 31, 2019
adc8466
Merge branch 'master' into feat/log-silent
rishabh3112 Sep 3, 2019
6c60d8b
test: add quiet flag test
rishabh3112 Sep 4, 2019
4775e52
Merge branch 'feat/log-silent' of https://github.com/rishabh3112/webp…
rishabh3112 Sep 4, 2019
bd3e2c2
chore: lint
rishabh3112 Sep 4, 2019
517d5e9
Merge branch 'master' into feat/log-silent
rishabh3112 Sep 6, 2019
f802b18
Merge branch 'master' into feat/log-silent
evilebottnawi Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/utils/status.js
View file
Open in desktop
@@ -1,10 +1,20 @@
'use strict';

const logger = require('webpack-log');
const colors = require('./colors');
const runOpen = require('./runOpen');

// TODO: don't emit logs when webpack-dev-server is used via Node.js API
function status(uri, options, log, useColor) {
if (options.quiet === true) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean that there is now no way to silence these extra logs when using in node api?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this log should be outputted always, in future we will use webpack logger and you can filter this output

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be great. this seems like a regression IMO. we now have an extra set of console output. if possible when the webpack logger option is available would be great to link that to this issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use custom logger as workaround

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link to documentation? not sure what you mean.

Copy link

@endiliey endiliey Oct 15, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evilebottnawi

I am facing this as well. I use webpackbar and this extra console log noise actually make it weird because webpackbar tries to clear screen when showing the fancy bar

After this PR
sad

Before
good

Can we add an option to hide this output ? I'm sure many packages that use webpackbar is affected by this unknowingly :(

cc my team @yangshun

Update: We have adjusted to wds so whether to hide the output/not no longer matter to us 😂

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty pretty please? this output decreases dev experience IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to send a PR with new option

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reverts #1486 and I believe it to now be a breaking change.

this log should be outputted always

Sorry, strongly disagree. What's the point of the option if it doesn't do what it says? I understand the custom logger is just a documentation issue right now, but it still points to quiet (and noInfo) being pointless/improperly named

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You actually can't fix this with a custom logger, because this setup noise is printed by utils/status.js directly, which is where this log override is happening. There's no intermediary step

// Add temporary logger to output just the status of the dev server
log = logger({
rishabh3112 marked this conversation as resolved.
Show resolved Hide resolved
name: 'wds',
level: 'info',
timestamp: options.logTime,
});
}

const contentBase = Array.isArray(options.contentBase)
? options.contentBase.join(', ')
: options.contentBase;
Expand Down
16 changes: 16 additions & 0 deletions test/cli/cli.test.js
View file
Open in desktop
Expand Up @@ -29,6 +29,22 @@ describe('CLI', () => {
.catch(done);
});

it('--quiet', async (done) => {
const output = await testBin('--quiet');
expect(output.code).toEqual(0);
expect(output.stdout.split('\n').length === 3).toBe(true);
expect(
output.stdout.includes('Project is running at http://localhost:8080/')
rishabh3112 marked this conversation as resolved.
Show resolved Hide resolved
).toBe(true);
expect(output.stdout.includes('webpack output is served from /')).toBe(
true
);
expect(
output.stdout.includes('Content not from webpack is served from')
).toBe(true);
done();
});

it('--progress --profile', (done) => {
testBin('--progress --profile')
.then((output) => {
Expand Down