From 77563be0565b3e1b9805ef798b18c3fd39ee2a6a Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 5 Nov 2018 15:24:14 -0800 Subject: [PATCH] Add support for SASS_PATH (#514) Closes #512 --- CHANGELOG.md | 4 ++ lib/sass.dart | 6 +++ lib/src/async_import_cache.dart | 14 +++++++ lib/src/import_cache.dart | 16 +++++++- lib/src/importer/node/implementation.dart | 12 +++++- lib/src/io/interface.dart | 4 ++ lib/src/io/node.dart | 5 +++ lib/src/io/vm.dart | 3 ++ test/cli/dart_test.dart | 4 +- test/cli/node_test.dart | 10 +++-- test/cli/shared.dart | 49 +++++++++++++++++++++-- test/dart_api_test.dart | 2 + test/node_api/utils.dart | 8 ++++ test/node_api_test.dart | 40 ++++++++++++++++++ 14 files changed, 168 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13f3608e..28dd1f29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ * Add support for interpolation in at-rule names. See [the proposal][at-rule-interpolation] for details. +* Add paths from the `SASS_PATH` environment variable to the load paths in the + command-line interface, Dart API, and JS API. These load paths are checked + just after the load paths explicitly passed by the user. + [at-rule-interpolation]: https://github.com/sass/language/blob/master/accepted/at-rule-interpolation.md ### JavaScript API diff --git a/lib/sass.dart b/lib/sass.dart index 749b7bc3..482edc35 100644 --- a/lib/sass.dart +++ b/lib/sass.dart @@ -44,6 +44,9 @@ export 'src/visitor/serialize.dart' show OutputStyle; /// * Each load path in [loadPaths]. Note that this is a shorthand for adding /// [FilesystemImporter]s to [importers]. /// +/// * Each load path specified in the `SASS_PATH` environment variable, which +/// should be semicolon-separated on Windows and colon-separated elsewhere. +/// /// * `package:` resolution using [packageResolver], which is a /// [`SyncPackageResolver`][] from the `package_resolver` package. Note that /// this is a shorthand for adding a [PackageImporter] to [importers]. @@ -114,6 +117,9 @@ String compile(String path, /// * Each load path in [loadPaths]. Note that this is a shorthand for adding /// [FilesystemImporter]s to [importers]. /// +/// * Each load path specified in the `SASS_PATH` environment variable, which +/// should be semicolon-separated on Windows and colon-separated elsewhere. +/// /// * `package:` resolution using [packageResolver], which is a /// [`SyncPackageResolver`][] from the `package_resolver` package. Note that /// this is a shorthand for adding a [PackageImporter] to [importers]. diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 2f1a6303..907c2cc6 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -10,6 +10,7 @@ import 'package:tuple/tuple.dart'; import 'ast/sass.dart'; import 'importer.dart'; +import 'io.dart'; import 'logger.dart'; import 'sync_package_resolver.dart'; import 'utils.dart'; // ignore: unused_import @@ -45,6 +46,9 @@ class AsyncImportCache { /// * Each load path in [loadPaths]. Note that this is a shorthand for adding /// [FilesystemImporter]s to [importers]. /// + /// * Each load path specified in the `SASS_PATH` environment variable, which + /// should be semicolon-separated on Windows and colon-separated elsewhere. + /// /// * `package:` resolution using [packageResolver], which is a /// [`SyncPackageResolver`][] from the `package_resolver` package. Note that /// this is a shorthand for adding a [PackageImporter] to [importers]. @@ -64,12 +68,22 @@ class AsyncImportCache { static List _toImporters(Iterable importers, Iterable loadPaths, SyncPackageResolver packageResolver) { var list = importers?.toList() ?? []; + if (loadPaths != null) { list.addAll(loadPaths.map((path) => new FilesystemImporter(path))); } + + var sassPath = getEnvironmentVariable('SASS_PATH'); + if (sassPath != null) { + list.addAll(sassPath + .split(isWindows ? ';' : ':') + .map((path) => new FilesystemImporter(path))); + } + if (packageResolver != null) { list.add(new PackageImporter(packageResolver)); } + return list; } diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index 3afc3e29..5838a8c3 100644 --- a/lib/src/import_cache.dart +++ b/lib/src/import_cache.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_import_cache.dart. // See tool/synchronize.dart for details. // -// Checksum: 57c42546fb8e0b68e29ea841ba106ee99127bede +// Checksum: 3fd08a2b9ad7226a0e3b34057eede0595b6d5aa4 import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; @@ -13,6 +13,7 @@ import 'package:tuple/tuple.dart'; import 'ast/sass.dart'; import 'importer.dart'; +import 'io.dart'; import 'logger.dart'; import 'sync_package_resolver.dart'; import 'utils.dart'; // ignore: unused_import @@ -48,6 +49,9 @@ class ImportCache { /// * Each load path in [loadPaths]. Note that this is a shorthand for adding /// [FilesystemImporter]s to [importers]. /// + /// * Each load path specified in the `SASS_PATH` environment variable, which + /// should be semicolon-separated on Windows and colon-separated elsewhere. + /// /// * `package:` resolution using [packageResolver], which is a /// [`SyncPackageResolver`][] from the `package_resolver` package. Note that /// this is a shorthand for adding a [PackageImporter] to [importers]. @@ -67,12 +71,22 @@ class ImportCache { static List _toImporters(Iterable importers, Iterable loadPaths, SyncPackageResolver packageResolver) { var list = importers?.toList() ?? []; + if (loadPaths != null) { list.addAll(loadPaths.map((path) => new FilesystemImporter(path))); } + + var sassPath = getEnvironmentVariable('SASS_PATH'); + if (sassPath != null) { + list.addAll(sassPath + .split(isWindows ? ';' : ':') + .map((path) => new FilesystemImporter(path))); + } + if (packageResolver != null) { list.add(new PackageImporter(packageResolver)); } + return list; } diff --git a/lib/src/importer/node/implementation.dart b/lib/src/importer/node/implementation.dart index c2f739de..7fe664dd 100644 --- a/lib/src/importer/node/implementation.dart +++ b/lib/src/importer/node/implementation.dart @@ -36,6 +36,7 @@ import '../utils.dart'; /// 1. Filesystem imports relative to the base file. /// 2. Filesystem imports relative to the working directory. /// 3. Filesystem imports relative to an `includePaths` path. +/// 3. Filesystem imports relative to a `SASS_PATH` path. /// 4. Custom importer imports. class NodeImporter { /// The `this` context in which importer functions are invoked. @@ -48,9 +49,18 @@ class NodeImporter { final List _importers; NodeImporter(this._context, Iterable includePaths, Iterable importers) - : _includePaths = new List.unmodifiable(includePaths), + : _includePaths = new List.unmodifiable(_addSassPath(includePaths)), _importers = new List.unmodifiable(importers); + /// Returns [includePaths] followed by any paths loaded from the `SASS_PATH` + /// environment variable. + static Iterable _addSassPath(Iterable includePaths) sync* { + yield* includePaths; + var sassPath = getEnvironmentVariable("SASS_PATH"); + if (sassPath == null) return; + yield* sassPath.split(isWindows ? ';' : ':'); + } + /// Loads the stylesheet at [url]. /// /// The [previous] URL is the URL of the stylesheet in which the import diff --git a/lib/src/io/interface.dart b/lib/src/io/interface.dart index c539a13c..d079a2e5 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -83,6 +83,10 @@ Iterable listDir(String path) => null; /// Returns the modification time of the file at [path]. DateTime modificationTime(String path) => null; +/// Returns the value of the environment variable with the given [name], or +/// `null` if it's not set. +String getEnvironmentVariable(String name) => null; + /// Gets and sets the exit code that the process will use when it exits. int exitCode; diff --git a/lib/src/io/node.dart b/lib/src/io/node.dart index d1378196..35fb035c 100644 --- a/lib/src/io/node.dart +++ b/lib/src/io/node.dart @@ -4,6 +4,7 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:js_util'; import 'package:js/js.dart'; import 'package:path/path.dart' as p; @@ -59,6 +60,7 @@ class _SystemError { @JS() class _Process { external String get platform; + external Object get env; external String cwd(); } @@ -204,6 +206,9 @@ DateTime modificationTime(String path) => _systemErrorToFileSystemException( () => new DateTime.fromMillisecondsSinceEpoch( _fs.statSync(path).mtime.getTime())); +String getEnvironmentVariable(String name) => + getProperty(_process.env, name) as String; + /// Runs callback and converts any [_SystemError]s it throws into /// [FileSystemException]s. T _systemErrorToFileSystemException(T callback()) { diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index 766517c3..876b61fb 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -83,6 +83,9 @@ DateTime modificationTime(String path) { return stat.modified; } +String getEnvironmentVariable(String name) => + io.Platform.environment['SASS_PATH']; + Future> watchDir(String path, {bool poll: false}) async { var watcher = poll ? new PollingDirectoryWatcher(path) : new DirectoryWatcher(path); diff --git a/test/cli/dart_test.dart b/test/cli/dart_test.dart index b7cd2304..f28f7ece 100644 --- a/test/cli/dart_test.dart +++ b/test/cli/dart_test.dart @@ -45,7 +45,8 @@ void ensureSnapshotUpToDate() { } } -Future runSass(Iterable arguments) { +Future runSass(Iterable arguments, + {Map environment}) { var executable = _snapshotPaths.firstWhere( (path) => new File(path).existsSync(), orElse: () => p.absolute("bin/sass.dart")); @@ -61,5 +62,6 @@ Future runSass(Iterable arguments) { ..add(executable) ..addAll(arguments), workingDirectory: d.sandbox, + environment: environment, description: "sass"); } diff --git a/test/cli/node_test.dart b/test/cli/node_test.dart index e85dcfb6..e832868a 100644 --- a/test/cli/node_test.dart +++ b/test/cli/node_test.dart @@ -30,6 +30,10 @@ void main() { }); } -Future runSass(Iterable arguments) => TestProcess.start( - "node", [p.absolute("build/npm/sass.js")]..addAll(arguments), - workingDirectory: d.sandbox, description: "sass"); +Future runSass(Iterable arguments, + {Map environment}) => + TestProcess.start( + "node", [p.absolute("build/npm/sass.js")]..addAll(arguments), + workingDirectory: d.sandbox, + environment: environment, + description: "sass"); diff --git a/test/cli/shared.dart b/test/cli/shared.dart index 54ae68fe..5586f634 100644 --- a/test/cli/shared.dart +++ b/test/cli/shared.dart @@ -3,6 +3,7 @@ // https://opensource.org/licenses/MIT. import 'dart:async'; +import 'dart:io'; import 'package:path/path.dart' as p; import 'package:test/test.dart'; @@ -10,12 +11,16 @@ import 'package:test_descriptor/test_descriptor.dart' as d; import 'package:test_process/test_process.dart'; /// Defines test that are shared between the Dart and Node.js CLI test suites. -void sharedTests(Future runSass(Iterable arguments)) { +void sharedTests( + Future runSass(Iterable arguments, + {Map environment})) { /// Runs the executable on [arguments] plus an output file, then verifies that /// the contents of the output file match [expected]. - Future expectCompiles(List arguments, expected) async { + Future expectCompiles(List arguments, expected, + {Map environment}) async { var sass = await runSass( - arguments.toList()..add("out.css")..add("--no-source-map")); + arguments.toList()..add("out.css")..add("--no-source-map"), + environment: environment); await sass.shouldExit(0); await d.file("out.css", expected).validate(); } @@ -123,6 +128,21 @@ void sharedTests(Future runSass(Iterable arguments)) { equalsIgnoringWhitespace("a { b: c; }")); }); + test("from SASS_PATH", () async { + await d.file("test.scss", """ + @import 'test2'; + @import 'test3'; + """).create(); + + await d.dir("dir2", [d.file("test2.scss", "a {b: c}")]).create(); + await d.dir("dir3", [d.file("test3.scss", "x {y: z}")]).create(); + + var separator = Platform.isWindows ? ';' : ':'; + await expectCompiles( + ["test.scss"], equalsIgnoringWhitespace("a { b: c; } x { y: z; }"), + environment: {"SASS_PATH": "dir2${separator}dir3"}); + }); + // Regression test for #369 test("from within a directory, relative to a file on the load path", () async { @@ -161,6 +181,29 @@ void sharedTests(Future runSass(Iterable arguments)) { equalsIgnoringWhitespace("x { y: z; }")); }); + test("from the load path in preference to from SASS_PATH", () async { + await d.file("test.scss", "@import 'test2'").create(); + + await d.dir("dir1", [d.file("test2.scss", "a {b: c}")]).create(); + await d.dir("dir2", [d.file("test2.scss", "x {y: z}")]).create(); + + await expectCompiles(["--load-path", "dir2", "test.scss"], + equalsIgnoringWhitespace("x { y: z; }"), + environment: {"SASS_PATH": "dir1"}); + }); + + test("in SASS_PATH order", () async { + await d.file("test.scss", "@import 'test2'").create(); + + await d.dir("dir1", [d.file("test2.scss", "a {b: c}")]).create(); + await d.dir("dir2", [d.file("test2.scss", "x {y: z}")]).create(); + + var separator = Platform.isWindows ? ';' : ':'; + await expectCompiles( + ["test.scss"], equalsIgnoringWhitespace("x { y: z; }"), + environment: {"SASS_PATH": "dir2${separator}dir3"}); + }); + // Regression test for an internal Google issue. test("multiple times from different load paths", () async { await d.file("test.scss", """ diff --git a/test/dart_api_test.dart b/test/dart_api_test.dart index bf37c1cb..cbaa0c62 100644 --- a/test/dart_api_test.dart +++ b/test/dart_api_test.dart @@ -13,6 +13,8 @@ import 'package:sass/sass.dart'; import 'package:sass/src/exception.dart'; main() { + // TODO(nweiz): test SASS_PATH when dart-lang/sdk#28160 is fixed. + group("importers", () { test("is used to resolve imports", () async { await d.dir("subdir", [d.file("subtest.scss", "a {b: c}")]).create(); diff --git a/test/node_api/utils.dart b/test/node_api/utils.dart index c588553b..ccf279fa 100644 --- a/test/node_api/utils.dart +++ b/test/node_api/utils.dart @@ -15,6 +15,9 @@ import 'package:sass/src/node/function.dart'; import '../hybrid.dart'; import 'api.dart'; +@JS('process.env') +external Object get _environment; + String sandbox; void useSandbox() { @@ -97,3 +100,8 @@ void runTestInSandbox() { chdir(sandbox); addTearDown(() => chdir(oldWorkingDirectory)); } + +/// Sets the environment variable [name] to [value] within this process. +void setEnvironmentVariable(String name, String value) { + setProperty(_environment, name, value); +} diff --git a/test/node_api_test.dart b/test/node_api_test.dart index 52dcfb32..422c28bc 100644 --- a/test/node_api_test.dart +++ b/test/node_api_test.dart @@ -12,6 +12,7 @@ import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'package:sass/src/node/utils.dart'; +import 'package:sass/src/io.dart'; import 'ensure_npm_package.dart'; import 'hybrid.dart'; @@ -95,6 +96,45 @@ void main() { equalsIgnoringWhitespace('a { b: c; }')); }); + test("supports SASS_PATH", () async { + await createDirectory(p.join(sandbox, 'dir1')); + await createDirectory(p.join(sandbox, 'dir2')); + await writeTextFile(p.join(sandbox, 'dir1', 'test1.scss'), 'a {b: c}'); + await writeTextFile(p.join(sandbox, 'dir2', 'test2.scss'), 'x {y: z}'); + + var separator = isWindows ? ';' : ':'; + setEnvironmentVariable("SASS_PATH", + p.join(sandbox, 'dir1') + separator + p.join(sandbox, 'dir2')); + + try { + expect(renderSync(new RenderOptions(data: """ + @import 'test1'; + @import 'test2'; + """)), equalsIgnoringWhitespace('a { b: c; } x { y: z; }')); + } finally { + setEnvironmentVariable("SASS_PATH", null); + } + }); + + test("load path takes precedence over SASS_PATH", () async { + await createDirectory(p.join(sandbox, 'dir1')); + await createDirectory(p.join(sandbox, 'dir2')); + await writeTextFile(p.join(sandbox, 'dir1', 'test.scss'), 'a {b: c}'); + await writeTextFile(p.join(sandbox, 'dir2', 'test.scss'), 'x {y: z}'); + + setEnvironmentVariable("SASS_PATH", p.join(sandbox, 'dir1')); + + try { + expect( + renderSync(new RenderOptions( + data: "@import 'test'", + includePaths: [p.join(sandbox, 'dir2')])), + equalsIgnoringWhitespace('x { y: z; }')); + } finally { + setEnvironmentVariable("SASS_PATH", null); + } + }); + // Regression test for #314 test( "a file imported through a relative load path supports relative "