Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show Webpack compilation warnings in CLI #2186

Merged
merged 7 commits into from
Feb 15, 2024
Merged

Conversation

Mrtenz
Copy link
Member

@Mrtenz Mrtenz commented Feb 15, 2024

Compilation warnings were previously ignored, potentially hiding important warnings from Webpack. This updates the Webpack config to show these warnings, and does some other changes to make warnings more readable.

Closes #2145.

@Mrtenz Mrtenz requested a review from a team as a code owner February 15, 2024 12:07
environment: defaulted(record(string(), unknown()), {
NODE_DEBUG: false,
NODE_ENV: 'production',
DEBUG: false,
}),
environment: defaulted(record(string(), unknown()), {}),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, the defaults would be completely ignored if any value was used. I've changed it to merge the defaults with the custom values provided in the config.

*/
new EnvironmentPlugin(config.environment),
new DefinePlugin(getEnvironmentVariables(config.environment)),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EnvironmentPlugin was causing compiler warnings. Switching to DefinePlugin seems to have resolved this issue.

Comment on lines +99 to +101
const path = dirname(this.resourcePath);
for (const name of Object.keys(imports)) {
this.addDependency(name);
this.addDependency(resolve(path, name));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependencies must use absolute paths. This solves a warning when using the WASM loader.

@Mrtenz Mrtenz force-pushed the mrtenz/show-compilation-warnings branch from b7a200d to c6e6a3e Compare February 15, 2024 12:48
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7be3338) 96.54% compared to head (c6e6a3e) 96.55%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2186   +/-   ##
=======================================
  Coverage   96.54%   96.55%           
=======================================
  Files         331      331           
  Lines        7466     7482   +16     
  Branches     1148     1152    +4     
=======================================
+ Hits         7208     7224   +16     
  Misses        258      258           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Mrtenz Mrtenz merged commit c0a0805 into main Feb 15, 2024
147 checks passed
@Mrtenz Mrtenz deleted the mrtenz/show-compilation-warnings branch February 15, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI swallows important Webpack errors
2 participants