From ac8c01a595aae9a27fa60b73820b636d5c60bc6c Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Fri, 27 Apr 2018 16:57:37 -0700 Subject: [PATCH] Add an --update flag Partially addresses #264 --- CHANGELOG.md | 3 + lib/src/executable.dart | 37 +++-- lib/src/executable_options.dart | 20 ++- lib/src/stylesheet_graph.dart | 160 +++++++++++++++++++++ lib/src/visitor/recursive_statement.dart | 4 +- pubspec.yaml | 2 +- test/cli_shared.dart | 175 +++++++++++++++++++++++ test/utils.dart | 12 ++ 8 files changed, 398 insertions(+), 15 deletions(-) create mode 100644 lib/src/stylesheet_graph.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index ddff18bd..bfbdb56e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ `sass templates/stylesheets:public/css` compiles all non-partial Sass files in `templates/stylesheets` to CSS files in `public/css`. +* Add an `--update` flag that tells Sass to compile only stylesheets that have + been (transitively) modified since the CSS file was generated. + ### Dart API * Add `Importer.modificationTime()` and `AsyncImporter.modificationTime()` which diff --git a/lib/src/executable.dart b/lib/src/executable.dart index 72a636bf..f0ec1b63 100644 --- a/lib/src/executable.dart +++ b/lib/src/executable.dart @@ -17,6 +17,7 @@ import 'exception.dart'; import 'executable_options.dart'; import 'import_cache.dart'; import 'io.dart'; +import 'stylesheet_graph.dart'; import 'visitor/async_evaluate.dart'; import 'visitor/evaluate.dart'; import 'visitor/serialize.dart'; @@ -48,12 +49,12 @@ main(List args) async { return; } - var importCache = new ImportCache([], - loadPaths: options.loadPaths, logger: options.logger); + var graph = new StylesheetGraph(new ImportCache([], + loadPaths: options.loadPaths, logger: options.logger)); for (var source in options.sourcesToDestinations.keys) { var destination = options.sourcesToDestinations[source]; try { - await _compileStylesheet(options, importCache, source, destination); + await _compileStylesheet(options, graph, source, destination); } on SassException catch (error, stackTrace) { printError(error.toString(color: options.color), options.trace ? stackTrace : null); @@ -111,15 +112,28 @@ Future _loadVersion() async { /// Compiles the stylesheet at [source] to [destination]. /// -/// Loads files from [importCache] when possible. +/// Loads files using `graph.importCache` when possible. /// /// If [source] is `null`, that indicates that the stylesheet should be read /// from stdin. If [destination] is `null`, that indicates that the stylesheet /// should be emitted to stdout. -Future _compileStylesheet(ExecutableOptions options, ImportCache importCache, +Future _compileStylesheet(ExecutableOptions options, StylesheetGraph graph, String source, String destination) async { - var stylesheet = await _parseStylesheet(options, importCache, source); var importer = new FilesystemImporter('.'); + if (options.update) { + try { + if (source != null && + destination != null && + !graph.modifiedSince( + p.toUri(source), modificationTime(destination), importer)) { + return; + } + } on FileSystemException catch (_) { + // Compile as normal if the destination file doesn't exist. + } + } + + var stylesheet = await _parseStylesheet(options, graph.importCache, source); var evaluateResult = options.asynchronous ? await evaluateAsync(stylesheet, importCache: new AsyncImportCache([], @@ -128,7 +142,7 @@ Future _compileStylesheet(ExecutableOptions options, ImportCache importCache, logger: options.logger, sourceMap: options.emitSourceMap) : await evaluate(stylesheet, - importCache: importCache, + importCache: graph.importCache, importer: importer, logger: options.logger, sourceMap: options.emitSourceMap); @@ -144,9 +158,16 @@ Future _compileStylesheet(ExecutableOptions options, ImportCache importCache, ensureDir(p.dirname(destination)); writeFile(destination, css + "\n"); } + + if (!options.update || options.quiet) return; + var buffer = new StringBuffer(); + if (options.color) buffer.write('\u001b[32m'); + buffer.write('Compiled ${source ?? 'stdin'} to $destination.'); + if (options.color) buffer.write('\u001b[0m'); + print(buffer); } -/// Parses [source] according to [options], loading it from [importCache] if +/// Parses [source] according to [options], loading it from [graph] if /// possible. /// /// Returns the parsed [Stylesheet]. diff --git a/lib/src/executable_options.dart b/lib/src/executable_options.dart index 27399907..e1bec694 100644 --- a/lib/src/executable_options.dart +++ b/lib/src/executable_options.dart @@ -48,7 +48,9 @@ class ExecutableOptions { valueHelp: 'NAME', help: 'Output style.', allowed: ['expanded', 'compressed'], - defaultsTo: 'expanded'); + defaultsTo: 'expanded') + ..addFlag('update', + help: 'Only compile out-of-date stylesheets.', negatable: false); parser ..addSeparator(_separator('Source Maps')) @@ -110,10 +112,11 @@ class ExecutableOptions { bool get color => _options.wasParsed('color') ? _options['color'] as bool : hasTerminal; + /// Whether to silence normal output. + bool get quiet => _options['quiet'] as bool; + /// The logger to use to emit messages from Sass. - Logger get logger => _options['quiet'] as bool - ? Logger.quiet - : new Logger.stderr(color: color); + Logger get logger => quiet ? Logger.quiet : new Logger.stderr(color: color); /// The style to use for the generated CSS. OutputStyle get style => _options['style'] == 'compressed' @@ -129,6 +132,9 @@ class ExecutableOptions { /// Whether to print the full Dart stack trace on exceptions. bool get trace => _options['trace'] as bool; + /// Whether to update only files that have changed since the last compilation. + bool get update => _options['update'] as bool; + /// A map from source paths to the destination paths where the compiled CSS /// should be written. /// @@ -165,6 +171,8 @@ class ExecutableOptions { } else if (stdin) { if (_options.rest.length > 1) { _fail("Only one argument is allowed with --stdin."); + } else if (update) { + _fail("--update is not allowed with --stdin."); } return {null: _options.rest.isEmpty ? null : _options.rest.first}; } else if (_options.rest.length > 2) { @@ -172,6 +180,10 @@ class ExecutableOptions { } else { var source = _options.rest.first == '-' ? null : _options.rest.first; var destination = _options.rest.length == 1 ? null : _options.rest.last; + if (update && destination == null) { + _fail("--update is not allowed when printing to stdout."); + } + return {source: destination}; } } diff --git a/lib/src/stylesheet_graph.dart b/lib/src/stylesheet_graph.dart new file mode 100644 index 00000000..ba462270 --- /dev/null +++ b/lib/src/stylesheet_graph.dart @@ -0,0 +1,160 @@ +// Copyright 2018 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'ast/sass.dart'; +import 'import_cache.dart'; +import 'importer.dart'; +import 'visitor/find_imports.dart'; + +/// A graph of the import relationships between stylesheets. +class StylesheetGraph { + /// A map from canonical URLs to the stylesheet nodes for those URLs. + final _nodes = {}; + + /// The import cache used to load stylesheets. + final ImportCache importCache; + + /// A map from canonical URLs to the time the corresponding stylesheet or any + /// of the stylesheets it transitively imports was modified. + final _transitiveModificationTimes = {}; + + StylesheetGraph(this.importCache); + + /// Returns whether the stylesheet at [url] or any of the stylesheets it + /// imports were modified since [since]. + /// + /// If [baseImporter] is non-`null`, this first tries to use [baseImporter] to + /// import [url] (resolved relative to [baseUrl] if it's passed). + /// + /// Returns `true` if the import cache can't find a stylesheet at [url]. + bool modifiedSince(Uri url, DateTime since, + [Importer baseImporter, Uri baseUrl]) { + DateTime transitiveModificationTime(_StylesheetNode node) { + return _transitiveModificationTimes.putIfAbsent(node.canonicalUrl, () { + var latest = node.importer.modificationTime(node.canonicalUrl); + for (var upstream in node.upstream) { + var upstreamTime = transitiveModificationTime(upstream); + if (upstreamTime.isAfter(latest)) latest = upstreamTime; + } + return latest; + }); + } + + var node = _add(url, baseImporter, baseUrl); + if (node == null) return true; + return transitiveModificationTime(node).isAfter(since); + } + + /// Adds the stylesheet at [url] and all the stylesheets it imports to this + /// graph and returns its node. + /// + /// If [baseImporter] is non-`null`, this first tries to use [baseImporter] to + /// import [url] (resolved relative to [baseUrl] if it's passed). + /// + /// Returns `null` if the import cache can't find a stylesheet at [url]. + _StylesheetNode _add(Uri url, [Importer baseImporter, Uri baseUrl]) { + var tuple = importCache.canonicalize(url, baseImporter, baseUrl); + if (tuple == null) return null; + var importer = tuple.item1; + var canonicalUrl = tuple.item2; + + return _nodes.putIfAbsent(canonicalUrl, () { + var stylesheet = importCache.importCanonical(importer, canonicalUrl); + if (stylesheet == null) return null; + + var active = new Set.from([canonicalUrl]); + return new _StylesheetNode( + stylesheet, + importer, + canonicalUrl, + findImports(stylesheet) + .map((import) => _nodeFor( + Uri.parse(import.url), importer, canonicalUrl, active)) + .where((node) => node != null)); + }); + } + + /// Returns the [StylesheetNode] for the stylesheet at the given [url], which + /// appears within [baseUrl] imported by [baseImporter]. + /// + /// The [active] set should contain the canonical URLs that are currently + /// being imported. It's used to detect circular imports. + _StylesheetNode _nodeFor( + Uri url, Importer baseImporter, Uri baseUrl, Set active) { + var tuple = importCache.canonicalize(url, baseImporter, baseUrl); + + // If an import fails, let the evaluator surface that error rather than + // surfacing it here. + if (tuple == null) return null; + var importer = tuple.item1; + var canonicalUrl = tuple.item2; + + // Don't use [putIfAbsent] here because we want to avoid adding an entry if + // the import fails. + if (_nodes.containsKey(canonicalUrl)) return _nodes[canonicalUrl]; + + /// If we detect a circular import, act as though it doesn't exist. A better + /// error will be produced during compilation. + if (active.contains(canonicalUrl)) return null; + + var stylesheet = importCache.importCanonical(importer, canonicalUrl); + if (stylesheet == null) return null; + + active.add(canonicalUrl); + var node = new _StylesheetNode( + stylesheet, + importer, + canonicalUrl, + findImports(stylesheet) + .map((import) => + _nodeFor(Uri.parse(import.url), importer, canonicalUrl, active)) + .where((node) => node != null)); + active.remove(canonicalUrl); + _nodes[canonicalUrl] = node; + return node; + } +} + +/// A node in a [StylesheetGraph] that tracks a single stylesheet and all the +/// upstream stylesheets it imports and the downstream stylesheets that import +/// it. +/// +/// A [StylesheetNode] is immutable except for its downstream nodes. When the +/// stylesheet itself changes, a new node should be generated. +class _StylesheetNode { + /// The parsed stylesheet. + final Stylesheet stylesheet; + + /// The importer that was used to load this stylesheet. + final Importer importer; + + /// The canonical URL of [stylesheet]. + final Uri canonicalUrl; + + /// The stylesheets that [stylesheet] imports. + final List<_StylesheetNode> upstream; + + /// The stylesheets that import [stylesheet]. + /// + /// This is automatically populated when new [_StylesheetNode]s are created + /// that list this as an upstream node. + final downstream = new Set<_StylesheetNode>(); + + _StylesheetNode(this.stylesheet, this.importer, this.canonicalUrl, + Iterable<_StylesheetNode> upstream) + : upstream = new List.unmodifiable(upstream) { + for (var node in upstream) { + node.downstream.add(this); + } + } + + /// Removes [this] as a downstream node from all the upstream nodes that it + /// imports. + void remove() { + for (var node in upstream) { + var wasRemoved = node.downstream.remove(this); + assert(wasRemoved); + } + } +} diff --git a/lib/src/visitor/recursive_statement.dart b/lib/src/visitor/recursive_statement.dart index 6139877a..020e1848 100644 --- a/lib/src/visitor/recursive_statement.dart +++ b/lib/src/visitor/recursive_statement.dart @@ -28,7 +28,7 @@ abstract class RecursiveStatementVisitor implements StatementVisitor { T visitAtRule(AtRule node) { visitInterpolation(node.value); - return visitChildren(node); + return node.children == null ? null : visitChildren(node); } T visitContentRule(ContentRule node) => null; @@ -41,7 +41,7 @@ abstract class RecursiveStatementVisitor implements StatementVisitor { T visitDeclaration(Declaration node) { visitInterpolation(node.name); visitExpression(node.value); - return visitChildren(node); + return node.children == null ? null : visitChildren(node); } T visitEachRule(EachRule node) { diff --git a/pubspec.yaml b/pubspec.yaml index f2dd0907..54de0dcd 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.4.0-dev +version: 1.4.0 description: A Sass implementation in Dart. author: Dart Team homepage: https://github.com/sass/dart-sass diff --git a/test/cli_shared.dart b/test/cli_shared.dart index 893c59f9..5ac804cc 100644 --- a/test/cli_shared.dart +++ b/test/cli_shared.dart @@ -377,6 +377,181 @@ void sharedTests(Future runSass(Iterable arguments)) { }); }); + group("with --update", () { + group("updates CSS", () { + test("that doesn't exist yet", () async { + await d.file("test.scss", "a {b: c}").create(); + + var sass = + await runSass(["--no-source-map", "--update", "test.scss:out.css"]); + expect(sass.stdout, emits('Compiled test.scss to out.css.')); + await sass.shouldExit(0); + + await d + .file("out.css", equalsIgnoringWhitespace("a { b: c; }")) + .validate(); + }); + + test("whose source was modified", () async { + await d.file("out.css", "x {y: z}").create(); + await tick; + await d.file("test.scss", "a {b: c}").create(); + + var sass = + await runSass(["--no-source-map", "--update", "test.scss:out.css"]); + expect(sass.stdout, emits('Compiled test.scss to out.css.')); + await sass.shouldExit(0); + + await d + .file("out.css", equalsIgnoringWhitespace("a { b: c; }")) + .validate(); + }); + + test("whose source was transitively modified", () async { + await d.file("other.scss", "a {b: c}").create(); + await d.file("test.scss", "@import 'other'").create(); + + var sass = + await runSass(["--no-source-map", "--update", "test.scss:out.css"]); + expect(sass.stdout, emits('Compiled test.scss to out.css.')); + await sass.shouldExit(0); + + await tick; + await d.file("other.scss", "x {y: z}").create(); + + sass = + await runSass(["--no-source-map", "--update", "test.scss:out.css"]); + expect(sass.stdout, emits('Compiled test.scss to out.css.')); + await sass.shouldExit(0); + + await d + .file("out.css", equalsIgnoringWhitespace("x { y: z; }")) + .validate(); + }); + + 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(); + await d.file("test2.scss", r"$var: 2; @import 'other'").create(); + + var sass = await runSass([ + "--no-source-map", + "--update", + "test1.scss:out1.css", + "test2.scss:out2.css" + ]); + expect(sass.stdout, emits('Compiled test1.scss to out1.css.')); + expect(sass.stdout, emits('Compiled test2.scss to out2.css.')); + await sass.shouldExit(0); + + await tick; + await d.file("other.scss", r"x {y: $var}").create(); + + sass = await runSass([ + "--no-source-map", + "--update", + "test1.scss:out1.css", + "test2.scss:out2.css" + ]); + expect(sass.stdout, emits('Compiled test1.scss to out1.css.')); + expect(sass.stdout, emits('Compiled test2.scss to out2.css.')); + await sass.shouldExit(0); + + await d + .file("out1.css", equalsIgnoringWhitespace("x { y: 1; }")) + .validate(); + await d + .file("out2.css", equalsIgnoringWhitespace("x { y: 2; }")) + .validate(); + }); + + test("from stdin", () async { + var sass = await runSass(["--no-source-map", "--update", "-:out.css"]); + sass.stdin.writeln("a {b: c}"); + sass.stdin.close(); + expect(sass.stdout, emits('Compiled stdin to out.css.')); + await sass.shouldExit(0); + + await d + .file("out.css", equalsIgnoringWhitespace("a { b: c; }")) + .validate(); + + sass = await runSass(["--no-source-map", "--update", "-:out.css"]); + sass.stdin.writeln("x {y: z}"); + sass.stdin.close(); + expect(sass.stdout, emits('Compiled stdin to out.css.')); + await sass.shouldExit(0); + + await d + .file("out.css", equalsIgnoringWhitespace("x { y: z; }")) + .validate(); + }); + + test("without printing anything if --quiet is passed", () async { + await d.file("test.scss", "a {b: c}").create(); + + var sass = await runSass( + ["--no-source-map", "--update", "--quiet", "test.scss:out.css"]); + expect(sass.stdout, emitsDone); + await sass.shouldExit(0); + + await d + .file("out.css", equalsIgnoringWhitespace("a { b: c; }")) + .validate(); + }); + }); + + group("doesn't update a CSS file", () { + test("whose sources weren't modified", () async { + await d.file("test.scss", "a {b: c}").create(); + await d.file("out.css", "x {y: z}").create(); + + var sass = + await runSass(["--no-source-map", "--update", "test.scss:out.css"]); + expect(sass.stdout, emitsDone); + await sass.shouldExit(0); + + await d.file("out.css", "x {y: z}").validate(); + }); + + test("whose sibling was modified", () async { + await d.file("test1.scss", "a {b: c}").create(); + await d.file("out1.css", "x {y: z}").create(); + + await d.file("out2.css", "q {r: s}").create(); + await tick; + await d.file("test2.scss", "d {e: f}").create(); + + var sass = await runSass([ + "--no-source-map", + "--update", + "test1.scss:out1.css", + "test2.scss:out2.css" + ]); + expect(sass.stdout, emits('Compiled test2.scss to out2.css.')); + await sass.shouldExit(0); + + await d.file("out1.css", "x {y: z}").validate(); + }); + }); + + group("doesn't allow", () { + test("--stdin", () async { + var sass = await runSass( + ["--no-source-map", "--stdin", "--update", "test.scss"]); + expect(sass.stdout, emits('--update is not allowed with --stdin.')); + await sass.shouldExit(64); + }); + + test("printing to stderr", () async { + var sass = await runSass(["--no-source-map", "--update", "test.scss"]); + expect(sass.stdout, + emits('--update is not allowed when printing to stdout.')); + await sass.shouldExit(64); + }); + }); + }); + test("gracefully reports errors from stdin", () async { var sass = await runSass(["-"]); sass.stdin.writeln("a {b: 1 + }"); diff --git a/test/utils.dart b/test/utils.dart index 229cf362..546384e5 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -2,13 +2,25 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:async'; + import 'package:dart2_constant/convert.dart' as convert; import 'package:test/test.dart'; +import 'package:sass/src/io.dart'; + /// A regular expression for matching the URL in a source map comment. final _sourceMapCommentRegExp = new RegExp(r"/\*# sourceMappingURL=(.*) \*/\s*$"); +/// Returns a [Future] that waits long enough for modification times to be +/// different. +/// +/// Windows (or at least Appveyor) seems to require a more coarse-grained time +/// than Unixes. +Future get tick => + new Future.delayed(new Duration(milliseconds: isWindows ? 1000 : 50)); + /// Loads and decodes the source map embedded as a `data:` URI in [css]. /// /// Throws a [TestFailure] if [css] doesn't have such a source map.