Improve importing files through load paths and node_modules

lib/src/migrator.dart

@@ -15,12 +15,13 @@ import 'package:sass/src/import_cache.dart';
 import 'package:args/command_runner.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
-import 'package:sass_migrator/src/util/node_modules_importer.dart';
 import 'package:source_span/source_span.dart';
 
 import 'exception.dart';
 import 'io.dart';
 import 'utils.dart';
+import 'util/node_modules_importer.dart';
+import 'util/reversible_filesystem_importer.dart';
 
 /// A migrator is a command that migrates the entrypoints provided to it and
 /// (optionally) their dependencies.
@@ -74,8 +75,11 @@ abstract class Migrator extends Command<Map<Uri, String>> {
   Map<Uri, String> run() {
     var allMigrated = <Uri, String>{};
     var importer = FilesystemImporter('.');
-    var importCache = ImportCache([NodeModulesImporter()],
-        loadPaths: globalResults['load-path']);
+    var importCache = ImportCache([
+      for (var path in globalResults['load-path'] as List<String>)
+        ReversibleFilesystemImporter(path),
+      NodeModulesImporter()
+    ]);
     for (var entrypoint in argResults.rest) {
       var tuple = importCache.import(Uri.parse(entrypoint), importer);
       if (tuple == null) {

lib/src/migrators/module.dart

@@ -15,15 +15,13 @@ import 'package:args/args.dart';
 import 'package:collection/collection.dart';
 import 'package:path/path.dart' as p;
 import 'package:source_span/source_span.dart';
-import 'package:tuple/tuple.dart';
 
 import '../exception.dart';
 import '../migration_visitor.dart';
 import '../migrator.dart';
 import '../patch.dart';
 import '../utils.dart';
-import '../util/node_modules_importer.dart';
-
+import '../util/reversible_importer.dart';
 import 'module/built_in_functions.dart';
 import 'module/forward_type.dart';
 import 'module/member_declaration.dart';
@@ -121,10 +119,6 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// declaration will be used for namespacing instead of this one.
   final _reassignedVariables = <MemberDeclaration<VariableDeclaration>>{};
 
-  /// Maps canonical URLs to the original URL and importer from the `@import`
-  /// rule that last imported that URL.
-  final _originalImports = <Uri, Tuple2<String, Importer>>{};
-
   /// Tracks members that are unreferencable in the current scope.
   UnreferencableMembers _unreferencable = UnreferencableMembers();
 
@@ -151,6 +145,10 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// Whether @use and @forward are allowed in the current context.
   var _useAllowed = true;
 
+  /// The importer used to load files relative to the entrypoint, as opposed to
+  /// through `node_modules` or load paths.
+  Importer _entrypointImporter;
+
   /// A mapping between member declarations and references.
   ///
   /// This performs an initial pass to determine how a declaration seen in the
@@ -192,6 +190,7 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// migrator.
   @override
   Map<Uri, String> run(Stylesheet stylesheet, Importer importer) {
+    _entrypointImporter = importer;
     references.globalDeclarations.forEach(_renameDeclaration);
     var migrated = super.run(stylesheet, importer);
 
@@ -689,13 +688,8 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
     if (migrateDependencies) visitDependency(parsedUrl, import.span);
     _upstreamStylesheets.remove(currentUrl);
 
-    // Associate the importer for this URL with the resolved URL so that we can
-    // re-use this import URL later on.
-    var tuple = importCache.canonicalize(parsedUrl, importer, currentUrl);
-    var resolvedUrl = tuple.item2;
-    _originalImports.putIfAbsent(
-        resolvedUrl, () => Tuple2(import.url, tuple.item1));
-
+    var resolvedUrl =
+        importCache.canonicalize(parsedUrl, importer, currentUrl).item2;
     var asClause = '';
     if (!_useAllowed) {
       _unreferencable = _unreferencable.parent;
@@ -957,8 +951,11 @@ class _ModuleMigrationVisitor extends MigrationVisitor {
   /// `@use`, `@forward`, or `@import` rule.
   String _absoluteUrlToDependency(Uri url, {Uri relativeTo}) {
     relativeTo ??= currentUrl;
-    var tuple = _originalImports[url];
-    if (tuple?.item2 is NodeModulesImporter) return tuple.item1;
+
+    var importer = references.importers[url];
+    if (importer != _entrypointImporter) {
+      return (importer as ReversibleImporter).decanonicalize(url).toString();
+    }
 
     var loadPathUrls = loadPaths.map((path) => p.toUri(p.absolute(path)));
     var potentialUrls = [

lib/src/util/node_modules_importer.dart

@@ -8,17 +8,23 @@ import 'package:path/path.dart' as p;
 import 'package:sass/sass.dart';
 
 import '../io.dart';
+import '../utils.dart';
+import 'reversible_importer.dart';
 
 /// An importer that resolves URLs starting with `~` by searching in
 /// `node_modules` directories.
 ///
 /// This doesn't completely match Webpack's behavior (as it lacks the support
 /// for configuring its behavior), but it should be good enough to run the
 /// migrator with for most cases.
-class NodeModulesImporter extends Importer {
+class NodeModulesImporter extends Importer implements ReversibleImporter {
   /// Importers that load from various `node_modules` directories.
   final _fsImporters = <FilesystemImporter>[];
 
+  /// The paths to the `node_modules` directories in which this importer
+  /// searches for stylesheets.
+  final _nodeModules = <String>[];
+
   /// Constructs a new [NodeModulesImporter] that searches for `node_modules` in
   /// [baseDirectory] and all of its ancestors.
   ///
@@ -28,6 +34,7 @@ class NodeModulesImporter extends Importer {
     while (true) {
       var loadPath = p.join(directory, 'node_modules');
       if (Directory(loadPath).existsSync()) {
+        _nodeModules.add(loadPath);
         _fsImporters.add(FilesystemImporter(loadPath));
       }
       var parent = p.dirname(directory);
@@ -56,6 +63,15 @@ class NodeModulesImporter extends Importer {
     return null;
   }
 
+  Uri decanonicalize(Uri url) {
+    var path = p.fromUri(url);
+    for (var dir in _nodeModules) {
+      if (!p.isWithin(dir, path)) continue;
+      return cleanBasename(p.toUri('~' + p.relative(path, from: dir)));
+    }
+    throw ArgumentError("Couldn't find $url in any node_modules directory.");
+  }
+
   /// Loads [url] using a [FilesystemImporter].
   ///
   /// [url] must be the canonical URL returned by [canonicalize].

lib/src/util/reversible_filesystem_importer.dart

@@ -0,0 +1,34 @@
+// Copyright 2019 Google LLC
+//
+// 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:path/path.dart' as p;
+import 'package:sass/sass.dart';
+
+import '../utils.dart';
+import 'reversible_importer.dart';
+
+/// A version of [FilesystemImporter] that implements [ReversibleImporter] as
+/// well.
+class ReversibleFilesystemImporter extends Importer
+    implements ReversibleImporter {
+  /// The wrapped importer to which this delegates.
+  final FilesystemImporter _inner;
+
+  /// The path relative to which this importer looks for files.
+  final String _loadPath;
+
+  ReversibleFilesystemImporter(this._loadPath)
+      : _inner = FilesystemImporter(_loadPath);
+
+  Uri decanonicalize(Uri canonicalUrl) => cleanBasename(
+      p.toUri(p.relative(p.fromUri(canonicalUrl), from: _loadPath)));
+
+  Uri canonicalize(Uri url) => _inner.canonicalize(url);
+
+  ImporterResult load(Uri canonicalUrl) => _inner.load(canonicalUrl);
+
+  String toString() => _inner.toString();
+}

lib/src/util/reversible_importer.dart

@@ -0,0 +1,18 @@
+// Copyright 2019 Google LLC
+//
+// 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';
+
+/// An [Importer] interface that supports converting canonical URLs back into
+/// their non-canonical forms.
+abstract class ReversibleImporter implements Importer {
+  /// Converts [canonicalUrl] into a non-canonical form.
+  ///
+  /// Callers should only call this with [canonicalUrl]s returned by this
+  /// importer. Implementors must guarantee that calling
+  /// `canonicalize(decanonicalize(url))` will return the original `url`.
+  Uri decanonicalize(Uri canonicalUrl);
+}

lib/src/utils.dart

@@ -5,6 +5,7 @@
 // https://opensource.org/licenses/MIT.
 
 import 'package:charcode/charcode.dart';
+import 'package:path/path.dart' as p;
 import 'package:source_span/source_span.dart';
 import 'package:tuple/tuple.dart';
 
@@ -195,6 +196,14 @@ Uri getImportOnlyUrl(Uri url) {
   return url.resolve('$basename.import.$extension');
 }
 
+/// Removes a non-canonical URL's basename's initial `_` and extension.
+Uri cleanBasename(Uri url) {
+  var string = url.toString();
+  var basename = p.url.basenameWithoutExtension(string);
+  if (basename.startsWith("_")) basename = basename.substring(1);
+  return Uri.parse(p.url.join(p.url.dirname(string), basename));
+}
+
 /// Partitions [iterable] into two lists based on the types of its inputs.
 ///
 /// This asserts that every element in [iterable] is either an `F` or a `G`, and