Log vfile messages in Webpack loader - #2683
Conversation
Some of our integrations turn vfile messages into log messages for the tool they integrate with. This was missing from the Webpack loader. This change logs vfile messages in the Webpack loader. For more information on Webpack logger severity, see https://webpack.js.org/api/logging/#logger-methods. The Webpack logger allows you to log arbritrary values. These get stringified. However, this is extremely verbose. Instead, I decided to represent messaged with single-line strings. The logs contain a full file path with line number. I find this useful, because some terminal editors make this format clickable.
github-actions
Bot
added
👋 phase/new
remcohaszing
added
🦋 type/enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2683 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 21 21
Lines 2649 2649
Branches 2 2
=========================================
Hits 2649 2649 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| } else { | ||
| logger.warn(log) | ||
| } | ||
| } |
There was a problem hiding this comment.
Somewhat important:
- Maybe a separate function, to build a message?
More options:
- other than positional info (maybe also start/end), source/ruleId, maybe
causecan be added?mdx/packages/esbuild/lib/index.js
Lines 203 to 206 in c20db6a
- have you considered using
reasoninstead ofmessage? I think that’s the normal field inVFileMessage, IIRCmessagehas a little more things (because it’s anError)
There could of course be more (https://github.com/vfile/vfile-reporter/blob/270c90b69c836ed03aab5e49fc3fe93d372fe2cd/lib/index.js#L363), and that can be your answer to the above too, but things can be added too.
There was a problem hiding this comment.
From https://webpack.js.org/api/logging/:
Warning
Avoid noise in the log! Keep in mind that multiple plugins and loaders are used together. Loaders are usually processing multiple files and are invoked for every file. Choose a logging level as low as possible to keep the log output informative.
So I decided to keep the logs minimal, but informative, on a single line.
The source and rule id are already logged.
It would be possible to log the cause, note, and url at a more verbose severity.
The message and reason properties of VFileMessage are assigned the same value, but I can use reason if your prefer.
There was a problem hiding this comment.
Yes, I think reason is better; that’s the one chosen for vfile-message; message is from Error / for Error compatibility
As for their warning: understandable, but I don’t think MDX should make that choice. I think plugins need to make that choice.
There was a problem hiding this comment.
As for their warning: understandable, but I don’t think MDX should make that choice. I think plugins need to make that choice.
I don’t understand. This loader is where we do make the choice what to display to the user and how to format it. It’s up to the plugin what information to provide.
There was a problem hiding this comment.
In short:
a user can intentionally use a plugin that provides a lot of info in logs.
Neither MDX nor Webpack should remove all that important info.
Only if there are options
There was a problem hiding this comment.
Yes, the option is the logger verbosity. We can log other fields at a different verbosity.
Also use `message.reason` instead of `message.message`.
2 participants
Initial checklist
Description of changes
Some of our integrations turn vfile messages into log messages for the tool they integrate with. This was missing from the Webpack loader. This change logs vfile messages in the Webpack loader.
For more information on Webpack logger severity, see https://webpack.js.org/api/logging/#logger-methods.
The Webpack logger allows you to log arbritrary values. These get stringified. However, this is extremely verbose. Instead, I decided to represent messaged with single-line strings. The logs contain a full file path with line number. I find this useful, because some terminal editors make this format clickable.
The following screenshot shows that it looks like