Add verbosity to Webpack. #24
Conversation
konraddysput
left a comment
konraddysput
left a comment
There was a problem hiding this comment.
I have mixed feelings about the design and decisions made in the logging system. For me it's not natural to spam error logs in the end instead of showing them when they happened. Why we want to still add a source map or continue adding source maps if we fail somewhere?
| logger.timeEnd(`[${key}] inject sourcemap key`); | ||
| stats.sourceComment = true; | ||
| } catch (err) { | ||
| stats.sourceComment = err instanceof Error ? err : new Error('Unknown error.'); |
There was a problem hiding this comment.
the time command from line 47 will never end. Should we add an error log about that? RIght now the user needs to check the source map comment however no one will do that in practice
| logger.timeEnd(`[${key}] append sourcemap key`); | ||
| stats.sourceMapAppend = true; | ||
| } catch (err) { | ||
| stats.sourceMapAppend = err instanceof Error ? err : new Error('Unknown error.'); |
There was a problem hiding this comment.
the time command from line 66 will not end. Should we add a log error about that?
|
|
||
| export function statsPrinter(logger: webpack.Compilation['logger']) { | ||
| return function printStats(key: string, stats: AssetStats) { | ||
| const errors = [stats.sourceComment, stats.sourceMapAppend, stats.sourceMapUpload, stats.sourceSnippet].some( |
There was a problem hiding this comment.
how about strings? sourceSnippet can be string
|
|
||
| export interface AssetStats { | ||
| readonly debugId: string; | ||
| sourceSnippet?: boolean | string | Error; |
There was a problem hiding this comment.
- why we should continue generating source maps information if we receive an error?
- why are we making this object so hard to maintain. Why do we want to print logs in the end instead of showing them when they happened? It's not natural. We can show the summary however a list of errors in the end might be huge if sourcemap cannot work for some kind of reason (let's say permission denied)
| @@ -0,0 +1,9 @@ | |||
| import { UploadResult } from '@backtrace/sourcemap-tools'; | |||
|
|
|||
| export interface AssetStats { | |||
There was a problem hiding this comment.
Let's think about what we have here. The purpose of the debugId property is to generate an output for the user in the source map. Why are we mixing it together with errors etc. It doesnt' sound right to me
|
Closing in favor of #29 |
Adds logging to Webpack so we know what is going on.
Also adds test Webpack configuration with the plugin used to test the output.