refactor: print diagnostic code in front of message#10227
refactor: print diagnostic code in front of message#10227LingyuCoder wants to merge 1 commit intomainfrom
Conversation
✅ Deploy Preview for rspack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors diagnostic messages by printing a diagnostic code in front of the message for improved clarity when identifying error scopes. Key changes include updates to the formatting logic in error handling (e.g. in concatErrorMsgAndStack and loader-runner), standardized diagnostic code labels across multiple plugins, and consistent renaming of diagnostic titles in tests and plugins.
Reviewed Changes
Copilot reviewed 14 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rspack/src/util/index.ts | Updates error message formatting by stripping the error name from the stack trace. |
| packages/rspack/src/loader-runner/index.ts | Simplifies diagnostic message generation by using the original diagnostic message. |
| packages/rspack-test-tools/tests/errorCases/warning-map.js | Renames the diagnostic name to align with updated formatting. |
| crates/rspack_plugin_wasm/src/parser_and_generator.rs | Refactors error diagnostic titles to the shorter "WasmParser". |
| crates/rspack_plugin_warn_sensitive_module/src/lib.rs | Renames the diagnostic title from "Sensitive Modules Warn" to "WarnCaseSensitive". |
| crates/rspack_plugin_sri/src/lib.rs | Adjusts the SRI error message for better clarity. |
| crates/rspack_plugin_size_limits/src/lib.rs | Changes warning titles from descriptive messages to the concise "SizeLimits". |
| crates/rspack_plugin_mf/src/sharing/provide_shared_plugin.rs | Updates plugin title from a verbose string to "ProvideShared". |
| crates/rspack_plugin_mf/src/sharing/consume_shared_plugin.rs | Updates plugin title but introduces a potential typo in the new title. |
| crates/rspack_plugin_css/src/plugin/impl_plugin_for_css_plugin.rs | Shortens diagnostic title to "CssPlugin". |
| crates/rspack_plugin_copy/src/lib.rs | Standardizes error messages by using "CopyPlugin" consistently. |
| crates/rspack_plugin_circular_dependencies/src/lib.rs | Streamlines the circular dependency error message format. |
| crates/rspack_error/src/graphical.rs | Adjusts the error rendering to include the diagnostic code in the output. |
| crates/rspack_core/src/diagnostics.rs | Modifies display implementations for module build and parse errors for consistency. |
Files not reviewed (4)
- packages/rspack-test-tools/tests/snapshots/NewCodeSplitting-stats-output.test.js.snap: Language not supported
- packages/rspack-test-tools/tests/snapshots/StatsOutput.test.js.snap: Language not supported
- packages/rspack-test-tools/tests/diagnosticsCases/module-build-failed/loader-emit-diagnostic/stats.err: Language not supported
- packages/rspack-test-tools/tests/diagnosticsCases/plugins/circular-dependency-plugin/stats.err: Language not supported
Comments suppressed due to low confidence (1)
crates/rspack_plugin_mf/src/sharing/consume_shared_plugin.rs:475
- The diagnostic title 'CosumeShared' appears to be a typo. It should likely be 'ConsumeShared' for consistency and clarity.
+ "CosumeShared"
github-actions
bot
added
the
team
| impl miette::Diagnostic for ModuleBuildError { | ||
| fn code<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> { | ||
| Some(Box::new("ModuleBuildError")) | ||
| Some(Box::new("Module build failed")) |
There was a problem hiding this comment.
Diagnostic code should be PascalCase. This is intended to work as WebpackError.name in webpack.
| .expect("failed to obtain lock of `diagnostics`") | ||
| .push(Diagnostic::error( | ||
| "CopyRspackPlugin Error".into(), | ||
| "CopyPlugin".into(), |
There was a problem hiding this comment.
Both Diagnostic::warn and Diagnostic::error are legacy APIs. So the title is not the same as which in the TraceableError. I prefer to align this part with TraceableError.
|
IMO, If we need to print the |
CodSpeed Performance ReportMerging #10227 will not alter performanceComparing Summary
|
Ok, I will close this PR and perhaps do some work after #10057 is merged. |
LGTM |
2 participants
Summary
close #9724
perhaps relate to #10081
Print diagnostic code in front of message. This enables us to quickly identify the scope where the error lies.
Checklist