Emit comments in source order where possible (#1989)

nex3 · web-flow · commit e9e44d7edbb3 · 2023-06-23T15:52:06.000-07:00
See sass/sass#2848 Closes #1939
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+## 1.64.0
+
+* Comments that appear before or between `@use` and `@forward` rules are now
+  emitted in source order as much as possible, instead of always being emitted
+  after the CSS of all module dependencies.
+
 ## 1.63.6
 
 ### JavaScript API
diff --git a/lib/src/ast/css/modifiable/node.dart b/lib/src/ast/css/modifiable/node.dart
@@ -81,4 +81,13 @@ abstract class ModifiableCssParentNode extends ModifiableCssNode
     child._indexInParent = _children.length;
     _children.add(child);
   }
+
+  /// Destructively removes all elements from [children].
+  void clearChildren() {
+    for (var child in _children) {
+      child._parent = null;
+      child._indexInParent = null;
+    }
+    _children.clear();
+  }
 }
diff --git a/lib/src/async_environment.dart b/lib/src/async_environment.dart
@@ -802,11 +802,14 @@ class AsyncEnvironment {
   }
 
   /// Returns a module that represents the top-level members defined in [this],
-  /// that contains [css] as its CSS tree, which can be extended using
-  /// [extensionStore].
-  Module toModule(CssStylesheet css, ExtensionStore extensionStore) {
+  /// that contains [css] and [preModuleComments] as its CSS, which can be
+  /// extended using [extensionStore].
+  Module toModule(
+      CssStylesheet css,
+      Map<Module, List<CssComment>> preModuleComments,
+      ExtensionStore extensionStore) {
     assert(atRoot);
-    return _EnvironmentModule(this, css, extensionStore,
+    return _EnvironmentModule(this, css, preModuleComments, extensionStore,
         forwarded: _forwardedModules.andThen((modules) => MapKeySet(modules)));
   }
 
@@ -821,6 +824,7 @@ class AsyncEnvironment {
         this,
         CssStylesheet(const [],
             SourceFile.decoded(const [], url: "<dummy module>").span(0)),
+        const {},
         ExtensionStore.empty,
         forwarded: _forwardedModules.andThen((modules) => MapKeySet(modules)));
   }
@@ -902,6 +906,7 @@ class _EnvironmentModule implements Module {
   final Map<String, AsyncCallable> mixins;
   final ExtensionStore extensionStore;
   final CssStylesheet css;
+  final Map<Module, List<CssComment>> preModuleComments;
   final bool transitivelyContainsCss;
   final bool transitivelyContainsExtensions;
 
@@ -916,13 +921,20 @@ class _EnvironmentModule implements Module {
   /// defined at all.
   final Map<String, Module> _modulesByVariable;
 
-  factory _EnvironmentModule(AsyncEnvironment environment, CssStylesheet css,
+  factory _EnvironmentModule(
+      AsyncEnvironment environment,
+      CssStylesheet css,
+      Map<Module, List<CssComment>> preModuleComments,
       ExtensionStore extensionStore,
       {Set<Module>? forwarded}) {
     forwarded ??= const {};
     return _EnvironmentModule._(
         environment,
         css,
+        Map.unmodifiable({
+          for (var entry in preModuleComments.entries)
+            entry.key: List<CssComment>.unmodifiable(entry.value)
+        }),
         extensionStore,
         _makeModulesByVariable(forwarded),
         _memberMap(environment._variables.first,
@@ -934,6 +946,7 @@ class _EnvironmentModule implements Module {
         _memberMap(environment._mixins.first,
             forwarded.map((module) => module.mixins)),
         transitivelyContainsCss: css.children.isNotEmpty ||
+            preModuleComments.isNotEmpty ||
             environment._allModules
                 .any((module) => module.transitivelyContainsCss),
         transitivelyContainsExtensions: !extensionStore.isEmpty ||
@@ -981,6 +994,7 @@ class _EnvironmentModule implements Module {
   _EnvironmentModule._(
       this._environment,
       this.css,
+      this.preModuleComments,
       this.extensionStore,
       this._modulesByVariable,
       this.variables,
@@ -1020,6 +1034,7 @@ class _EnvironmentModule implements Module {
     return _EnvironmentModule._(
         _environment,
         newCssAndExtensionStore.item1,
+        preModuleComments,
         newCssAndExtensionStore.item2,
         _modulesByVariable,
         variables,
diff --git a/lib/src/environment.dart b/lib/src/environment.dart
@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_environment.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: 38c688423116df1e489aa6eafc16de1bf9bc2bf5
+// Checksum: 487c34d7387b6f368ed60ff2db6a748483aba2a1
 //
 // ignore_for_file: unused_import
 
@@ -808,11 +808,14 @@ class Environment {
   }
 
   /// Returns a module that represents the top-level members defined in [this],
-  /// that contains [css] as its CSS tree, which can be extended using
-  /// [extensionStore].
-  Module<Callable> toModule(CssStylesheet css, ExtensionStore extensionStore) {
+  /// that contains [css] and [preModuleComments] as its CSS, which can be
+  /// extended using [extensionStore].
+  Module<Callable> toModule(
+      CssStylesheet css,
+      Map<Module<Callable>, List<CssComment>> preModuleComments,
+      ExtensionStore extensionStore) {
     assert(atRoot);
-    return _EnvironmentModule(this, css, extensionStore,
+    return _EnvironmentModule(this, css, preModuleComments, extensionStore,
         forwarded: _forwardedModules.andThen((modules) => MapKeySet(modules)));
   }
 
@@ -827,6 +830,7 @@ class Environment {
         this,
         CssStylesheet(const [],
             SourceFile.decoded(const [], url: "<dummy module>").span(0)),
+        const {},
         ExtensionStore.empty,
         forwarded: _forwardedModules.andThen((modules) => MapKeySet(modules)));
   }
@@ -909,6 +913,7 @@ class _EnvironmentModule implements Module<Callable> {
   final Map<String, Callable> mixins;
   final ExtensionStore extensionStore;
   final CssStylesheet css;
+  final Map<Module<Callable>, List<CssComment>> preModuleComments;
   final bool transitivelyContainsCss;
   final bool transitivelyContainsExtensions;
 
@@ -924,12 +929,19 @@ class _EnvironmentModule implements Module<Callable> {
   final Map<String, Module<Callable>> _modulesByVariable;
 
   factory _EnvironmentModule(
-      Environment environment, CssStylesheet css, ExtensionStore extensionStore,
+      Environment environment,
+      CssStylesheet css,
+      Map<Module<Callable>, List<CssComment>> preModuleComments,
+      ExtensionStore extensionStore,
       {Set<Module<Callable>>? forwarded}) {
     forwarded ??= const {};
     return _EnvironmentModule._(
         environment,
         css,
+        Map.unmodifiable({
+          for (var entry in preModuleComments.entries)
+            entry.key: List<CssComment>.unmodifiable(entry.value)
+        }),
         extensionStore,
         _makeModulesByVariable(forwarded),
         _memberMap(environment._variables.first,
@@ -941,6 +953,7 @@ class _EnvironmentModule implements Module<Callable> {
         _memberMap(environment._mixins.first,
             forwarded.map((module) => module.mixins)),
         transitivelyContainsCss: css.children.isNotEmpty ||
+            preModuleComments.isNotEmpty ||
             environment._allModules
                 .any((module) => module.transitivelyContainsCss),
         transitivelyContainsExtensions: !extensionStore.isEmpty ||
@@ -989,6 +1002,7 @@ class _EnvironmentModule implements Module<Callable> {
   _EnvironmentModule._(
       this._environment,
       this.css,
+      this.preModuleComments,
       this.extensionStore,
       this._modulesByVariable,
       this.variables,
@@ -1028,6 +1042,7 @@ class _EnvironmentModule implements Module<Callable> {
     return _EnvironmentModule._(
         _environment,
         newCssAndExtensionStore.item1,
+        preModuleComments,
         newCssAndExtensionStore.item2,
         _modulesByVariable,
         variables,
diff --git a/lib/src/module.dart b/lib/src/module.dart
@@ -53,6 +53,10 @@ abstract class Module<T extends AsyncCallable> {
   /// The module's CSS tree.
   CssStylesheet get css;
 
+  /// A map from modules in [upstream] to loud comments written in this module
+  /// that should be emitted before the given module.
+  Map<Module<T>, List<CssComment>> get preModuleComments;
+
   /// Whether this module *or* any modules in [upstream] contain any CSS.
   bool get transitivelyContainsCss;
 
diff --git a/lib/src/module/built_in.dart b/lib/src/module/built_in.dart
@@ -23,6 +23,7 @@ class BuiltInModule<T extends AsyncCallable> implements Module<T> {
   Map<String, AstNode> get variableNodes => const {};
   ExtensionStore get extensionStore => ExtensionStore.empty;
   CssStylesheet get css => CssStylesheet.empty(url: url);
+  Map<Module<T>, List<CssComment>> get preModuleComments => const {};
   bool get transitivelyContainsCss => false;
   bool get transitivelyContainsExtensions => false;
 
diff --git a/lib/src/module/forwarded_view.dart b/lib/src/module/forwarded_view.dart
@@ -25,6 +25,8 @@ class ForwardedModuleView<T extends AsyncCallable> implements Module<T> {
   List<Module<T>> get upstream => _inner.upstream;
   ExtensionStore get extensionStore => _inner.extensionStore;
   CssStylesheet get css => _inner.css;
+  Map<Module<T>, List<CssComment>> get preModuleComments =>
+      _inner.preModuleComments;
   bool get transitivelyContainsCss => _inner.transitivelyContainsCss;
   bool get transitivelyContainsExtensions =>
       _inner.transitivelyContainsExtensions;
diff --git a/lib/src/module/shadowed_view.dart b/lib/src/module/shadowed_view.dart
@@ -22,6 +22,8 @@ class ShadowedModuleView<T extends AsyncCallable> implements Module<T> {
   List<Module<T>> get upstream => _inner.upstream;
   ExtensionStore get extensionStore => _inner.extensionStore;
   CssStylesheet get css => _inner.css;
+  Map<Module<T>, List<CssComment>> get preModuleComments =>
+      _inner.preModuleComments;
   bool get transitivelyContainsCss => _inner.transitivelyContainsCss;
   bool get transitivelyContainsExtensions =>
       _inner.transitivelyContainsExtensions;
diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart
diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart
diff --git a/lib/src/visitor/serialize.dart b/lib/src/visitor/serialize.dart
diff --git a/pubspec.yaml b/pubspec.yaml

Original file line number	Diff line number	Diff line change
`@@ -81,4 +81,13 @@ abstract class ModifiableCssParentNode extends ModifiableCssNode`
`81`	`81`	`child._indexInParent = _children.length;`
`82`	`82`	`_children.add(child);`
`83`	`83`	`}`
	`84`	`+`
	`85`	`+ /// Destructively removes all elements from [children].`
	`86`	`+ void clearChildren() {`
	`87`	`+ for (var child in _children) {`
	`88`	`+ child._parent = null;`
	`89`	`+ child._indexInParent = null;`
	`90`	`+ }`
	`91`	`+ _children.clear();`
	`92`	`+ }`
`84`	`93`	`}`