Fix import-only files for Node importers (#919)

Author: jathak

CHANGELOG.md

@@ -1,3 +1,8 @@
+## 1.24.2
+
+* Fix a bug introduced in the previous release that prevented custom importers
+  in Node.js from loading import-only files.
+
 ## 1.24.1
 
 * Fix a bug where the wrong file could be loaded when the same URL is used by

lib/src/importer/node/implementation.dart

@@ -67,10 +67,10 @@ class NodeImporter {
   /// The [previous] URL is the URL of the stylesheet in which the import
   /// appeared. Returns the contents of the stylesheet and the URL to use as
   /// [previous] for imports within the loaded stylesheet.
-  Tuple2<String, String> load(String url, Uri previous) {
+  Tuple2<String, String> load(String url, Uri previous, bool forImport) {
     var parsed = Uri.parse(url);
     if (parsed.scheme == '' || parsed.scheme == 'file') {
-      var result = _resolveRelativePath(p.fromUri(parsed), previous);
+      var result = _resolveRelativePath(p.fromUri(parsed), previous, forImport);
       if (result != null) return result;
     }
 
@@ -79,21 +79,24 @@ class NodeImporter {
         previous.scheme == 'file' ? p.fromUri(previous) : previous.toString();
     for (var importer in _importers) {
       var value = call2(importer, _context, url, previousString);
-      if (value != null) return _handleImportResult(url, previous, value);
+      if (value != null) {
+        return _handleImportResult(url, previous, value, forImport);
+      }
     }
 
-    return _resolveLoadPathFromUrl(parsed, previous);
+    return _resolveLoadPathFromUrl(parsed, previous, forImport);
   }
 
   /// Asynchronously loads the stylesheet at [url].
   ///
   /// The [previous] URL is the URL of the stylesheet in which the import
   /// appeared. Returns the contents of the stylesheet and the URL to use as
   /// [previous] for imports within the loaded stylesheet.
-  Future<Tuple2<String, String>> loadAsync(String url, Uri previous) async {
+  Future<Tuple2<String, String>> loadAsync(
+      String url, Uri previous, bool forImport) async {
     var parsed = Uri.parse(url);
     if (parsed.scheme == '' || parsed.scheme == 'file') {
-      var result = _resolveRelativePath(p.fromUri(parsed), previous);
+      var result = _resolveRelativePath(p.fromUri(parsed), previous, forImport);
       if (result != null) return result;
     }
 
@@ -102,22 +105,26 @@ class NodeImporter {
         previous.scheme == 'file' ? p.fromUri(previous) : previous.toString();
     for (var importer in _importers) {
       var value = await _callImporterAsync(importer, url, previousString);
-      if (value != null) return _handleImportResult(url, previous, value);
+      if (value != null) {
+        return _handleImportResult(url, previous, value, forImport);
+      }
     }
 
-    return _resolveLoadPathFromUrl(parsed, previous);
+    return _resolveLoadPathFromUrl(parsed, previous, forImport);
   }
 
   /// Tries to load a stylesheet at the given [path] relative to [previous].
   ///
   /// Returns the stylesheet at that path and the URL used to load it, or `null`
   /// if loading failed.
-  Tuple2<String, String> _resolveRelativePath(String path, Uri previous) {
-    if (p.isAbsolute(path)) return _tryPath(path);
+  Tuple2<String, String> _resolveRelativePath(
+      String path, Uri previous, bool forImport) {
+    if (p.isAbsolute(path)) return _tryPath(path, forImport);
 
     // 1: Filesystem imports relative to the base file.
     if (previous.scheme == 'file') {
-      var result = _tryPath(p.join(p.dirname(p.fromUri(previous)), path));
+      var result =
+          _tryPath(p.join(p.dirname(p.fromUri(previous)), path), forImport);
       if (result != null) return result;
     }
     return null;
@@ -128,24 +135,26 @@ class NodeImporter {
   ///
   /// Returns the stylesheet at that path and the URL used to load it, or `null`
   /// if loading failed.
-  Tuple2<String, String> _resolveLoadPathFromUrl(Uri url, Uri previous) =>
+  Tuple2<String, String> _resolveLoadPathFromUrl(
+          Uri url, Uri previous, bool forImport) =>
       url.scheme == '' || url.scheme == 'file'
-          ? _resolveLoadPath(p.fromUri(url), previous)
+          ? _resolveLoadPath(p.fromUri(url), previous, forImport)
           : null;
 
   /// Tries to load a stylesheet at the given [path] from a load path (including
   /// the working directory).
   ///
   /// Returns the stylesheet at that path and the URL used to load it, or `null`
   /// if loading failed.
-  Tuple2<String, String> _resolveLoadPath(String path, Uri previous) {
+  Tuple2<String, String> _resolveLoadPath(
+      String path, Uri previous, bool forImport) {
     // 2: Filesystem imports relative to the working directory.
-    var cwdResult = _tryPath(p.absolute(path));
+    var cwdResult = _tryPath(p.absolute(path), forImport);
     if (cwdResult != null) return cwdResult;
 
     // 3: Filesystem imports relative to [_includePaths].
     for (var includePath in _includePaths) {
-      var result = _tryPath(p.absolute(p.join(includePath, path)));
+      var result = _tryPath(p.absolute(p.join(includePath, path)), forImport);
       if (result != null) return result;
     }
 
@@ -156,8 +165,10 @@ class NodeImporter {
   ///
   /// Returns the stylesheet at that path and the URL used to load it, or `null`
   /// if loading failed.
-  Tuple2<String, String> _tryPath(String path) {
-    var resolved = resolveImportPath(path);
+  Tuple2<String, String> _tryPath(String path, bool forImport) {
+    var resolved = forImport
+        ? inImportRule(() => resolveImportPath(path))
+        : resolveImportPath(path);
     return resolved == null
         ? null
         : Tuple2(readFile(resolved), p.toUri(resolved).toString());
@@ -166,14 +177,14 @@ class NodeImporter {
   /// Converts an importer's return [value] to a tuple that can be returned by
   /// [load].
   Tuple2<String, String> _handleImportResult(
-      String url, Uri previous, Object value) {
+      String url, Uri previous, Object value, bool forImport) {
     if (isJSError(value)) throw value;
     if (value is! NodeImporterResult) return null;
 
     var result = value as NodeImporterResult;
     if (result.file != null) {
-      var resolved = _resolveRelativePath(result.file, previous) ??
-          _resolveLoadPath(result.file, previous);
+      var resolved = _resolveRelativePath(result.file, previous, forImport) ??
+          _resolveLoadPath(result.file, previous, forImport);
       if (resolved != null) return resolved;
 
       throw "Can't find stylesheet to import.";

lib/src/importer/node/interface.dart

@@ -10,7 +10,9 @@ class NodeImporter {
   NodeImporter(Object context, Iterable<String> includePaths,
       Iterable<Object> importers);
 
-  Tuple2<String, String> load(String url, Uri previous) => null;
+  Tuple2<String, String> load(String url, Uri previous, bool forImport) => null;
 
-  Future<Tuple2<String, String>> loadAsync(String url, Uri previous) => null;
+  Future<Tuple2<String, String>> loadAsync(
+          String url, Uri previous, bool forImport) =>
+      null;
 }

lib/src/visitor/async_evaluate.dart

@@ -1410,7 +1410,7 @@ class _EvaluateVisitor
       _importSpan = span;
 
       if (_nodeImporter != null) {
-        var stylesheet = await _importLikeNode(url);
+        var stylesheet = await _importLikeNode(url, forImport);
         if (stylesheet != null) return Tuple2(null, stylesheet);
       } else {
         var tuple = await _importCache.import(Uri.parse(url),
@@ -1445,9 +1445,9 @@ class _EvaluateVisitor
   /// Imports a stylesheet using [_nodeImporter].
   ///
   /// Returns the [Stylesheet], or `null` if the import failed.
-  Future<Stylesheet> _importLikeNode(String originalUrl) async {
-    var result =
-        await _nodeImporter.loadAsync(originalUrl, _stylesheet.span?.sourceUrl);
+  Future<Stylesheet> _importLikeNode(String originalUrl, bool forImport) async {
+    var result = await _nodeImporter.loadAsync(
+        originalUrl, _stylesheet.span?.sourceUrl, forImport);
     if (result == null) return null;
 
     var contents = result.item1;

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: e3d7bea705bed417db9925546c076caed4465b2e
+// Checksum: 98bd4418d21d4f005485e343869b458b44841450
 //
 // ignore_for_file: unused_import
 
@@ -1407,7 +1407,7 @@ class _EvaluateVisitor
       _importSpan = span;
 
       if (_nodeImporter != null) {
-        var stylesheet = _importLikeNode(url);
+        var stylesheet = _importLikeNode(url, forImport);
         if (stylesheet != null) return Tuple2(null, stylesheet);
       } else {
         var tuple = _importCache.import(Uri.parse(url),
@@ -1442,8 +1442,9 @@ class _EvaluateVisitor
   /// Imports a stylesheet using [_nodeImporter].
   ///
   /// Returns the [Stylesheet], or `null` if the import failed.
-  Stylesheet _importLikeNode(String originalUrl) {
-    var result = _nodeImporter.load(originalUrl, _stylesheet.span?.sourceUrl);
+  Stylesheet _importLikeNode(String originalUrl, bool forImport) {
+    var result =
+        _nodeImporter.load(originalUrl, _stylesheet.span?.sourceUrl, forImport);
     if (result == null) return null;
 
     var contents = result.item1;

pubspec.yaml

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

test/node_api/importer_test.dart

@@ -165,6 +165,32 @@ void main() {
           equalsIgnoringWhitespace('a { b: c; }'));
     });
 
+    test("supports import-only files", () async {
+      await writeTextFile(p.join(sandbox, 'target.scss'), 'a {b: regular}');
+      await writeTextFile(
+          p.join(sandbox, 'target.import.scss'), 'a {b: import-only}');
+
+      expect(
+          renderSync(RenderOptions(
+              data: "@import 'foo'",
+              importer: allowInterop((void _, void __) =>
+                  NodeImporterResult(file: p.join(sandbox, 'target.scss'))))),
+          equalsIgnoringWhitespace('a { b: import-only; }'));
+    });
+
+    test("supports mixed `@use` and `@import`", () async {
+      await writeTextFile(p.join(sandbox, 'target.scss'), 'a {b: regular}');
+      await writeTextFile(
+          p.join(sandbox, 'target.import.scss'), 'a {b: import-only}');
+
+      expect(
+          renderSync(RenderOptions(
+              data: "@use 'foo'; @import 'foo';",
+              importer: allowInterop((void _, void __) =>
+                  NodeImporterResult(file: p.join(sandbox, 'target.scss'))))),
+          equalsIgnoringWhitespace('a { b: regular; } a { b: import-only; }'));
+    });
+
     test("may be extensionless", () async {
       expect(
           renderSync(RenderOptions(

test/node_api_test.dart

@@ -77,6 +77,26 @@ void main() {
           equalsIgnoringWhitespace('a { b: c; }'));
     });
 
+    test("supports import-only files", () async {
+      await writeTextFile(p.join(sandbox, 'foo.scss'), 'a {b: regular}');
+      await writeTextFile(
+          p.join(sandbox, 'foo.import.scss'), 'a {b: import-only}');
+
+      runTestInSandbox();
+      expect(renderSync(RenderOptions(data: "@import 'foo'")),
+          equalsIgnoringWhitespace('a { b: import-only; }'));
+    });
+
+    test("supports mixed `@use` and `@import`", () async {
+      await writeTextFile(p.join(sandbox, 'foo.scss'), 'a {b: regular}');
+      await writeTextFile(
+          p.join(sandbox, 'foo.import.scss'), 'a {b: import-only}');
+
+      runTestInSandbox();
+      expect(renderSync(RenderOptions(data: "@use 'foo'; @import 'foo';")),
+          equalsIgnoringWhitespace('a { b: regular; } a { b: import-only; }'));
+    });
+
     test("renders a string", () {
       expect(renderSync(RenderOptions(data: "a {b: c}")),
           equalsIgnoringWhitespace('a { b: c; }'));