Skip to content

Commit

Permalink
Fix import-only files for Node importers
Browse files Browse the repository at this point in the history
  • Loading branch information
jathak committed Jan 3, 2020
1 parent 79d9a73 commit 24e80ca
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 32 deletions.
5 changes: 5 additions & 0 deletions 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
Expand Down
53 changes: 32 additions & 21 deletions lib/src/importer/node/implementation.dart
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -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());
Expand All @@ -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.";
Expand Down
6 changes: 4 additions & 2 deletions lib/src/importer/node/interface.dart
Expand Up @@ -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;
}
8 changes: 4 additions & 4 deletions lib/src/visitor/async_evaluate.dart
Expand Up @@ -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),
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions lib/src/visitor/evaluate.dart
Expand Up @@ -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

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Down
26 changes: 26 additions & 0 deletions test/node_api/importer_test.dart
Expand Up @@ -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(
Expand Down

0 comments on commit 24e80ca

Please sign in to comment.