dart-sass icon indicating copy to clipboard operation
dart-sass copied to clipboard

Exception when extending selector pseudo class (e.g. `not`, `matches`) containing a combinator without a rhs selector

Open connorskees opened this issue 5 years ago • 4 comments

dart-sass will raise an exception for the following input,

:has(a > ) b {
    @extend b;
}

where :has may be substituted for any of the selectors listed here. It is possible to trigger this without pseudo selectors as well, but I have not yet found a good minification for this case.

This error appears to occur in the done callback passed to chunks from weaveParents.

The easiest solution is to simply return true when sequence.isEmpty. Returning null from weaveParents when this error occurs provides a better output, but should presumably be much more complex to implement. libsass implements the former solution, though now that it is going to be deprecated, it seems that maintaining parity is less important.

If making the sequence.isEmpty change,

For the input

:has(a > ) b {
    @extend b;
    color: red;
}

dart-sass would emit (this is also what libsass emits)

:has(a >) b, :has(a >) :has(a >) :has(a >) b, :has(a >) :has(a >) :has(a >) b {
  color: red;
}

If instead returning early with null, dart-sass will emit the input unchanged.

Stacktrace
Unexpected exception:
Bad state: No element


dart:collection                                             ListMixin.first
package:sass/src/extend/functions.dart 231:65               _weaveParents.<fn>
package:sass/src/extend/functions.dart 483:15               _chunks
package:sass/src/extend/functions.dart 230:17               _weaveParents
package:sass/src/extend/functions.dart 156:28               weave
package:sass/src/extend/extender.dart 541:14                Extender._extendComplex.<fn>
dart:core                                                   Iterable.toList
package:sass/src/extend/extender.dart 557:8                 Extender._extendComplex
package:sass/src/extend/extender.dart 474:20                Extender._extendList
package:sass/src/extend/extender.dart 371:26                Extender._extendExistingSelectors
package:sass/src/extend/extender.dart 279:7                 Extender.addExtension
package:sass/src/visitor/evaluate.dart 1136:17              _EvaluateVisitor.visitExtendRule
package:sass/src/ast/sass/statement/extend_rule.dart 29:55  ExtendRule.accept
package:sass/src/visitor/evaluate.dart 1734:17              _EvaluateVisitor.visitStyleRule.<fn>.<fn>
package:sass/src/visitor/evaluate.dart 2837:26              _EvaluateVisitor._withStyleRule
package:sass/src/visitor/evaluate.dart 1732:7               _EvaluateVisitor.visitStyleRule.<fn>
package:sass/src/environment.dart 754:24                    Environment.scope
package:sass/src/visitor/evaluate.dart 2801:31              _EvaluateVisitor._withParent
package:sass/src/visitor/evaluate.dart 1731:5               _EvaluateVisitor.visitStyleRule
package:sass/src/ast/sass/statement/style_rule.dart 26:55   StyleRule.accept
package:sass/src/visitor/evaluate.dart 862:13               _EvaluateVisitor.visitStylesheet
package:sass/src/visitor/evaluate.dart 678:7                _EvaluateVisitor._execute.<fn>
package:sass/src/visitor/evaluate.dart 2700:26              _EvaluateVisitor._withEnvironment
package:sass/src/visitor/evaluate.dart 648:5                _EvaluateVisitor._execute
package:sass/src/visitor/evaluate.dart 486:20               _EvaluateVisitor.run.<fn>
package:sass/src/warn.dart 32:20                            withWarnCallback.<fn>
dart:async                                                  runZoned
package:sass/src/warn.dart 31:10                            withWarnCallback
package:sass/src/visitor/evaluate.dart 502:12               _EvaluateVisitor._withWarnCallback
package:sass/src/visitor/evaluate.dart 473:12               _EvaluateVisitor.run
package:sass/src/visitor/evaluate.dart 95:10                evaluate
package:sass/src/compile.dart 134:24                        _compileStylesheet
package:sass/src/compile.dart 64:10                         compile
package:sass/src/executable/compile_stylesheet.dart 91:13   compileStylesheet

connorskees avatar Aug 05 '20 06:08 connorskees

Reproduction without pseudo selectors:

a a b {
    @extend a;
}

a c > {
    @extend a, b;
}

connorskees avatar Aug 20 '20 14:08 connorskees

We should probably consider just declaring that selector pseudos that end in combinators are errors, too. But your pseudo-free example should definitely not crash.

nex3 avatar Sep 04 '20 19:09 nex3

Honestly, there's a case to be made that selectors with trailing extenders just shouldn't be treated as valid extenders at all, although that's enough of a breaking change it would need to go through a deprecation process.

nex3 avatar Sep 04 '20 20:09 nex3

It looks like the root cause of this logic error is that complexIsParentSuperselector() says that b is a superselector of a > > b, but doesn't recognize that a > > b is a superselector of itself. I tried adjusting it so that it handled repeated combinators more gracefully, but it seems to have triggered an infinite loop elsewhere. Given how complex this is, I'm going to mark this as "low priority" and plan to handle it via https://github.com/sass/sass/issues/3340 instead.

nex3 avatar Sep 04 '20 21:09 nex3