Skip to content

feat: Less verbose error output #206

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

Merged
reznord merged 2 commits into from
Jul 10, 2017
Merged

feat: Less verbose error output #206

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3df9328
feat(build, watch): Less verbose error output
rkostrzewski Jul 8, 2017
031d6cb
Clear console before throwing error stack trace in terminal
reznord Jul 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions src/lib/webpack/run-webpack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ const devBuild = async (env, onprogress) => {
let serverAddr = `${protocol}://${host}:${chalk.bold(port)}`;
let localIpAddr = `${protocol}://${ip.address()}:${chalk.bold(port)}`;

process.stdout.write(chalk.green('\nCompiled successfully!!\n\n'));
if (stats.hasErrors()) {
process.stdout.write(chalk.red('\Build failed!\n\n'));
} else {
process.stdout.write(chalk.green('\nCompiled successfully!\n\n'));
}
Copy link
Member

@reznord reznord Jul 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Better to expose the IP addresses & port message only when the build is successful !!

    if (userPort !== port) {
       process.stdout.write(`Port ${chalk.bold(userPort)} is in use, using ${chalk.bold(port)} instead\n\n`);
    }
    process.stdout.write('You can view the application in browser.\n\n');
    process.stdout.write(`${chalk.bold('Local:')}            ${serverAddr}\n`);
    process.stdout.write(`${chalk.bold('On Your Network:')}  ${localIpAddr}\n`);

otherwise LGTM !!

Copy link
Collaborator Author

@rkostrzewski rkostrzewski Jul 8, 2017

Choose a reason for hiding this comment

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

That could be possible but I don't know really. webpack-dev-server shows address always on first compilation.

We kinda don't inform users after they fixed the errors if anything occurred. Maybe the output should look like this:

You can view the application in browser.

   Local: ...
   On Your Network: ...

Build failed!
--Some error message--
Compiled successfully --date stamp--
Compiled successfully --date stamp--
Build failed!
--Some error message--
Compiled successfully --date stamp--

Let me know what you think (also @lukeed @developit @thangngoc89 )

Copy link
Collaborator

@thangngoc89 thangngoc89 Jul 8, 2017

Choose a reason for hiding this comment

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

Only show live URL on first render is more than enough for me. And I really hate any watcher that clear my terminal output

Copy link
Member

@developit developit Jul 8, 2017

Choose a reason for hiding this comment

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

I'm good with that format. I believe the "compiled successfully" bit can be done as the completion message of ProgressBarPlugin if it isn't already.

Copy link
Collaborator Author

@rkostrzewski rkostrzewski Jul 8, 2017

Choose a reason for hiding this comment

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

It is now kinda different with @reznord's changes. Basically we would now show address only for successful builds and clear console after each build (IMHO that's much nicer).


if (userPort !== port) {
process.stdout.write(`Port ${chalk.bold(userPort)} is in use, using ${chalk.bold(port)} instead\n\n`);
}
Expand Down Expand Up @@ -71,7 +76,8 @@ const prodBuild = async (env) => {
return await new Promise((resolve, reject) => {
compiler.run((err, stats) => {
if (err || stats.hasErrors()) {
reject(err || stats.toJson().errors.join('\n'));
showStats(stats);
reject(chalk.red('Build failed!'));
}
else {
// Timeout for plugins that work on `after-emit` event of webpack
Expand All @@ -82,16 +88,16 @@ const prodBuild = async (env) => {
};

export function showStats(stats) {
let info = stats.toJson();
let info = stats.toJson("errors-only");

if (stats.hasErrors()) {
info.errors.forEach( message => {
process.stderr.write(message+'\n');
info.errors.map(stripBabelLoaderPrefix).forEach( message => {
process.stderr.write(chalk.red(message)+'\n');
});
}

if (stats.hasWarnings()) {
info.warnings.forEach( message => {
info.warnings.map(stripBabelLoaderPrefix).forEach( message => {
process.stderr.write(chalk.yellow(message)+'\n');
});
}
Expand All @@ -109,8 +115,8 @@ export function writeJsonStats(stats) {

jsonStats = (jsonStats.children && jsonStats.children[0]) || jsonStats;

jsonStats.modules.forEach(normalizeModule);
jsonStats.chunks.forEach(c => c.modules.forEach(normalizeModule));
jsonStats.modules.forEach(stripBabelLoaderFromModuleNames);
jsonStats.chunks.forEach(c => c.modules.forEach(stripBabelLoaderFromModuleNames));

return fs.writeFile(outputPath, JSON.stringify(jsonStats))
.then(() => {
Expand All @@ -121,20 +127,20 @@ export function writeJsonStats(stats) {
});
}

const normalizeModule = m => {
const stripBabelLoaderFromModuleNames = m => {
const keysToNormalize = ['identifier', 'name', 'module', 'moduleName', 'moduleIdentifier'];

keysToNormalize.forEach(key => {
if (key in m) {
m[key] = normalizeName(m[key]);
m[key] = stripBabelLoaderPrefix(m[key]);
}
});

if (m.reasons) {
m.reasons.forEach(normalizeModule);
m.reasons.forEach(stripBabelLoaderFromModuleNames);
}

return m;
};

const normalizeName = p => p.substr(p.lastIndexOf('!') + 1);
const stripBabelLoaderPrefix = log => log.replace(/@?\s*(\.\/~\/babel-loader\/lib\?{[\s\S]*?}!)/g, '');
Copy link
Member

@developit developit Jul 8, 2017

Choose a reason for hiding this comment

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

I have a boatload more of these transforms we can add, I'll get clearance to release them shortly.

Copy link

@matthewmueller matthewmueller Sep 15, 2017

Choose a reason for hiding this comment

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

Would love to see those boatload of transforms @developit !