Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unifyComound() and unifyComplex() no longer move pseudo-classes across pseudo-element boundaries #2350

Merged
merged 11 commits into from
Sep 30, 2024
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
## 1.79.5

* Changes to how `selector.unify()` and `@extend` combine selectors:

* The relative order of pseudo-classes (like `:hover`) and pseudo-elements
(like `::before`) within each original selector is now preserved when
they're combined.

* Pseudo selectors are now consistently placed at the end of the combined
selector, regardless of which selector they came from. Previously, this
reordering only applied to pseudo-selectors in the second selector.

## 1.79.4

### JS API
Expand Down
2 changes: 1 addition & 1 deletion lib/src/ast/selector/type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ final class TypeSelector extends SimpleSelector {
/// @nodoc
@internal
List<SimpleSelector>? unify(List<SimpleSelector> compound) {
if (compound.first case UniversalSelector() || TypeSelector()) {
if (compound.firstOrNull case UniversalSelector() || TypeSelector()) {
var unified = unifyUniversalAndElement(this, compound.first);
if (unified == null) return null;
return [unified, ...compound.skip(1)];
Expand Down
51 changes: 37 additions & 14 deletions lib/src/extend/functions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ List<ComplexSelector>? unifyComplex(
List<ComplexSelector> complexes, FileSpan span) {
if (complexes.length == 1) return complexes;

List<SimpleSelector>? unifiedBase;
CompoundSelector? unifiedBase;
CssValue<Combinator>? leadingCombinator;
CssValue<Combinator>? trailingCombinator;
for (var complex in complexes) {
Expand Down Expand Up @@ -67,12 +67,10 @@ List<ComplexSelector>? unifyComplex(
}

if (unifiedBase == null) {
unifiedBase = base.selector.components;
unifiedBase = base.selector;
} else {
for (var simple in base.selector.components) {
unifiedBase = simple.unify(unifiedBase!); // dart-lang/sdk#45348
if (unifiedBase == null) return null;
}
unifiedBase = unifyCompound(unifiedBase, base.selector);
if (unifiedBase == null) return null;
}
}

Expand All @@ -87,7 +85,7 @@ List<ComplexSelector>? unifyComplex(
var base = ComplexSelector(
leadingCombinator == null ? const [] : [leadingCombinator],
[
ComplexSelectorComponent(CompoundSelector(unifiedBase!, span),
ComplexSelectorComponent(unifiedBase!,
trailingCombinator == null ? const [] : [trailingCombinator], span)
],
span,
Expand All @@ -106,19 +104,44 @@ List<ComplexSelector>? unifyComplex(
/// Returns a [CompoundSelector] that matches only elements that are matched by
/// both [compound1] and [compound2].
///
/// The [span] will be used for the new unified selector.
/// The [compound1]'s `span` will be used for the new unified selector.
///
/// This function ensures that the relative order of pseudo-classes (`:`) and
/// pseudo-elements (`::`) within each input selector is preserved in the
/// resulting combined selector.
///
/// This function enforces a general rule that pseudo-classes (`:`) should come
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
/// before pseudo-elements (`::`), but it won't change their order if they were
/// originally interleaved within a single input selector. This prevents
/// unintended changes to the selector's meaning. For example, unifying
/// `::foo:bar` and `:baz` results in `:baz::foo:bar`. `:baz` is a pseudo-class,
/// so it is moved before the pseudo-class `::foo`. Meanwhile, `:bar` is not
/// moved before `::foo` because it appeared after `::foo` in the original
/// selector.
///
/// If no such selector can be produced, returns `null`.
CompoundSelector? unifyCompound(
CompoundSelector compound1, CompoundSelector compound2) {
var result = compound2.components;
for (var simple in compound1.components) {
var unified = simple.unify(result);
if (unified == null) return null;
result = unified;
var result = compound1.components;
var pseudoResult = <SimpleSelector>[];
var pseudoElementFound = false;

for (var simple in compound2.components) {
// All pseudo-classes are unified separately after a pseudo-element to
// preserve their relative order with the pseudo-element.
if (pseudoElementFound && simple is PseudoSelector) {
var unified = simple.unify(pseudoResult);
Goodwine marked this conversation as resolved.
Show resolved Hide resolved
if (unified == null) return null;
pseudoResult = unified;
} else {
pseudoElementFound |= simple is PseudoSelector && simple.isElement;
var unified = simple.unify(result);
if (unified == null) return null;
result = unified;
}
}

return CompoundSelector(result, compound1.span);
return CompoundSelector([...result, ...pseudoResult], compound1.span);
}

/// Returns a [SimpleSelector] that matches only elements that are matched by
Expand Down
4 changes: 4 additions & 0 deletions pkg/sass-parser/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.2.4

* No user-visible changes.

## 0.2.3

* No user-visible changes.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sass-parser/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "sass-parser",
"version": "0.2.3",
"version": "0.2.4",
"description": "A PostCSS-compatible wrapper of the official Sass parser",
"repository": "sass/sass",
"author": "Google Inc.",
Expand Down
4 changes: 4 additions & 0 deletions pkg/sass_api/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 12.0.5

* No user-visible changes.

## 12.0.4

* No user-visible changes.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sass_api/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ 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: 12.0.4
version: 12.0.5
description: Additional APIs for Dart Sass.
homepage: https://github.com/sass/dart-sass

environment:
sdk: ">=3.0.0 <4.0.0"

dependencies:
sass: 1.79.4
sass: 1.79.5

dev_dependencies:
dartdoc: ^8.0.14
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass
version: 1.79.4
version: 1.79.5
description: A Sass implementation in Dart.
homepage: https://github.com/sass/dart-sass

Expand Down