linkedin / css-blocks

High performance, maintainable stylesheets.
http://css-blocks.com/
BSD 2-Clause "Simplified" License
6.33k stars 152 forks source link

More robust importing #399

Closed chriseppstein closed 4 years ago

chriseppstein commented 4 years ago

My fix for https://github.com/linkedin/css-blocks/issues/248 introduced a stack on error objects that allowed errors to track the location of the error within a series of imports. This turned out to be problematic because our blocks are singletons and so the errors for that block also want to be singletons. Because a block might be imported (and exported) from different blocks, the stack needed to be tracked differently.

This PR adds the CascadingError error for indicating an error was caused by some previous error and uses it for block imports and exports.

This allowed me to stop doing weird things trying to parse a block a second time if it previously had an error -- just to get a different error instance.

I've done a pretty major refactor of how @block at-rules are resolved. The old code was a convoluted mess of promise chains spawned while AST walking and then resolved afterwards. I've rewritten with async/await and have separated out the AST walking from the work done based on what we found in the document.

Building on the MultipleBlockErrors type by @ramitha, I've updated our error reporting in a few places to describe the sequence of events that lead up to an error.

For example, the CLI now displays the root cause for an error like so:

error   test/fixtures/basic/error.block.css
        Two distinct classes cannot be selected on the same element: .foo.bar
        At test/fixtures/basic/error.block.css:1:5
        1: .foo.bar {
        2:   color: red;
caused  test/fixtures/basic/transitive-error.block.css
        Error in imported block.
        At test/fixtures/basic/transitive-error.block.css:1:1
        1: @block error from "./error.block.css";
        2:

error   test/fixtures/basic/transitive-error.block.css
        No Block named "error" found in scope.
        At test/fixtures/basic/transitive-error.block.css:4:3
        3: :scope {
        4:   extends: error;
        5: }

Found 2 errors in 1 file.

This PR also introduces a utility function allDone that is like Promise.all() except it waits for all the work to complete first. In most places, I've stopped using Promise.all() after I became aware that it has potential race condition behavior within a broccoli build/rebuild loop.