Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 24 additions & 1 deletion lib/src/executable/compile_stylesheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Future<void> _compileStylesheetWithoutErrorHandling(
String? destination, {
bool ifModified = false,
}) async {
var compilationStarted = DateTime.now();
var importer = FilesystemImporter.cwd;
if (ifModified) {
try {
Expand Down Expand Up @@ -192,18 +193,25 @@ Future<void> _compileStylesheetWithoutErrorHandling(
} else {
ensureDir(p.dirname(destination));
writeFile(destination, "${error.toCssString()}\n");
_trySetModificationTime(destination, compilationStarted);
}
}
rethrow;
}

var css = result.css;
css += _writeSourceMap(options, result.sourceMap, destination);
css += _writeSourceMap(
options,
result.sourceMap,
destination,
compilationStarted,
);
if (destination == null) {
if (css.isNotEmpty) print(css);
} else {
ensureDir(p.dirname(destination));
writeFile(destination, "$css\n");
_trySetModificationTime(destination, compilationStarted);
}

if (options.quiet || (!options.update && !options.watch)) return;
Expand Down Expand Up @@ -238,6 +246,7 @@ String _writeSourceMap(
ExecutableOptions options,
SingleMapping? sourceMap,
String? destination,
DateTime compilationStarted,
) {
if (sourceMap == null) return "";

Expand Down Expand Up @@ -268,6 +277,7 @@ String _writeSourceMap(
var sourceMapPath = '${destination!}.map';
ensureDir(p.dirname(sourceMapPath));
writeFile(sourceMapPath, sourceMapText);
_trySetModificationTime(sourceMapPath, compilationStarted);

url = p.toUri(p.relative(sourceMapPath, from: p.dirname(destination)));
}
Expand All @@ -289,6 +299,19 @@ void _tryDelete(String path) {
}
}

/// Backdate [path] to [time], ignoring failures.
///
/// Outputs keep the time compilation started so that sources modified during
/// compilation compare as newer. Best effort on filesystems with coarse
/// granularity.
void _trySetModificationTime(String path, DateTime time) {
try {
setModificationTime(path, time);
} on FileSystemException {
// The output is still usable with its actual modification time.
}
}

/// Return a Record of `(exitCode, error, stackTrace)` for the given error.
(int, String, String?) _getErrorWithStackTrace(
int exitCode,
Expand Down
3 changes: 3 additions & 0 deletions lib/src/io/interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ String realpath(String path) => throw '';
/// Returns the modification time of the file at [path].
DateTime modificationTime(String path) => throw '';

/// Sets the modification time of the file at [path] to [time].
void setModificationTime(String path, DateTime time) => throw '';

/// Returns the value of the environment variable with the given [name], or
/// `null` if it's not set.
String? getEnvironmentVariable(String name) => throw '';
Expand Down
12 changes: 12 additions & 0 deletions lib/src/io/js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,18 @@ DateTime modificationTime(String path) {
);
}

void setModificationTime(String path, DateTime time) {
if (!isNodeJs) {
throw UnsupportedError(
"setModificationTime() is only supported on Node.js",
);
}
return _systemErrorToFileSystemException(() {
var seconds = time.millisecondsSinceEpoch / 1000;
fs.utimesSync(path, seconds, seconds);
});
}

String? getEnvironmentVariable(String name) {
var env = _process?.env;
return env == null ? null : getProperty(env as Object, name) as String?;
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 @@ -101,6 +101,9 @@ DateTime modificationTime(String path) {
return stat.modified;
}

void setModificationTime(String path, DateTime time) =>
io.File(path).setLastModifiedSync(time);

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

Future<Stream<WatchEvent>> watchDir(String path, {bool poll = false}) async {
Expand Down
58 changes: 58 additions & 0 deletions test/cli/shared/update.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,52 @@ void sharedTests(
.validate();
});

// Regression test for #2849.
test("a source modified during a compilation", () async {
await d.file("_slow.scss", _slowSource).create();
await d.file("_other.scss", "a {b: c}").create();
await d.file("test.scss", "@use 'slow'; @use 'other'").create();

var watcher = await runSass([
"--no-source-map",
"--watch",
"test.scss:out.css",
]);
await expectLater(
watcher.stdout,
emits(endsWith('Compiled test.scss to out.css.')),
);
await expectLater(
watcher.stdout,
emitsInOrder([
"Sass is watching for changes. Press Ctrl-C to stop.",
"",
]),
);

// Modifying the slow stylesheet starts a compilation that runs for
// several seconds. The delay is short enough to land while that
// compilation is still in progress.
await d.file("_slow.scss", "$_slowSource\n// modified").create();
await Future<void>.delayed(Duration(seconds: 3));
await d.file("_other.scss", "x {y: z}").create();

// Stop once the compilation that missed the second modification is
// written, leaving the CSS on disk one generation behind.
await expectLater(
watcher.stdout,
emits(endsWith('Compiled test.scss to out.css.')),
);
await watcher.kill();

// A later `--update` must pick up the missed generation.
var sass = await update(["test.scss:out.css"]);
expect(sass.stdout, emits(endsWith('Compiled test.scss to out.css.')));
await sass.shouldExit(0);

await d.file("out.css", contains("y: z")).validate();
}, timeout: Timeout.factor(4));

test("files that share a modified import", () async {
await d.file("other.scss", r"a {b: $var}").create();
await d.file("test1.scss", r"$var: 1; @import 'other'").create();
Expand Down Expand Up @@ -311,3 +357,15 @@ void sharedTests(
});
});
}

/// A stylesheet that takes multiple seconds to compile, so that another file
/// can be modified while a compilation that includes it is in progress.
const _slowSource = r"""
@use "sass:math";
@for $i from 1 through 80 {
.a-#{$i} { %ph-#{$i} { color: red; } }
@for $j from 1 through 80 {
.b-#{$i}-#{$j} { @extend %ph-#{$i}; width: math.div($i * 100%, $j); }
}
}
""";
47 changes: 47 additions & 0 deletions test/cli/shared/watch.dart
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,41 @@ void sharedTests(
.validate();
});

// Regression test for #2849.
test("when a dependency is modified during a compilation", () async {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth having a parallel test for --update as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought about modifying the target's date, but I thought that this way it is a smaller change. Implementing the suggestion and adding the additional test.

await d.file("_slow.scss", _slowSource).create();
await d.file("_other.scss", "a {b: c}").create();
await d.file("test.scss", "@use 'slow'; @use 'other'").create();

var sass = await watch(["test.scss:out.css"]);
await expectLater(
sass.stdout,
emits(endsWith('Compiled test.scss to out.css.')),
);
await expectLater(sass.stdout, _watchingForChanges);
await tickIfPoll();

// Modifying the slow stylesheet starts a compilation that runs for
// several seconds. The delay is longer than the polling interval so
// that the modification below is seen as a separate change, but short
// enough that it lands while that compilation is still in progress.
await d.file("_slow.scss", "$_slowSource\n// modified").create();
await Future<void>.delayed(Duration(seconds: 3));
await d.file("_other.scss", "x {y: z}").create();

await expectLater(
sass.stdout,
emits(endsWith('Compiled test.scss to out.css.')),
);
await expectLater(
sass.stdout,
emits(endsWith('Compiled test.scss to out.css.')),
);
await sass.kill();

await d.file("out.css", contains("y: z")).validate();
}, timeout: Timeout.factor(4));

test("when it's modified when watched from a directory", () async {
await d.dir("dir", [d.file("test.scss", "a {b: c}")]).create();

Expand Down Expand Up @@ -1216,6 +1251,18 @@ void sharedTests(
}
}

/// A stylesheet that takes multiple seconds to compile, so that another file
/// can be modified while a compilation that includes it is in progress.
const _slowSource = r"""
@use "sass:math";
@for $i from 1 through 80 {
.a-#{$i} { %ph-#{$i} { color: red; } }
@for $j from 1 through 80 {
.b-#{$i}-#{$j} { @extend %ph-#{$i}; width: math.div($i * 100%, $j); }
}
}
""";

/// Writes [contents] to [path] atomically.
///
/// This matches the "atomic write" pattern used by various tools in which a
Expand Down
Loading