feat: strf-9440 Stencil Bundle: fail on scss failure compilation #884
Conversation
|
|
||
| try { | ||
| const css = await stencilStyles.compileCss('scss', params); | ||
| const css = cssCompiler.compile(configuration, themeAssetsPath, fileName); |
There was a problem hiding this comment.
Do we need to await cssCompiler.compile here?
lib/bundle-validator.js
Outdated
| throw new Error(`Error: ${e.message}, while reading files from "${stylesPath}".`.red); | ||
| } | ||
|
|
||
| for await (const file of cssFiles) { |
There was a problem hiding this comment.
Does the use of for await here mean that cssFiles is an async iterable object? It seems like we're doing await inside of the try so does that mean the filter/slice operation results in an async type?
There was a problem hiding this comment.
I think it was a leftover from previous solution. But you are right, that for await should be used for async iterable objects. On other hand it also supports sync iterables, I think it's cleaner to separate those concerns
There was a problem hiding this comment.
I think the problem was in the eslint as well: https://eslint.org/docs/rules/no-await-in-loop
But those operation steps should go in sync way, so I disabled that rule for the next line
jairo-bc
force-pushed
the
STRF-9440
branch
2 times, most recently
from
dcb13a4 to
294bfea
Compare
|
Hi @jairo-bc , this is great in theory, but in practice it is breaking our themes. We declare SCSS variables in a separate file that gets included before our other SCSS files. The compiler is telling us that a variable is undefined, and thus it will not bundle the theme. However, the variable is defined, it's just in another file. Try it out yourself by declaring a variable, and then using it in another file. This is a common practice for developers, and I don't think it should be disallowed. |
|
@jairo-bc , sorry, this appears to now be fixed in 4.1! |
What?
Scss compilation validation task during theme bundling by running
stencil bundlecommandTickets / Documentation
Screenshots (if appropriate)
cc @bigcommerce/storefront-team