feat: support Rollup logging functions #13757
Conversation
|
|
mato533
changed the title
sapphi-red
added
enhancement
mato533
changed the title
There was a problem hiding this comment.
Thank you for the PR!
The support for this.info/this.debug is needed too.
https://github.com/vitejs/vite/pull/13608/files#diff-c0b6eb176448702b6325d2e1b2567c5ccf969f03eef1242950e0bfb18877022fR192-R196
https://github.com/vitejs/vite/pull/13608/files#diff-c0b6eb176448702b6325d2e1b2567c5ccf969f03eef1242950e0bfb18877022fR412-R413
packages/vite/src/node/build.ts
Outdated
| config.logger.info( | ||
| `${colors.bold( | ||
| colors.gray(`[plugin:${logging.plugin}]`), | ||
| )} ${colors.gray(logging.message)}`, | ||
| ) |
There was a problem hiding this comment.
We use debug function for debug logs in other places. Let's use it here as well.
https://github.com/vitejs/vite/blob/1292ad08a06e2ae8c44307797ba533f64a602759/packages/vite/src/node/utils.ts#L157C17-L157C31
There was a problem hiding this comment.
@sapphi-red
Thank you for your review.
I missed vite's debugger implementation. I pushed the commit fixing it to use Vite's debugger.
Please check it.
| onwarn(warning, warn) { | ||
| onRollupWarning(warning, warn, config) | ||
| onLog(level, log, defaultHandler) { | ||
| onRollupLog(level, log, defaultHandler, config) |
There was a problem hiding this comment.
Would you check if onwarn is called before onLog or not? If onLog is called before onwarn, I think we'll need to pass both onwarn and onLog.
There was a problem hiding this comment.
@sapphi-red
Thank you for your review!
When rollup calls the logger with warn, onwarn is called after onLog. And my understanding is that onwarn is only called when the default handler is called at onLog.
In the event of a warning, only logs that are not handled by the handling implemented in onRollupLog (ignoring specific warning logs or handling plugin logs) will be passed to the default handler and onwarn will be called. On that premise, passing the same implementation to onLog and onwarn does not make sense because the same handling (which is also executed in onlog) is executed.
What do you think about this?
Description
This PR contains feature of supporting Rollup logging function.
Originally, the Vite used onWarn to handle only warnings, but this pull request uses onLog to add handling for info, debug, and error log levels.
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).