Make error reports be user friendly

[zindel]

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:

1. Parse errors
   Parse errors are received from the Flow_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.

2. 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 or file-loader

3. Suggest the mock package for node libraries
   Sometimes some of the 2+ level dependencies requires some of node.js base libraries, like fs or stream. 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

4. 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 like npm-error.log). I've been thinking about the fpack-debug/error.txt and fpack-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

[samouri]

Hi @zindel!

Thank you for spelling everything out so clearly.
I've started working on solving number 1 (Parse errors) here

[samouri]

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.

[zindel]

@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 ^