fix(diagnostics): workaround Rollup duplicating error messages#373
Merged
Conversation
- per my investigation in the linked issues, it seems like Rollup has a bug where it duplicates some error message
- this occurs when the error has a stack (or frame) which contains the error message itself
- Rollup prints _both_ the error message _and_ the stack in that case, causing duplication
- this fix adds a workaround for this upstream Rollup bug
- it detects if there is a stack and if the message is duplicated in the stack
- if so, it removes the duplication in the stack
- this workaround should be forward-compatible if this behavior is fixed upstream
- this code should just end up re-throwing in that case (effectively a no-op)
Collaborator
Author
|
sh*t, I was reading #103 (comment) and this seems to break watch mode for some reason.... back to the drawing board again.... |
Collaborator
Author
|
Oh... well apparently Rollup needs some other properties that it attaches to the error: {
id: '/project-dir/src/difference.ts',
hook: 'transform',
code: 'PLUGIN_ERROR',
plugin: 'rpt2',
watchFiles: [
'/project-dir/src/index.ts',
'/project-dir/tsconfig.json',
'/project-dir/src/types.ts',
'/project-dir/src/difference.ts'
]
}
}So need to spread those in. And spreading them in makes watch mode work again, thankfully |
- apparently Rollup attaches several properties to the error object, including `watchFiles`
- so removing them / not spreading causes watch to just stop working
here are some of the additional properties I logged out, for example:
```js
{
id: '/project-dir/src/difference.ts',
hook: 'transform',
code: 'PLUGIN_ERROR',
plugin: 'rpt2',
watchFiles: [
'/project-dir/src/index.ts',
'/project-dir/tsconfig.json',
'/project-dir/src/types.ts',
'/project-dir/src/difference.ts'
]
}
}
```
Collaborator
Author
|
This has been fixed upstream in Rollup in rollup/rollup#4749 and released in Rollup v3.7.5 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a workaround for a Rollup bug that causes duplicate error messages
pretty: true) #103prettydefaults totruein TS 2.9+ #372, which makes this bug much more noticeableDetails
per Duplicate error message (esp. when
pretty: true) #103 (comment) and my investigation in fix(diagnostics):prettydefaults totruein TS 2.9+ #372, it seems like Rollup has a bug where it duplicates some error messagethis fix adds a workaround for this upstream Rollup bug
Review Notes
This is gonna merge conflict pretty hard with #345 's current code (as these both add the
buildEndhook), but I made them both compatible with each other. Will just have to update one or the other when one is mergedFuture Work
I'll make a PR to Rollup proper and link it here as well