Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SASS_PATH #514

Merged
merged 5 commits into from
Nov 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:/sass/language/blob/master/accepted/at-rule-interpolation.md

### JavaScript API
Expand Down
6 changes: 6 additions & 0 deletions lib/sass.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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].
Expand Down
14 changes: 14 additions & 0 deletions lib/src/async_import_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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].
Expand All @@ -64,12 +68,22 @@ class AsyncImportCache {
static List<AsyncImporter> _toImporters(Iterable<AsyncImporter> importers,
Iterable<String> 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;
}

Expand Down
16 changes: 15 additions & 1 deletion lib/src/import_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
// 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;
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
Expand Down Expand Up @@ -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].
Expand All @@ -67,12 +71,22 @@ class ImportCache {
static List<Importer> _toImporters(Iterable<Importer> importers,
Iterable<String> 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;
}

Expand Down
12 changes: 11 additions & 1 deletion lib/src/importer/node/implementation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -48,9 +49,18 @@ class NodeImporter {
final List<JSFunction> _importers;

NodeImporter(this._context, Iterable<String> 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<String> _addSassPath(Iterable<String> 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
Expand Down
4 changes: 4 additions & 0 deletions lib/src/io/interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ Iterable<String> 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;

Expand Down
5 changes: 5 additions & 0 deletions lib/src/io/node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -59,6 +60,7 @@ class _SystemError {
@JS()
class _Process {
external String get platform;
external Object get env;
external String cwd();
}

Expand Down Expand Up @@ -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>(T callback()) {
Expand Down
3 changes: 3 additions & 0 deletions lib/src/io/vm.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ DateTime modificationTime(String path) {
return stat.modified;
}

String getEnvironmentVariable(String name) =>
io.Platform.environment['SASS_PATH'];

Future<Stream<WatchEvent>> watchDir(String path, {bool poll: false}) async {
var watcher =
poll ? new PollingDirectoryWatcher(path) : new DirectoryWatcher(path);
Expand Down
4 changes: 3 additions & 1 deletion test/cli/dart_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ void ensureSnapshotUpToDate() {
}
}

Future<TestProcess> runSass(Iterable<String> arguments) {
Future<TestProcess> runSass(Iterable<String> arguments,
{Map<String, String> environment}) {
var executable = _snapshotPaths.firstWhere(
(path) => new File(path).existsSync(),
orElse: () => p.absolute("bin/sass.dart"));
Expand All @@ -61,5 +62,6 @@ Future<TestProcess> runSass(Iterable<String> arguments) {
..add(executable)
..addAll(arguments),
workingDirectory: d.sandbox,
environment: environment,
description: "sass");
}
10 changes: 7 additions & 3 deletions test/cli/node_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ void main() {
});
}

Future<TestProcess> runSass(Iterable<String> arguments) => TestProcess.start(
"node", [p.absolute("build/npm/sass.js")]..addAll(arguments),
workingDirectory: d.sandbox, description: "sass");
Future<TestProcess> runSass(Iterable<String> arguments,
{Map<String, String> environment}) =>
TestProcess.start(
"node", [p.absolute("build/npm/sass.js")]..addAll(arguments),
workingDirectory: d.sandbox,
environment: environment,
description: "sass");
49 changes: 46 additions & 3 deletions test/cli/shared.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,24 @@
// https://opensource.org/licenses/MIT.

import 'dart:async';
import 'dart:io';

import 'package:path/path.dart' as p;
import 'package:test/test.dart';
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<TestProcess> runSass(Iterable<String> arguments)) {
void sharedTests(
Future<TestProcess> runSass(Iterable<String> arguments,
{Map<String, String> environment})) {
/// Runs the executable on [arguments] plus an output file, then verifies that
/// the contents of the output file match [expected].
Future expectCompiles(List<String> arguments, expected) async {
Future expectCompiles(List<String> arguments, expected,
{Map<String, String> 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();
}
Expand Down Expand Up @@ -123,6 +128,21 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> 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 {
Expand Down Expand Up @@ -161,6 +181,29 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> 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", """
Expand Down
2 changes: 2 additions & 0 deletions test/dart_api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 8 additions & 0 deletions test/node_api/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
40 changes: 40 additions & 0 deletions test/node_api_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 "
Expand Down