Skip to content

Fix crash when @importing a Sass file that @uses another file #2599

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix crash when @importing a Sass file that @uses another file #2599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7974240
Fix crash when mixin @import with @use
Goodwine Jul 2, 2025
4b9205a
dart run grind
Goodwine Jul 2, 2025
9952e29
Goodwine Jul 2, 2025
af92d93
add changelog and bump version
Goodwine Jul 9, 2025
7cdf182
add backticks to @import
Goodwine Jul 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 1.89.3

* Fixed a crash when `@import` was nested in a style rule and loaded a Sass file
that `@use`s a non-built-in module and a top-level `@include` that emits
declarations.

## 1.89.2

### Embedded Host
Expand Down
27 changes: 20 additions & 7 deletions lib/src/visitor/async_evaluate.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ final class _EvaluateVisitor

ModifiableCssParentNode? __parent;

/// The original parent node for a stylesheet that was loaded with `@import`.
///
/// This value is only set when a file uses `@import` in combination with
/// non-built-in Sass modules.
ModifiableCssParentNode? _importParent;

/// The name of the current declaration parent.
String? _declarationName;

Expand Down Expand Up @@ -1321,7 +1327,12 @@ final class _EvaluateVisitor
}

Future<Value?> visitDeclaration(Declaration node) async {
if (_styleRule == null && !_inUnknownAtRule && !_inKeyframes) {
// If a stylesheet is @imported inside a style rule, declarations from that
// imported sheet are parented by the outer style rule.
var parent = _parent.parent == null ? _importParent : _parent;

if ((_styleRule == null && !_inUnknownAtRule && !_inKeyframes) ||
parent == null) {
throw _exception(
"Declarations may only be used within style rules.",
node.span,
Expand All @@ -1334,14 +1345,14 @@ final class _EvaluateVisitor
);
}

var siblings = _parent.parent!.children;
var siblings = parent.parent?.children ?? [];
var interleavedRules = <CssStyleRule>[];
if (siblings.last != _parent &&
// Reproduce this condition from [_warn] so that we don't add anything to
// [interleavedRules] for declarations in dependencies.
if (siblings.last != parent &&
// Reproduce this condition from [_warn] so that we don't add anything
// to [interleavedRules] for declarations in dependencies.
!(_quietDeps && _inDependency)) {
loop:
for (var sibling in siblings.skip(siblings.indexOf(_parent) + 1)) {
for (var sibling in siblings.skip(siblings.indexOf(parent) + 1)) {
switch (sibling) {
case CssComment():
continue loop;
Expand Down Expand Up @@ -1387,7 +1398,7 @@ final class _EvaluateVisitor
_isEmptyList(value) ||
// Custom properties are allowed to have empty values, per spec.
name.value.startsWith('--')) {
_parent.addChild(
parent.addChild(
ModifiableCssDeclaration(
name,
CssValue(value, expression.span),
Expand Down Expand Up @@ -1895,6 +1906,7 @@ final class _EvaluateVisitor
_stylesheet = stylesheet;
if (loadsUserDefinedModules) {
_root = ModifiableCssStylesheet(stylesheet.span);
_importParent = _parent;
_parent = _root;
Comment on lines +1909 to 1910
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the issue here is that _parent is reset to _root. What happens if we don't do that, rather than adding an extra variable to track (and then need to un-track) a different type of parent?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this is used to track ordering when doing @import containing @use.

e.g.

// This file is @imported into main.scss
@use 'uptream'
.foo-midstream{}

Results in

.foo-midstream{}
.foo-upstream{}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the existence of _parent cause that?

_endOfImports = 0;
_outOfOrderImports = null;
Expand All @@ -1915,6 +1927,7 @@ final class _EvaluateVisitor
if (loadsUserDefinedModules) {
_root = oldRoot;
_parent = oldParent;
_importParent = null;
_endOfImports = oldEndOfImports;
_outOfOrderImports = oldOutOfOrderImports;
}
Expand Down
29 changes: 21 additions & 8 deletions lib/src/visitor/evaluate.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: a3068d04660dd2bed34b884aa6e1a21d423dc4e5
// Checksum: 0db2da7b7facb3c5c79cfba1d6829d18a5de5cdd
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -216,6 +216,12 @@ final class _EvaluateVisitor

ModifiableCssParentNode? __parent;

/// The original parent node for a stylesheet that was loaded with `@import`.
///
/// This value is only set when a file uses `@import` in combination with
/// non-built-in Sass modules.
ModifiableCssParentNode? _importParent;

/// The name of the current declaration parent.
String? _declarationName;

Expand Down Expand Up @@ -1329,7 +1335,12 @@ final class _EvaluateVisitor
}

Value? visitDeclaration(Declaration node) {
if (_styleRule == null && !_inUnknownAtRule && !_inKeyframes) {
// If a stylesheet is @imported inside a style rule, declarations from that
// imported sheet are parented by the outer style rule.
var parent = _parent.parent == null ? _importParent : _parent;

if ((_styleRule == null && !_inUnknownAtRule && !_inKeyframes) ||
parent == null) {
throw _exception(
"Declarations may only be used within style rules.",
node.span,
Expand All @@ -1342,14 +1353,14 @@ final class _EvaluateVisitor
);
}

var siblings = _parent.parent!.children;
var siblings = parent.parent?.children ?? [];
var interleavedRules = <CssStyleRule>[];
if (siblings.last != _parent &&
// Reproduce this condition from [_warn] so that we don't add anything to
// [interleavedRules] for declarations in dependencies.
if (siblings.last != parent &&
// Reproduce this condition from [_warn] so that we don't add anything
// to [interleavedRules] for declarations in dependencies.
!(_quietDeps && _inDependency)) {
loop:
for (var sibling in siblings.skip(siblings.indexOf(_parent) + 1)) {
for (var sibling in siblings.skip(siblings.indexOf(parent) + 1)) {
switch (sibling) {
case CssComment():
continue loop;
Expand Down Expand Up @@ -1395,7 +1406,7 @@ final class _EvaluateVisitor
_isEmptyList(value) ||
// Custom properties are allowed to have empty values, per spec.
name.value.startsWith('--')) {
_parent.addChild(
parent.addChild(
ModifiableCssDeclaration(
name,
CssValue(value, expression.span),
Expand Down Expand Up @@ -1903,6 +1914,7 @@ final class _EvaluateVisitor
_stylesheet = stylesheet;
if (loadsUserDefinedModules) {
_root = ModifiableCssStylesheet(stylesheet.span);
_importParent = _parent;
_parent = _root;
_endOfImports = 0;
_outOfOrderImports = null;
Expand All @@ -1923,6 +1935,7 @@ final class _EvaluateVisitor
if (loadsUserDefinedModules) {
_root = oldRoot;
_parent = oldParent;
_importParent = null;
_endOfImports = oldEndOfImports;
_outOfOrderImports = oldOutOfOrderImports;
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/sass-parser/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.4.25

* No user-visible changes.

## 0.4.24

* No user-visible changes.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sass-parser/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "sass-parser",
"version": "0.4.24",
"version": "0.4.25",
"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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 15.7.2

* No user-visible changes.

## 15.7.1

* No user-visible changes.
Expand Down
4 changes: 2 additions & 2 deletions pkg/sass_api/pubspec.yaml
View file
Open in desktop
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: 15.7.1
version: 15.7.2
description: Additional APIs for Dart Sass.
homepage: https://github.com/sass/dart-sass

environment:
sdk: ">=3.6.0 <4.0.0"

dependencies:
sass: 1.89.2
sass: 1.89.3

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

Expand Down
Loading