Fix @at-root bug for common case of built-in use (#1469)

`@at-root` should now work properly in a nested import as long as the
only `@use` or `@forward` rules present are for built-in modules.

This is a partial but not complete fix for #1347. It's a bit hacky to
special case built-in modules here, but since they represent the only
real legitimate case for using this combination of features, it lets us
fix the bug for this case and avoid the bordering-on-infeasible work of
fixing this for user-defined modules.

Author: jathak

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.39.1
+
+* Partial fix for a bug where `@at-root` does not work properly in nested
+  imports that contain `@use` rules. If the only `@use` rules in the nested
+  import are for built-in modules, `@at-root` should now work properly. 
+
 ## 1.39.0
 
 ### JS API

lib/src/visitor/async_evaluate.dart

@@ -1481,6 +1481,15 @@ class _EvaluateVisitor
         return;
       }
 
+      // If only built-in modules are loaded, we still need a separate
+      // environment to ensure their namespaces aren't exposed in the outer
+      // environment, but we don't need to worry about `@extend`s, so we can
+      // add styles directly to the existing stylesheet instead of creating a
+      // new one.
+      var loadsUserDefinedModules =
+          stylesheet.uses.any((rule) => rule.url.scheme != 'sass') ||
+              stylesheet.forwards.any((rule) => rule.url.scheme != 'sass');
+
       late List<ModifiableCssNode> children;
       var environment = _environment.forImport();
       await _withEnvironment(environment, () async {
@@ -1494,10 +1503,12 @@ class _EvaluateVisitor
         var oldInDependency = _inDependency;
         _importer = result.importer;
         _stylesheet = stylesheet;
-        _root = ModifiableCssStylesheet(stylesheet.span);
-        _parent = _root;
-        _endOfImports = 0;
-        _outOfOrderImports = null;
+        if (loadsUserDefinedModules) {
+          _root = ModifiableCssStylesheet(stylesheet.span);
+          _parent = _root;
+          _endOfImports = 0;
+          _outOfOrderImports = null;
+        }
         _inDependency = result.isDependency;
 
         // This configuration is only used if it passes through a `@forward`
@@ -1507,7 +1518,7 @@ class _EvaluateVisitor
         }
 
         await visitStylesheet(stylesheet);
-        children = _addOutOfOrderImports();
+        children = loadsUserDefinedModules ? _addOutOfOrderImports() : [];
 
         _importer = oldImporter;
         _stylesheet = oldStylesheet;
@@ -1525,18 +1536,21 @@ class _EvaluateVisitor
       var module = environment.toDummyModule();
       _environment.importForwards(module);
 
-      if (module.transitivelyContainsCss) {
-        // If any transitively used module contains extensions, we need to clone
-        // all modules' CSS. Otherwise, it's possible that they'll be used or
-        // imported from another location that shouldn't have the same extensions
-        // applied.
-        await _combineCss(module, clone: module.transitivelyContainsExtensions)
-            .accept(this);
-      }
+      if (loadsUserDefinedModules) {
+        if (module.transitivelyContainsCss) {
+          // If any transitively used module contains extensions, we need to
+          // clone all modules' CSS. Otherwise, it's possible that they'll be
+          // used or imported from another location that shouldn't have the same
+          // extensions applied.
+          await _combineCss(module,
+                  clone: module.transitivelyContainsExtensions)
+              .accept(this);
+        }
 
-      var visitor = _ImportedCssVisitor(this);
-      for (var child in children) {
-        child.accept(visitor);
+        var visitor = _ImportedCssVisitor(this);
+        for (var child in children) {
+          child.accept(visitor);
+        }
       }
 
       _activeModules.remove(url);

lib/src/visitor/evaluate.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_evaluate.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: 2d9c3bb53fc29472f7fbc68d9a3f5b8464f95bc5
+// Checksum: 99e7cdfc43a20e2c0827753081db6dcbd914ec75
 //
 // ignore_for_file: unused_import
 
@@ -1481,6 +1481,15 @@ class _EvaluateVisitor
         return;
       }
 
+      // If only built-in modules are loaded, we still need a separate
+      // environment to ensure their namespaces aren't exposed in the outer
+      // environment, but we don't need to worry about `@extend`s, so we can
+      // add styles directly to the existing stylesheet instead of creating a
+      // new one.
+      var loadsUserDefinedModules =
+          stylesheet.uses.any((rule) => rule.url.scheme != 'sass') ||
+              stylesheet.forwards.any((rule) => rule.url.scheme != 'sass');
+
       late List<ModifiableCssNode> children;
       var environment = _environment.forImport();
       _withEnvironment(environment, () {
@@ -1494,10 +1503,12 @@ class _EvaluateVisitor
         var oldInDependency = _inDependency;
         _importer = result.importer;
         _stylesheet = stylesheet;
-        _root = ModifiableCssStylesheet(stylesheet.span);
-        _parent = _root;
-        _endOfImports = 0;
-        _outOfOrderImports = null;
+        if (loadsUserDefinedModules) {
+          _root = ModifiableCssStylesheet(stylesheet.span);
+          _parent = _root;
+          _endOfImports = 0;
+          _outOfOrderImports = null;
+        }
         _inDependency = result.isDependency;
 
         // This configuration is only used if it passes through a `@forward`
@@ -1507,7 +1518,7 @@ class _EvaluateVisitor
         }
 
         visitStylesheet(stylesheet);
-        children = _addOutOfOrderImports();
+        children = loadsUserDefinedModules ? _addOutOfOrderImports() : [];
 
         _importer = oldImporter;
         _stylesheet = oldStylesheet;
@@ -1525,18 +1536,20 @@ class _EvaluateVisitor
       var module = environment.toDummyModule();
       _environment.importForwards(module);
 
-      if (module.transitivelyContainsCss) {
-        // If any transitively used module contains extensions, we need to clone
-        // all modules' CSS. Otherwise, it's possible that they'll be used or
-        // imported from another location that shouldn't have the same extensions
-        // applied.
-        _combineCss(module, clone: module.transitivelyContainsExtensions)
-            .accept(this);
-      }
+      if (loadsUserDefinedModules) {
+        if (module.transitivelyContainsCss) {
+          // If any transitively used module contains extensions, we need to
+          // clone all modules' CSS. Otherwise, it's possible that they'll be
+          // used or imported from another location that shouldn't have the same
+          // extensions applied.
+          _combineCss(module, clone: module.transitivelyContainsExtensions)
+              .accept(this);
+        }
 
-      var visitor = _ImportedCssVisitor(this);
-      for (var child in children) {
-        child.accept(visitor);
+        var visitor = _ImportedCssVisitor(this);
+        for (var child in children) {
+          child.accept(visitor);
+        }
       }
 
       _activeModules.remove(url);

pkg/sass_api/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.0.0-beta.7
+
+* No user-visible changes.
+
 ## 1.0.0-beta.6
 
 * Add the `SassApiColor` extension to the "Value" DartDoc category.

pkg/sass_api/pubspec.yaml

@@ -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: 1.0.0-beta.6
+version: 1.0.0-beta.7
 description: Additional APIs for Dart Sass.
 homepage: https://github.com/sass/dart-sass
 
 environment:
   sdk: '>=2.12.0 <3.0.0'
 
 dependencies:
-  sass: 1.39.0
+  sass: 1.39.1
 
 dependency_overrides:
   sass: {path: ../..}

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.39.0
+version: 1.39.1
 description: A Sass implementation in Dart.
 homepage: https://github.com/sass/dart-sass