Fail stencil bundle on Webpack compile errors

[jbruni]

What?

When Webpack compilation fails, stencil bundle blindly ignores it.

As a result, the generated theme ZIP file does not contain the assets/dist folder (just like in issue bigcommerce/stencil-cli#379)

How to reproduce

Step-by-step from fresh Cornerstone, below.

In this example, we will simply add a line of JavaScript code which Babel will fail to understand:

# 1. Fresh clone of Cornerstone repository
git clone https://github.com/bigcommerce/cornerstone.git
# 2. Make cornerstone the current working directory
cd cornerstone
# 3. Install dependencies
npm ci
# 4. Generate theme ZIP
stencil bundle
# 5. Check that the generated ZIP file contains assets/dist folder
# 6. Append a line of code to "assets/js/app.js"
echo 'console.log(<h1>Hi</h1>); // eslint-disable-line' >> assets/js/app.js
# This is valid JSX code... but babel-loader needs a plugin to understand it
# 7. Generate theme ZIP again
stencil bundle
# 8. Check that the generated ZIP file does not contain assets/dist folder

In our real-world scenario, the Webpack build failed for other reasons, and we didn't need to shut up ESLint as in this example (the code passed in ESLint checks). Our scenario is more complex to reproduce. It requires other Webpack loaders and configurations.

The aim with this easy-to-reproduce example is to show how a Webpack build failure is ignored.

Later we learned that this happens when an exception is thrown from a Webpack loader (comment from sepo-one at webpack/webpack#708).

Solution

The callback for Webpack's run is (err, [stats](#stats-object))

Currently, err is considered, but stats is ignored.

Problem is that even when there is no err, errors may have happened in Webpack compilation, and they are available at the stats object. Screenshot below is from Webpack documentation:

This resembles Ajax error handling... there, we have "two layers" of possible errors... one of the Ajax call failing itself... and the other of the Ajax call returning an error code, or even some "error" code/message contained in an HTTP 200 response (terrible practice, but we can find it out there).

Likewise, we have something similar in Webpack's run... an err for when the overall compile itself fails... and the other (stats) of the compiler completing its work and returning an error code/message...

Tickets / Documentation

• Webpack run method documentation
• Webpack stats object documentation
• Webpack options for stats.toString() method

Screenshots

Before changes from this PR, stencil bundle fails silently:

After changes from this PR, we can see Webpack error message, and the bundle is interrupted:

[jbruni]

Thanks to @carsonreinke which first reported the issue.

[junedkazi]

@jbruni thanks for the PR. This is awesome. Can you also add a changelog entry to the PR.

[carsonreinke]

@jbruni I think this should:

1. Report warnings besides just errors
2. Report errors in Development environment

But at least now you can see the the failure when bundling.

[junedkazi]

I agree if we have warning we should console log it. We shouldn't exit or fail the bundle but atleast notify the developer.

[jbruni]

• Added changelog entry

• Added error report in Development environment; looks good:
  

• I initially added the warning report, but then I reverted it. Why? Because this is the result when bundling Cornerstone with warning report:

In my opinion, we shall first ensure Cornerstone out-of-the-box won't raise warnings, and then we can include the Webpack warning report.

[jbruni]

@junedkazi - Please take a look at this commit which I pushed here: jbruni@e86e9b2

• Changes in stencil.conf.js enable Webpack warning report on stencil start (development) and stencil bundle (production);

• Change in webpack.common.js increases the maximum asset size from 150 KiB to 300 KiB; I was surprised to find these settings there... although the current limit is set to 150 KiB, it makes no difference at all, since it has no effect without warnings being reported; increasing the size to 300 KiB avoids the warning when bundling out-of-the-box Cornerstone, with the warning reports turned on;

• Change at webpack.dev.js turns off the performance hint warning for development; the size of the assets in development is much bigger (unminified, with source maps, etc); the theme-bundle.main.js size grows to 1.61 MiB; thus, we don't want asset size warning in development.

Related documentation here:

• https://webpack.js.org/configuration/performance/

If these changes look good to you, I can add that commit to this PR branch.

[junedkazi]

@jbruni they look good.

[jbruni]

@junedkazi - Ok. 👍 I've just pushed the other commit to this PR branch.

[junedkazi]

@jbruni thank you for the awesome work on this PR 👏.