Conversation
860b385 to
f24aa09
Compare
|
Is this a workaround? Can you provide more context about the impl of Webpack, that might help alot? |
|
No, it's not a workaround. For webpack, NormalModule has an |
| @@ -0,0 +1,19 @@ | |||
| try { | |||
| require("./recoverable.js"); | |||
| require("./non-recoverable.js"); | |||
There was a problem hiding this comment.
Can you shed more light on the intention of splitting these two?
There was a problem hiding this comment.
Recoverable means 'whether these errors recoverable'. For example, let let = let in strict mode is a recoverable error, so parse_file_as_module will return Ok and errors is not empty, ##!!## is not a recoverable error, so parse_file_as_module will return Err.
There was a problem hiding this comment.
The naming of these two error types seems introduced by SWC, not by the spec, it's kind of confusing for reviewer.
There was a problem hiding this comment.
Actually it is called 'early error' in the spec, but seems swc call it recoverable error.
There was a problem hiding this comment.
The spec also defines that:
All errors that are not early errors are runtime errors.
The fact is the snippet of ##!!## is not even Lexical grammarly right because in order to become an early error you have to be a semantic error in the first place and it's neither a runtime error. so I don't think the naming here is a good choice. Following SWC's wording makes people confused more.
There was a problem hiding this comment.
What name do you suggest?
There was a problem hiding this comment.
I would like to call the recoverable error an semantic-early-error like what you suggested and the non-recoverable a parse-error. Just an advice.
LGTM, please make sure CI passed before merge. Would you mind pasting these to the description part of this PR? Developers in the future may refer to this (comment maybe skipped by quick viewers). |
| Error::TraceableError(v) => write!(f, "{}", v), | ||
| Error::Io { source } => write!(f, "{source}"), | ||
| Error::Anyhow { source } => write!(f, "{source}"), | ||
| Error::BatchErrors(errs) => write!( |
There was a problem hiding this comment.
Remove thiserror because of this line.
|
!snapshot |
|
0.0.0-20221108071754 |
Summary
fixes #1026 #1042 #973
Test Plan
Related issue (if exists)
Further reading
For webpack, NormalModule has an error field, in build webpack will assign the error field according the loaders or parse result, in codeGeneration webpack will generate different code according the error field. In this implementation I merged the source field and the error field by enum.