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

Don't compile .css files in directories to themselves #862

Merged
merged 2 commits into from
Oct 29, 2019
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
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 1.23.2

### Command-Line Interface

* Fix a bug when compiling all Sass files in a directory where a CSS file could
be compiled to its own location, creating an infinite loop in `--watch` mode.

* Properly compile CSS entrypoints in directories outside of `--watch` mode.

## 1.23.1

* Fix a bug preventing built-in modules from being loaded within a configured
Expand Down
6 changes: 4 additions & 2 deletions lib/src/executable/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ class ExecutableOptions {
Map<String, String> _listSourceDirectory(String source, String destination) {
return {
for (var path in listDir(source, recursive: true))
if (_isEntrypoint(path))
if (_isEntrypoint(path) &&
// Don't compile a CSS file to its own location.
!(source == destination && p.extension(path) == '.css'))
path: p.join(destination,
p.setExtension(p.relative(path, from: source), '.css'))
};
Expand All @@ -383,7 +385,7 @@ class ExecutableOptions {
bool _isEntrypoint(String path) {
if (p.basename(path).startsWith("_")) return false;
var extension = p.extension(path);
return extension == ".scss" || extension == ".sass";
return extension == ".scss" || extension == ".sass" || extension == ".css";
}

/// Whether to emit a source map file.
Expand Down
12 changes: 8 additions & 4 deletions lib/src/executable/watch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,14 @@ class _Watcher {
if (p.basename(source).startsWith('_')) return null;

for (var sourceDir in _options.sourceDirectoriesToDestinations.keys) {
if (p.isWithin(sourceDir, source)) {
return p.join(_options.sourceDirectoriesToDestinations[sourceDir],
p.setExtension(p.relative(source, from: sourceDir), '.css'));
}
if (!p.isWithin(sourceDir, source)) continue;

var destination = p.join(
_options.sourceDirectoriesToDestinations[sourceDir],
p.setExtension(p.relative(source, from: sourceDir), '.css'));

// Don't compile ".css" files to their own locations.
if (destination != source) return destination;
}

return null;
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass
version: 1.23.1
version: 1.23.2
description: A Sass implementation in Dart.
author: Sass Team
homepage: https://github.com/sass/dart-sass
Expand Down
16 changes: 14 additions & 2 deletions test/cli/shared/colon_args.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
test("compiles all the stylesheets in the directory", () async {
await d.dir("in", [
d.file("test1.scss", "a {b: c}"),
d.file("test2.sass", "x\n y: z")
d.file("test2.sass", "x\n y: z"),
d.file("test3.css", "q {r: s}")
]).create();

var sass = await runSass(["--no-source-map", "in:out"]);
Expand All @@ -115,7 +116,8 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {

await d.dir("out", [
d.file("test1.css", equalsIgnoringWhitespace("a { b: c; }")),
d.file("test2.css", equalsIgnoringWhitespace("x { y: z; }"))
d.file("test2.css", equalsIgnoringWhitespace("x { y: z; }")),
d.file("test3.css", equalsIgnoringWhitespace("q { r: s; }"))
]).validate();
});

Expand Down Expand Up @@ -183,6 +185,16 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
d.nothing("fake.css")
]).validate();
});

test("ignores a CSS file that would compile to itself", () async {
await d.dir("dir", [d.file("test.css", "a {b: c}")]).create();

var sass = await runSass(["dir:dir"]);
expect(sass.stdout, emitsDone);
await sass.shouldExit(0);

await d.file("dir/test.css", "a {b: c}").validate();
});
});

group("reports all", () {
Expand Down
10 changes: 10 additions & 0 deletions test/cli/shared/update.dart
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,16 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {

await d.file("out.css", contains(message)).validate();
});

test("from itself", () async {
await d.dir("dir", [d.file("test.css", "a {b: c}")]).create();

var sass = await update(["dir:dir"]);
expect(sass.stdout, emitsDone);
await sass.shouldExit(0);

await d.file("dir/test.css", "a {b: c}").validate();
});
});

group("updates a CSS file", () {
Expand Down
23 changes: 23 additions & 0 deletions test/cli/shared/watch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,29 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
await d.nothing("out/test.scss").validate();
});

// Regression test for #853.
test("doesn't try to compile a CSS file to itself", () async {
await d.dir("dir").create();

var sass = await watch(["dir:dir"]);
await expectLater(sass.stdout, _watchingForChanges);
await tickIfPoll();

await d.file("dir/test.css", "a {b: c}").create();
await tick;

// Create a new file that *will* be compiled so that if the first change
// did incorrectly trigger a compilation, it would emit a message
// before the message for this change.
await d.file("dir/test2.scss", "x {y: z}").create();
await expectLater(
sass.stdout, emits(_compiled('dir/test2.scss', 'dir/test2.css')));

await sass.kill();

await d.file("dir/test.css", "a {b: c}").validate();
});

group("doesn't allow", () {
test("--stdin", () async {
var sass = await watch(["--stdin", "test.scss"]);
Expand Down