Skip to content

Improved output on webpack error #3241

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

Merged

Conversation

pocari
Copy link
Contributor

@pocari pocari commented Nov 29, 2021

When webpack terminates with an error, the reason may not be clear from the stdout and stderr output alone.
For example, I could not understand the reason when webpack was killed. (See the before output below).
For this reason, when webpack fails, it outputs the status information in addition to the stdout and stderr output. Now, I can notice when webpack is killed. (See the after output below).

output examples

before:

** Execute webpacker:compile
Compiling...
Compilation failed:
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating

after:

** Execute webpacker:compile
Compiling...
Compilation failed:
 ExitStatus: pid 1064 SIGKILL (signal 9)
 Outputs: Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating

@@ -89,7 +89,7 @@ def run_webpack
end
else
non_empty_streams = [stdout, stderr].delete_if(&:empty?)
logger.error "Compilation failed:\n#{non_empty_streams.join("\n\n")}"
logger.error "Compilation failed:\n ExitStatus: #{status}\n Outputs: #{non_empty_streams.join("\n\n")}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can make this even a bit more obvious.

Can you try adding a \n before the non_empty_streams part, as your example shows it's hard to see how the messages are grouped by the indenting?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can add a blank line before this and make all caps.

Compilation failed:

Copy link
Contributor Author

@pocari pocari Nov 30, 2021

Choose a reason for hiding this comment

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

@justin808 Thanks for the comment.
The following is the result of the modification ( 5900430 , 354af2a ).
Does the output meet your requirements?

** Execute webpacker:compile
Compiling...

COMPILATION FAILED:
EXIT STATUS: pid 414 SIGTERM (signal 15)
OUTPUTS:
Browserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db

Why you should do it regularly:
https://github.com/browserslist/browserslist#browsers-data-updating

@justin808 justin808 added this to the 6.0 milestone Dec 5, 2021
@justin808
Copy link
Contributor

@guillaumebriday this one looks good to merge. Safe change.

@pocari Thanks for the contribution! Much appreciated!

@guillaumebriday guillaumebriday merged commit 614cb1c into rails:master Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants