Merge pull request #1309 from sass/fromImport

Add the ability for importers to detect @import

CHANGELOG.md

@@ -1,3 +1,21 @@
+## 1.33.0
+
+### JS API
+
+* The `this` context for importers now has a `fromImport` field, which is `true`
+  if the importer is being invoked from an `@import` and `false` otherwise.
+  Importers should only use this to determine whether to load [import-only
+  files].
+
+[import-only files]: https://sass-lang.com/documentation/at-rules/import#import-only-files
+
+### Dart API
+
+* Add an `Importer.fromImport` getter, which is `true` if the current
+  `Importer.canonicalize()` call comes from an `@import` rule and `false`
+  otherwise. Importers should only use this to determine whether to load
+  [import-only files].
+
 ## 1.32.13
 
 * **Potentially breaking bug fix:** Null values in `@use` and `@forward`

lib/src/importer/async.dart

@@ -4,7 +4,10 @@
 
 import 'dart:async';
 
+import 'package:meta/meta.dart';
+
 import 'result.dart';
+import 'utils.dart' as utils;
 
 /// An interface for importers that resolves URLs in `@import`s to the contents
 /// of Sass files.
@@ -20,6 +23,22 @@ import 'result.dart';
 ///
 /// Subclasses should extend [AsyncImporter], not implement it.
 abstract class AsyncImporter {
+  /// Whether the current [canonicalize] invocation comes from an `@import`
+  /// rule.
+  ///
+  /// When evaluating `@import` rules, URLs should canonicalize to an
+  /// [import-only file] if one exists for the URL being canonicalized.
+  /// Otherwise, canonicalization should be identical for `@import` and `@use`
+  /// rules.
+  ///
+  /// [import-only file]: https://sass-lang.com/documentation/at-rules/import#import-only-files
+  ///
+  /// Subclasses should only access this from within calls to [canonicalize].
+  /// Outside of that context, its value is undefined and subject to change.
+  @protected
+  @nonVirtual
+  bool get fromImport => utils.fromImport;
+
   /// If [url] is recognized by this importer, returns its canonical format.
   ///
   /// Note that canonical URLs *must* be absolute, including a scheme. Returning

lib/src/importer/node/implementation.dart

@@ -13,6 +13,7 @@ import '../../node/function.dart';
 import '../../node/importer_result.dart';
 import '../../node/utils.dart';
 import '../../util/nullable.dart';
+import '../../node/render_context.dart';
 import '../utils.dart';
 
 /// An importer that encapsulates Node Sass's import logic.
@@ -40,8 +41,12 @@ import '../utils.dart';
 ///   4. Filesystem imports relative to an `includePaths` path.
 ///   5. Filesystem imports relative to a `SASS_PATH` path.
 class NodeImporter {
-  /// The `this` context in which importer functions are invoked.
-  final Object _context;
+  /// The options for the `this` context in which importer functions are
+  /// invoked.
+  ///
+  /// This is typed as [Object] because the public interface of [NodeImporter]
+  /// is shared with the VM, which can't handle JS interop types.
+  final Object _options;
 
   /// The include paths passed in by the user.
   final List<String> _includePaths;
@@ -50,7 +55,7 @@ class NodeImporter {
   final List<JSFunction> _importers;
 
   NodeImporter(
-      this._context, Iterable<String> includePaths, Iterable<Object> importers)
+      this._options, Iterable<String> includePaths, Iterable<Object> importers)
       : _includePaths = List.unmodifiable(_addSassPath(includePaths)),
         _importers = List.unmodifiable(importers.cast());
 
@@ -78,7 +83,8 @@ class NodeImporter {
     // The previous URL is always an absolute file path for filesystem imports.
     var previousString = _previousToString(previous);
     for (var importer in _importers) {
-      var value = call2(importer, _context, url, previousString);
+      var value =
+          call2(importer, _renderContext(forImport), url, previousString);
       if (value != null) {
         return _handleImportResult(url, previous, value, forImport);
       }
@@ -103,7 +109,8 @@ class NodeImporter {
     // The previous URL is always an absolute file path for filesystem imports.
     var previousString = _previousToString(previous);
     for (var importer in _importers) {
-      var value = await _callImporterAsync(importer, url, previousString);
+      var value =
+          await _callImporterAsync(importer, url, previousString, forImport);
       if (value != null) {
         return _handleImportResult(url, previous, value, forImport);
       }
@@ -193,13 +200,21 @@ class NodeImporter {
   }
 
   /// Calls an importer that may or may not be asynchronous.
-  Future<Object?> _callImporterAsync(
-      JSFunction importer, String url, String previousString) async {
+  Future<Object?> _callImporterAsync(JSFunction importer, String url,
+      String previousString, bool forImport) async {
     var completer = Completer<Object>();
 
-    var result = call3(importer, _context, url, previousString,
+    var result = call3(importer, _renderContext(forImport), url, previousString,
         allowInterop(completer.complete));
     if (isUndefined(result)) return await completer.future;
     return result;
   }
+
+  /// Returns the [RenderContext] in which to invoke importers.
+  RenderContext _renderContext(bool fromImport) {
+    var context = RenderContext(
+        options: _options as RenderContextOptions, fromImport: fromImport);
+    context.options.context = context;
+    return context;
+  }
 }

lib/src/importer/node/interface.dart

@@ -5,7 +5,7 @@
 import 'package:tuple/tuple.dart';
 
 class NodeImporter {
-  NodeImporter(Object context, Iterable<String> includePaths,
+  NodeImporter(Object options, Iterable<String> includePaths,
       Iterable<Object> importers);
 
   Tuple2<String, String>? load(String url, Uri? previous, bool forImport) =>

lib/src/importer/utils.dart

@@ -2,6 +2,8 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import 'dart:async';
+
 import 'package:path/path.dart' as p;
 
 import '../io.dart';
@@ -13,30 +15,12 @@ import '../io.dart';
 /// canonicalization should be identical for `@import` and `@use` rules. It's
 /// admittedly hacky to set this globally, but `@import` will eventually be
 /// removed, at which point we can delete this and have one consistent behavior.
-bool _inImportRule = false;
-
-/// Runs [callback] in a context where [resolveImportPath] uses `@import`
-/// semantics rather than `@use` semantics.
-T inImportRule<T>(T callback()) {
-  var wasInImportRule = _inImportRule;
-  _inImportRule = true;
-  try {
-    return callback();
-  } finally {
-    _inImportRule = wasInImportRule;
-  }
-}
+bool get fromImport => Zone.current[#_inImportRule] as bool? ?? false;
 
-/// Like [inImportRule], but asynchronous.
-Future<T> inImportRuleAsync<T>(Future<T> callback()) async {
-  var wasInImportRule = _inImportRule;
-  _inImportRule = true;
-  try {
-    return await callback();
-  } finally {
-    _inImportRule = wasInImportRule;
-  }
-}
+/// Runs [callback] in a context where [inImportRule] returns `true` and
+/// [resolveImportPath] uses `@import` semantics rather than `@use` semantics.
+T inImportRule<T>(T callback()) =>
+    runZoned(callback, zoneValues: {#_inImportRule: true});
 
 /// Resolves an imported path using the same logic as the filesystem importer.
 ///
@@ -95,7 +79,7 @@ String? _exactlyOne(List<String> paths) {
       paths.map((path) => "  " + p.prettyUri(p.toUri(path))).join("\n");
 }
 
-/// If [_inImportRule] is `true`, invokes callback and returns the result.
+/// If [fromImport] is `true`, invokes callback and returns the result.
 ///
 /// Otherwise, returns `null`.
-T? _ifInImport<T>(T callback()) => _inImportRule ? callback() : null;
+T? _ifInImport<T>(T callback()) => fromImport ? callback() : null;

lib/src/node.dart

@@ -204,7 +204,8 @@ List<AsyncCallable> _parseFunctions(RenderOptions options, DateTime start,
           'Invalid signature "$signature": ${error.message}', error.span);
     }
 
-    var context = _contextWithOptions(options, start);
+    var context = RenderContext(options: _contextOptions(options, start));
+    context.options.context = context;
 
     var fiber = options.fiber;
     if (fiber != null) {
@@ -261,9 +262,8 @@ NodeImporter _parseImporter(RenderOptions options, DateTime start) {
     importers = [options.importer as JSFunction];
   }
 
-  var context = importers.isNotEmpty
-      ? _contextWithOptions(options, start)
-      : const Object();
+  var contextOptions =
+      importers.isNotEmpty ? _contextOptions(options, start) : Object();
 
   var fiber = options.fiber;
   if (fiber != null) {
@@ -288,29 +288,26 @@ NodeImporter _parseImporter(RenderOptions options, DateTime start) {
   }
 
   var includePaths = List<String>.from(options.includePaths ?? []);
-  return NodeImporter(context, includePaths, importers);
+  return NodeImporter(contextOptions, includePaths, importers);
 }
 
-/// Creates a `this` context that contains the render options.
-RenderContext _contextWithOptions(RenderOptions options, DateTime start) {
+/// Creates the [RenderContextOptions] for the `this` context in which custom
+/// functions and importers will be evaluated.
+RenderContextOptions _contextOptions(RenderOptions options, DateTime start) {
   var includePaths = List<String>.from(options.includePaths ?? []);
-  var context = RenderContext(
-      options: RenderContextOptions(
-          file: options.file,
-          data: options.data,
-          includePaths:
-              ([p.current, ...includePaths]).join(isWindows ? ';' : ':'),
-          precision: SassNumber.precision,
-          style: 1,
-          indentType: options.indentType == 'tab' ? 1 : 0,
-          indentWidth: _parseIndentWidth(options.indentWidth) ?? 2,
-          linefeed: _parseLineFeed(options.linefeed).text,
-          result: RenderContextResult(
-              stats: RenderContextResultStats(
-                  start: start.millisecondsSinceEpoch,
-                  entry: options.file ?? 'data'))));
-  context.options.context = context;
-  return context;
+  return RenderContextOptions(
+      file: options.file,
+      data: options.data,
+      includePaths: ([p.current, ...includePaths]).join(isWindows ? ';' : ':'),
+      precision: SassNumber.precision,
+      style: 1,
+      indentType: options.indentType == 'tab' ? 1 : 0,
+      indentWidth: _parseIndentWidth(options.indentWidth) ?? 2,
+      linefeed: _parseLineFeed(options.linefeed).text,
+      result: RenderContextResult(
+          stats: RenderContextResultStats(
+              start: start.millisecondsSinceEpoch,
+              entry: options.file ?? 'data')));
 }
 
 /// Parse [style] into an [OutputStyle].

lib/src/node/render_context.dart

@@ -8,8 +8,10 @@ import 'package:js/js.dart';
 @anonymous
 class RenderContext {
   external RenderContextOptions get options;
+  external bool? get fromImport;
 
-  external factory RenderContext({required RenderContextOptions options});
+  external factory RenderContext(
+      {required RenderContextOptions options, bool? fromImport});
 }
 
 @JS()

lib/src/parse/sass.dart

@@ -414,7 +414,7 @@ class SassParser extends StylesheetParser {
     do {
       containsTab = false;
       containsSpace = false;
-       nextIndentation = 0;
+      nextIndentation = 0;
 
       while (true) {
         var next = scanner.peekChar();

pubspec.yaml

@@ -1,5 +1,5 @@
 name: sass
-version: 1.32.13
+version: 1.33.0-dev
 description: A Sass implementation in Dart.
 author: Sass Team
 homepage: https://github.com/sass/dart-sass

test/dart_api/from_import_importer.dart

@@ -0,0 +1,28 @@
+// Copyright 2017 Google Inc. Use of this source code is governed by an
+// MIT-style license that can be found in the LICENSE file or at
+// https://opensource.org/licenses/MIT.
+
+import 'package:sass/sass.dart';
+import 'package:test/test.dart';
+
+/// An [Importer] whose [canonicalize] method asserts the value of
+/// [Importer.fromImport].
+class FromImportImporter extends Importer {
+  /// The expected value of [Importer.fromImport] in the call to [canonicalize].
+  final bool _expected;
+
+  /// The callback to call once [canonicalize] is called.
+  ///
+  /// This ensures that the test doesn't exit until [canonicalize] is called.
+  final void Function() _done;
+
+  FromImportImporter(this._expected) : _done = expectAsync0(() {});
+
+  Uri? canonicalize(Uri url) {
+    expect(fromImport, equals(_expected));
+    _done();
+    return Uri.parse('u:');
+  }
+
+  ImporterResult? load(Uri url) => ImporterResult("", syntax: Syntax.scss);
+}

test/dart_api/importer_test.dart

@@ -12,6 +12,7 @@ import 'package:test/test.dart';
 import 'package:sass/sass.dart';
 import 'package:sass/src/exception.dart';
 
+import 'from_import_importer.dart';
 import 'test_importer.dart';
 import '../utils.dart';
 
@@ -182,4 +183,23 @@ void main() {
       return true;
     })));
   });
+
+  group("currentLoadFromImport is", () {
+    test("true from an @import", () {
+      compileString('@import "foo"', importers: [FromImportImporter(true)]);
+    });
+
+    test("false from a @use", () {
+      compileString('@use "foo"', importers: [FromImportImporter(false)]);
+    });
+
+    test("false from a @forward", () {
+      compileString('@forward "foo"', importers: [FromImportImporter(false)]);
+    });
+
+    test("false from meta.load-css", () {
+      compileString('@use "sass:meta"; @include meta.load-css("")',
+          importers: [FromImportImporter(false)]);
+    });
+  });
 }