Merge pull request #1526 from sass/file-importer-result

Replace FileImporterResult with a plain URL
sass · Oct 9, 2021 · 978e5e4 · 978e5e4
2 parents d667843 + 2d56fe0
commit 978e5e4
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 122 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -36,12 +36,9 @@ jobs:
       - uses: dart-lang/setup-dart@v1
         with: {sdk: "${{ matrix.dart_channel }}"}
       - run: dart pub get
-      - name: Set up sass-spec
-        run: tool/github-action/check-out-sass-spec.sh
-        env:
-          PR_BRANCH: "${{ github.base_ref }}"
-          CURRENT_REF: "${{ github.ref }}"
-          PR_BODY: "${{ github.event.pull_request.body }}"
+      - name: Check out sass-spec
+        uses: sass/clone-linked-repo@v1
+        with: {repo: sass/sass-spec}
       - uses: actions/setup-node@v2
         with: {node-version: "${{ env.DEFAULT_NODE_VERSION }}"}
       - run: npm install
@@ -86,11 +83,8 @@ jobs:
       - run: npm install
 
       - name: Check out sass-spec
-        run: tool/github-action/check-out-sass-spec.sh
-        env:
-          PR_BRANCH: "${{ github.base_ref }}"
-          CURRENT_REF: "${{ github.ref }}"
-          PR_BODY: "${{ github.event.pull_request.body }}"
+        uses: sass/clone-linked-repo@v1
+        with: {repo: sass/sass-spec}
 
       - name: Build JS
         run: dart pub run grinder pkg-npm-dev
@@ -100,7 +94,10 @@ jobs:
         working-directory: sass-spec
 
       - name: Check out Sass specification
-        run: git clone https://github.com/sass/sass.git language --depth 1
+        uses: sass/clone-linked-repo@v1
+        with:
+          repo: sass/sass
+          path: language
 
       - name: Run tests
         run: npm run js-api-spec -- --sassSassRepo ../language

diff --git a/lib/src/importer/node_to_dart/async_file.dart b/lib/src/importer/node_to_dart/async_file.dart
@@ -6,12 +6,10 @@ import 'dart:async';
 
 import 'package:node_interop/js.dart';
 import 'package:node_interop/util.dart';
-import 'package:path/path.dart' as p;
 
-import '../../io.dart' as io;
 import '../../node/importer.dart';
+import '../../node/url.dart';
 import '../../node/utils.dart';
-import '../../syntax.dart';
 import '../async.dart';
 import '../filesystem.dart';
 import '../result.dart';
@@ -29,9 +27,6 @@ class NodeToDartAsyncFileImporter extends AsyncImporter {
   /// The wrapped `findFileUrl` function.
   final Object? Function(String, CanonicalizeOptions) _findFileUrl;
 
-  /// A map from canonical URLs to the `sourceMapUrl`s associated with them.
-  final _sourceMapUrls = <Uri, Uri>{};
-
   NodeToDartAsyncFileImporter(this._findFileUrl);
 
   FutureOr<Uri?> canonicalize(Uri url) async {
@@ -41,36 +36,21 @@ class NodeToDartAsyncFileImporter extends AsyncImporter {
         url.toString(), CanonicalizeOptions(fromImport: fromImport));
     if (isPromise(result)) result = await promiseToFuture(result as Promise);
     if (result == null) return null;
-
-    result as NodeFileImporterResult;
-    var dartUrl = result.url;
-    var sourceMapUrl = result.sourceMapUrl;
-    if (dartUrl == null) {
-      jsThrow(JsError(
-          "The findFileUrl() method must return an object a url field."));
+    if (!isJSUrl(result)) {
+      jsThrow(JsError("The findFileUrl() method must return a URL."));
     }
 
-    var resultUrl = jsToDartUrl(dartUrl);
+    var resultUrl = jsToDartUrl(result as JSUrl);
     if (resultUrl.scheme != 'file') {
       jsThrow(JsError(
           'The findFileUrl() must return a URL with scheme file://, was '
           '"$url".'));
     }
 
-    var canonical = _filesystemImporter.canonicalize(resultUrl);
-    if (canonical == null) return null;
-    if (sourceMapUrl != null) {
-      _sourceMapUrls[canonical] = jsToDartUrl(sourceMapUrl);
-    }
-
-    return canonical;
+    return _filesystemImporter.canonicalize(resultUrl);
   }
 
-  ImporterResult? load(Uri url) {
-    var path = p.fromUri(url);
-    return ImporterResult(io.readFile(path),
-        sourceMapUrl: _sourceMapUrls[url] ?? url, syntax: Syntax.forPath(path));
-  }
+  ImporterResult? load(Uri url) => _filesystemImporter.load(url);
 
   DateTime modificationTime(Uri url) =>
       _filesystemImporter.modificationTime(url);

diff --git a/lib/src/importer/node_to_dart/file.dart b/lib/src/importer/node_to_dart/file.dart
@@ -3,13 +3,11 @@
 // https://opensource.org/licenses/MIT.
 
 import 'package:node_interop/js.dart';
-import 'package:path/path.dart' as p;
 
-import '../../io.dart' as io;
 import '../../importer.dart';
 import '../../node/importer.dart';
+import '../../node/url.dart';
 import '../../node/utils.dart';
-import '../../syntax.dart';
 import '../filesystem.dart';
 import '../result.dart';
 import '../utils.dart';
@@ -26,9 +24,6 @@ class NodeToDartFileImporter extends Importer {
   /// The wrapped `findFileUrl` function.
   final Object? Function(String, CanonicalizeOptions) _findFileUrl;
 
-  /// A map from canonical URLs to the `sourceMapUrl`s associated with them.
-  final _sourceMapUrls = <Uri, Uri>{};
-
   NodeToDartFileImporter(this._findFileUrl);
 
   Uri? canonicalize(Uri url) {
@@ -40,39 +35,23 @@ class NodeToDartFileImporter extends Importer {
 
     if (isPromise(result)) {
       jsThrow(JsError(
-          "The canonicalize() function can't return a Promise for synchronous "
+          "The findFileUrl() function can't return a Promise for synchron "
           "compile functions."));
+    } else if (!isJSUrl(result)) {
+      jsThrow(JsError("The findFileUrl() method must return a URL."));
     }
 
-    result as NodeFileImporterResult;
-    var dartUrl = result.url;
-    var sourceMapUrl = result.sourceMapUrl;
-    if (dartUrl == null) {
-      jsThrow(JsError(
-          "The findFileUrl() method must return an object a url field."));
-    }
-
-    var resultUrl = jsToDartUrl(dartUrl);
+    var resultUrl = jsToDartUrl(result as JSUrl);
     if (resultUrl.scheme != 'file') {
       jsThrow(JsError(
           'The findFileUrl() must return a URL with scheme file://, was '
           '"$url".'));
     }
 
-    var canonical = _filesystemImporter.canonicalize(resultUrl);
-    if (canonical == null) return null;
-    if (sourceMapUrl != null) {
-      _sourceMapUrls[canonical] = jsToDartUrl(sourceMapUrl);
-    }
-
-    return canonical;
+    return _filesystemImporter.canonicalize(resultUrl);
   }
 
-  ImporterResult? load(Uri url) {
-    var path = p.fromUri(url);
-    return ImporterResult(io.readFile(path),
-        sourceMapUrl: _sourceMapUrls[url] ?? url, syntax: Syntax.forPath(path));
-  }
+  ImporterResult? load(Uri url) => _filesystemImporter.load(url);
 
   DateTime modificationTime(Uri url) =>
       _filesystemImporter.modificationTime(url);

diff --git a/lib/src/node/importer.dart b/lib/src/node/importer.dart
@@ -11,8 +11,7 @@ import 'url.dart';
 class NodeImporter {
   external Object? Function(String, CanonicalizeOptions)? get canonicalize;
   external Object? Function(JSUrl)? get load;
-  external NodeFileImporterResult? Function(String, CanonicalizeOptions)?
-      get findFileUrl;
+  external Object? Function(String, CanonicalizeOptions)? get findFileUrl;
 }
 
 @JS()
@@ -30,10 +29,3 @@ class NodeImporterResult {
   external String? get syntax;
   external JSUrl? get sourceMapUrl;
 }
-
-@JS()
-@anonymous
-class NodeFileImporterResult {
-  external JSUrl? get url;
-  external JSUrl? get sourceMapUrl;
-}
diff --git a/tool/github-action/check-out-sass-spec.sh b/tool/github-action/check-out-sass-spec.sh