From ecff05dfefc2da3f7e462c2a173ac12beeb51100 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Tue, 11 Jun 2024 16:05:41 -0700 Subject: [PATCH] Remove the heuristic where long selector lists wouldn't be trimmed (#2255) Testing this against the `@extend`-heavy stylesheets in vinceliuice/Colloid-gtk-theme, trimming everywhere actually *improves* performance rather than reducing it. --- CHANGELOG.md | 4 ++++ lib/src/ast/selector/compound.dart | 12 ++++++++++ lib/src/ast/selector/pseudo.dart | 4 ++++ lib/src/ast/selector/simple.dart | 10 ++++++++ lib/src/extend/extension_store.dart | 7 ------ lib/src/extend/functions.dart | 36 ++++++++++++++++++----------- pkg/sass_api/CHANGELOG.md | 4 ++++ pkg/sass_api/pubspec.yaml | 4 ++-- pubspec.yaml | 2 +- 9 files changed, 59 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b6c0862..5f299e042 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.77.5 + +* Fully trim redundant selectors generated by `@extend`. + ## 1.77.4 ### Embedded Sass diff --git a/lib/src/ast/selector/compound.dart b/lib/src/ast/selector/compound.dart index bcc2beb33..9fbc8d397 100644 --- a/lib/src/ast/selector/compound.dart +++ b/lib/src/ast/selector/compound.dart @@ -42,6 +42,18 @@ final class CompoundSelector extends Selector { SimpleSelector? get singleSimple => components.length == 1 ? components.first : null; + /// Whether any simple selector in this contains a selector that requires + /// complex non-local reasoning to determine whether it's a super- or + /// sub-selector. + /// + /// This includes both pseudo-elements and pseudo-selectors that take + /// selectors as arguments. + /// + /// #nodoc + @internal + late final bool hasComplicatedSuperselectorSemantics = components + .any((component) => component.hasComplicatedSuperselectorSemantics); + CompoundSelector(Iterable components, super.span) : components = List.unmodifiable(components) { if (this.components.isEmpty) { diff --git a/lib/src/ast/selector/pseudo.dart b/lib/src/ast/selector/pseudo.dart index 44a263d15..e632c651e 100644 --- a/lib/src/ast/selector/pseudo.dart +++ b/lib/src/ast/selector/pseudo.dart @@ -67,6 +67,10 @@ final class PseudoSelector extends SimpleSelector { bool get isHostContext => isClass && name == 'host-context' && selector != null; + @internal + bool get hasComplicatedSuperselectorSemantics => + isElement || selector != null; + /// The non-selector argument passed to this selector. /// /// This is `null` if there's no argument. If [argument] and [selector] are diff --git a/lib/src/ast/selector/simple.dart b/lib/src/ast/selector/simple.dart index d8ae7864d..5ac2f0e53 100644 --- a/lib/src/ast/selector/simple.dart +++ b/lib/src/ast/selector/simple.dart @@ -34,6 +34,16 @@ abstract base class SimpleSelector extends Selector { /// sequence will contain 1000 simple selectors. int get specificity => 1000; + /// Whether this requires complex non-local reasoning to determine whether + /// it's a super- or sub-selector. + /// + /// This includes both pseudo-elements and pseudo-selectors that take + /// selectors as arguments. + /// + /// #nodoc + @internal + bool get hasComplicatedSuperselectorSemantics => false; + SimpleSelector(super.span); /// Parses a simple selector from [contents]. diff --git a/lib/src/extend/extension_store.dart b/lib/src/extend/extension_store.dart index 3637b5aac..2bbc2a9cb 100644 --- a/lib/src/extend/extension_store.dart +++ b/lib/src/extend/extension_store.dart @@ -901,13 +901,6 @@ class ExtensionStore { // document, and thus should never be trimmed. List _trim(List selectors, bool isOriginal(ComplexSelector complex)) { - // Avoid truly horrific quadratic behavior. - // - // TODO(nweiz): I think there may be a way to get perfect trimming without - // going quadratic by building some sort of trie-like data structure that - // can be used to look up superselectors. - if (selectors.length > 100) return selectors; - // This is n² on the sequences, but only comparing between separate // sequences should limit the quadratic behavior. We iterate from last to // first and reverse the result so that, if two selectors are identical, we diff --git a/lib/src/extend/functions.dart b/lib/src/extend/functions.dart index 01d70d248..2fb1f2555 100644 --- a/lib/src/extend/functions.dart +++ b/lib/src/extend/functions.dart @@ -646,24 +646,28 @@ bool complexIsSuperselector(List complex1, var component1 = complex1[i1]; if (component1.combinators.length > 1) return false; if (remaining1 == 1) { - var parents = complex2.sublist(i2, complex2.length - 1); - if (parents.any((parent) => parent.combinators.length > 1)) return false; - - return compoundIsSuperselector( - component1.selector, complex2.last.selector, - parents: parents); + if (complex2.any((parent) => parent.combinators.length > 1)) { + return false; + } else { + return compoundIsSuperselector( + component1.selector, complex2.last.selector, + parents: component1.selector.hasComplicatedSuperselectorSemantics + ? complex2.sublist(i2, complex2.length - 1) + : null); + } } // Find the first index [endOfSubselector] in [complex2] such that // `complex2.sublist(i2, endOfSubselector + 1)` is a subselector of // [component1.selector]. var endOfSubselector = i2; - List? parents; while (true) { var component2 = complex2[endOfSubselector]; if (component2.combinators.length > 1) return false; if (compoundIsSuperselector(component1.selector, component2.selector, - parents: parents)) { + parents: component1.selector.hasComplicatedSuperselectorSemantics + ? complex2.sublist(i2, endOfSubselector) + : null)) { break; } @@ -675,13 +679,10 @@ bool complexIsSuperselector(List complex1, // to match. return false; } - - parents ??= []; - parents.add(component2); } if (!_compatibleWithPreviousCombinator( - previousCombinator, parents ?? const [])) { + previousCombinator, complex2.take(endOfSubselector).skip(i2))) { return false; } @@ -717,8 +718,8 @@ bool complexIsSuperselector(List complex1, /// Returns whether [parents] are valid intersitial components between one /// complex superselector and another, given that the earlier complex /// superselector had the combinator [previous]. -bool _compatibleWithPreviousCombinator( - CssValue? previous, List parents) { +bool _compatibleWithPreviousCombinator(CssValue? previous, + Iterable parents) { if (parents.isEmpty) return true; if (previous == null) return true; @@ -754,6 +755,13 @@ bool _isSupercombinator( bool compoundIsSuperselector( CompoundSelector compound1, CompoundSelector compound2, {Iterable? parents}) { + if (!compound1.hasComplicatedSuperselectorSemantics && + !compound2.hasComplicatedSuperselectorSemantics) { + if (compound1.components.length > compound2.components.length) return false; + return compound1.components + .every((simple1) => compound2.components.any(simple1.isSuperselector)); + } + // Pseudo elements effectively change the target of a compound selector rather // than narrowing the set of elements to which it applies like other // selectors. As such, if either selector has a pseudo element, they both must diff --git a/pkg/sass_api/CHANGELOG.md b/pkg/sass_api/CHANGELOG.md index 0006705c9..373024d43 100644 --- a/pkg/sass_api/CHANGELOG.md +++ b/pkg/sass_api/CHANGELOG.md @@ -1,3 +1,7 @@ +## 10.4.5 + +* No user-visible changes. + ## 10.4.4 * No user-visible changes. diff --git a/pkg/sass_api/pubspec.yaml b/pkg/sass_api/pubspec.yaml index c184e5f24..69c311a02 100644 --- a/pkg/sass_api/pubspec.yaml +++ b/pkg/sass_api/pubspec.yaml @@ -2,7 +2,7 @@ name: sass_api # Note: Every time we add a new Sass AST node, we need to bump the *major* # version because it's a breaking change for anyone who's implementing the # visitor interface(s). -version: 10.4.4 +version: 10.4.5 description: Additional APIs for Dart Sass. homepage: https://github.com/sass/dart-sass @@ -10,7 +10,7 @@ environment: sdk: ">=3.0.0 <4.0.0" dependencies: - sass: 1.77.4 + sass: 1.77.5 dev_dependencies: dartdoc: ">=6.0.0 <9.0.0" diff --git a/pubspec.yaml b/pubspec.yaml index c8656f802..668acf389 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.77.4 +version: 1.77.5 description: A Sass implementation in Dart. homepage: https://github.com/sass/dart-sass