Skip to content

Commit

Permalink
Don't read file if Node importer returns file and contents.
Browse files Browse the repository at this point in the history
Instead, use `contents` with `file` as the canonical url.

Closes #975.
  • Loading branch information
Awjin committed Apr 25, 2020
1 parent 7160b64 commit 21a1c58
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
* Add `sass.NULL`, `sass.TRUE`, and `sass.FALSE` constants to match Node Sass's
API.

* If a custom Node importer returns both `file` and `contents`, don't attempt to
read the `file`. Instead, use the `contents` provided by the importer, with
`file` as the canonical url.

## 1.26.5

* No user-visible changes.
Expand Down
9 changes: 5 additions & 4 deletions lib/src/importer/node/implementation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,15 @@ class NodeImporter {
if (value is! NodeImporterResult) return null;

var result = value as NodeImporterResult;
if (result.file != null) {
if (result.file == null) {
return Tuple2(result.contents ?? '', url);
} else if (result.contents != null) {
return Tuple2(result.contents, result.file);
} else {
var resolved = _resolveRelativePath(result.file, previous, forImport) ??
_resolveLoadPath(result.file, previous, forImport);
if (resolved != null) return resolved;

throw "Can't find stylesheet to import.";
} else {
return Tuple2(result.contents ?? '', url);
}
}

Expand Down
38 changes: 29 additions & 9 deletions test/node_api/importer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,6 @@ void main() {
await writeTextFile(sassPath, 'a {b: c}');
});

test("can import a file by contents", () {
expect(
renderSync(RenderOptions(
data: "@import 'foo'",
importer: allowInterop((void _, void __) =>
NodeImporterResult(contents: 'a {b: c}')))),
equalsIgnoringWhitespace('a { b: c; }'));
});

test("imports cascade through importers", () {
expect(
renderSync(RenderOptions(data: "@import 'foo'", importer: [
Expand Down Expand Up @@ -121,6 +112,35 @@ void main() {
});
});

group("with contents", () {
test("imports a file by contents", () {
expect(
renderSync(RenderOptions(
data: "@import 'foo'",
importer: allowInterop((void _, void __) =>
NodeImporterResult(contents: 'a {b: c}')))),
equalsIgnoringWhitespace('a { b: c; }'));
});

test("contents take precedence over file name", () {
expect(
renderSync(RenderOptions(
data: "@import 'foo'",
importer: allowInterop((void _, void __) =>
NodeImporterResult(contents: 'x {y: z}', file: sassPath)))),
equalsIgnoringWhitespace('x { y: z; }'));
});

test("contents use file name as canonical url", () {
var result = sass.renderSync(RenderOptions(
data: "@import 'foo'",
importer: allowInterop(expectAsync2((void _, void __) {
return NodeImporterResult(contents: '', file: 'bar');
}))));
expect(result.stats.includedFiles, equals(['bar']));
});
});

group("with a file redirect", () {
test("imports the chosen file", () {
expect(
Expand Down

0 comments on commit 21a1c58

Please sign in to comment.