Make error reports be user friendly
zindel opened this issue · 3 comments
I've been using fastpack lately with several projects and found out that error reporting is far from being perfect of even usable. Here are some ideas which I've got so far:
-
Parse errors
Parse errors are received from theFlow_parser
and then displayed by fastpack in a quite ugly manner:Project Directory: /.../test/error-parse Mode: development Call Stack: './a' from module: index.js './index.js' from module: $fp$main Processing Module: a.js Parse Error File: a.js (1:15) - (1:16): Unexpected token ;
As you can see, it shows the file and the location(s) of the error, as well as the error text. Ideally, we would show the code frame and highlight (using color or some ASCII art) the location for each error in the list.
-
Parse errors - non-JS files
The error message for the error above gets even uglier when fastpack consumes the CSS or static file, thinking of it as being JS. Here is an example:$ cat App.css @import '~antd/dist/antd.css'; .logo { height: 60px; font-weight: bold; font-size: 1.2em; background: white; text-align: center; } $ fpack App.css Project Directory: /Users/zindel/neuron/neuron-ant/src Mode: production Call Stack: './App.css' from module: $fp$main Processing Module: App.css Parse Error File: App.css (1:8) - (1:29): Unexpected string (1:29) - (1:30): Unexpected token ; (1:1) - (3:5): Found a decorator in an unsupported position. (4:12) - (4:16): Unexpected token ILLEGAL (4:14) - (4:16): Unexpected identifier (5:15) - (5:16): Unexpected token : (6:13) - (6:14): Unexpected token : (6:15) - (6:20): Unexpected token ILLEGAL (6:18) - (6:20): Unexpected identifier (8:14) - (8:15): Unexpected token :
Lots of misleading errors for such a short file! Imagine, how long is this list for the binary file. We definitely can do better here by handling the case when module doesn't have preprocessors (we expect all of them to return valid JS) + has some non-JS extension (
.less
,.sass
,.ts
,.png
etc). Moreover, we could suggest the--preprocess
command line argument if we see this instead of displaying the list of errors. For instance for the case above, we could say:Parse Error File: App.css It looks like you're trying to import the CSS file. Try adding the following argument to the config: --preprocess='\.css$:style-loader!css-loader'
Here is the list of extensions & loaders we could offer (fill free to add more :)):
.css
:style-loader!css-loader
.scss
,.sass
:style-loader!css-loader!sass-loader
.less
:style-loader!css-loader!less-loader
.ts
:ts-loader
.html
,.htm
:html-loader
.txt
:raw-loader
.woff
,.woff2
,.svg
,.png
,.jpg
,.jpeg
,.gif
,.ttf
:url-loader
orfile-loader
-
Suggest the mock package for node libraries
Sometimes some of the 2+ level dependencies requires some of node.js base libraries, likefs
orstream
. This may be a general error or done for reason, just accounting for the browser-specific mocks. It would be great if fastpack could detect those cases and make a suggestion alongside with reporting the resolve error. For example:Cannot resolve request 'stream' from module 'x.js'. This looks like base node.js library and unlikely is required in the browser environment. If you still want to use it, here is the suggested command line option: --mock stream:stream-browserify
Here is an example of possible pairs: https://github.com/webpack/node-libs-browser
-
Dump the last error & the last source file where it happened
Finally, it may be useful to be able to dump the last error alongside with the JS source processed into the file (just likenpm-error.log
). I've been thinking about thefpack-debug/error.txt
andfpack-debug/source.js
. Maybe only when--debug
is provided. There are few reasons for it:- sometimes we see the error in the internal transpiler/printer, which leads to a parse error on the bundling phase, but the location reported is misleading;
- we are currently reporting a lot of additional information (project directory, call stack etc.); maybe this should not be reported to the user interactively, but keeping this info for further processing may be useful.
- also, it'd be easier for people to report bugs and provide information by just attaching those files to the issue.
Thoughts? Also, feel free to append to this list if you think something is missing here :)
Checklist:
- Parse Errors
- Parse Errors (non-JS files)
- Suggest the mock package for node libraries
- Dump the last error
I have a few questions:
For number 2: Is there something better we could do? if we know the exact preprocessor that the user would ideally use couldn't we automatically apply the correct preprocessor without having to make the user configure it. We could output a message like: utilizing preprocessor "style-loader!css-loader" for file Y for css compatibility
.
Related to number 3: Since there are cases where we'd want to supply the browser implementation instead of the mock one, wouldn't --replace
be more accurate than --mock
.
@samouri Regarding the first suggestion, it looks quite magical. Also user may want to do it differently or handle the css with some other settings (for example style-loader!css-loader?cssModules=true
). I think the best we can do at a later phase is to have the --dry-run
mode of the fpack
(or some other binary) where it runs on the project & suggests the config.
The --mock
one. I agree that the name is less than ideal. I would offer we rename in a separate pull request though. --replace
sounds good. How about the --resolve
one?
@andreypopp @TrySound any opinion on this ^