From 79d9a734743b7a5cf3e1434bcd7d21183927408c Mon Sep 17 00:00:00 2001 From: Jennifer Thakar Date: Thu, 2 Jan 2020 15:25:02 -0800 Subject: [PATCH] Cache separate canonical URLs for @use and @import (#908) Fixes #899. The cache for canonical URLs is now keyed on *both* the rule URL and whether that URL was canonicalized for an `@import` rule. --- CHANGELOG.md | 5 ++ lib/src/async_import_cache.dart | 32 ++++++--- lib/src/executable/watch.dart | 24 +++++-- lib/src/import_cache.dart | 30 +++++---- lib/src/importer/utils.dart | 45 +++++++------ lib/src/stylesheet_graph.dart | 89 ++++++++++++++++++-------- lib/src/visitor/async_evaluate.dart | 16 +++-- lib/src/visitor/evaluate.dart | 17 ++--- lib/src/visitor/find_dependencies.dart | 50 +++++++++++++++ lib/src/visitor/find_imports.dart | 44 ------------- pubspec.yaml | 2 +- test/cli/shared.dart | 16 +++++ 12 files changed, 232 insertions(+), 138 deletions(-) create mode 100644 lib/src/visitor/find_dependencies.dart delete mode 100644 lib/src/visitor/find_imports.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index b29326068..b8ec06814 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.24.1 + +* Fix a bug where the wrong file could be loaded when the same URL is used by + both a `@use` rule and an `@import` rule. + ## 1.24.0 * Add an optional `with` clause to the `@forward` rule. This works like the diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 7385e84f1..94bc45ac3 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -11,6 +11,7 @@ import 'package:tuple/tuple.dart'; import 'ast/sass.dart'; import 'importer.dart'; import 'importer/result.dart'; +import 'importer/utils.dart'; import 'io.dart'; import 'logger.dart'; import 'sync_package_resolver.dart'; @@ -26,11 +27,15 @@ class AsyncImportCache { /// The canonicalized URLs for each non-canonical URL. /// + /// The second item in each key's tuple is true when this canonicalization is + /// for an `@import` rule. Otherwise, it's for a `@use` or `@forward` rule. + /// /// This map's values are the same as the return value of [canonicalize]. /// /// This cache isn't used for relative imports, because they're /// context-dependent. - final Map> _canonicalizeCache; + final Map, Tuple3> + _canonicalizeCache; /// The parsed stylesheets for each canonicalized import URL. final Map _importCache; @@ -109,18 +114,20 @@ class AsyncImportCache { /// If any importers understand [url], returns that importer as well as the /// canonicalized URL. Otherwise, returns `null`. Future> canonicalize(Uri url, - [AsyncImporter baseImporter, Uri baseUrl]) async { + {AsyncImporter baseImporter, Uri baseUrl, bool forImport = false}) async { if (baseImporter != null) { var resolvedUrl = baseUrl != null ? baseUrl.resolveUri(url) : url; - var canonicalUrl = await _canonicalize(baseImporter, resolvedUrl); + var canonicalUrl = + await _canonicalize(baseImporter, resolvedUrl, forImport); if (canonicalUrl != null) { return Tuple3(baseImporter, canonicalUrl, resolvedUrl); } } - return await putIfAbsentAsync(_canonicalizeCache, url, () async { + return await putIfAbsentAsync(_canonicalizeCache, Tuple2(url, forImport), + () async { for (var importer in _importers) { - var canonicalUrl = await _canonicalize(importer, url); + var canonicalUrl = await _canonicalize(importer, url, forImport); if (canonicalUrl != null) { return Tuple3(importer, canonicalUrl, url); } @@ -132,8 +139,11 @@ class AsyncImportCache { /// Calls [importer.canonicalize] and prints a deprecation warning if it /// returns a relative URL. - Future _canonicalize(AsyncImporter importer, Uri url) async { - var result = await importer.canonicalize(url); + Future _canonicalize( + AsyncImporter importer, Uri url, bool forImport) async { + var result = await (forImport + ? inImportRule(() => importer.canonicalize(url)) + : importer.canonicalize(url)); if (result?.scheme == '') { _logger.warn(""" Importer $importer canonicalized $url to $result. @@ -153,8 +163,9 @@ Relative canonical URLs are deprecated and will eventually be disallowed. /// /// Caches the result of the import and uses cached results if possible. Future> import(Uri url, - [AsyncImporter baseImporter, Uri baseUrl]) async { - var tuple = await canonicalize(url, baseImporter, baseUrl); + {AsyncImporter baseImporter, Uri baseUrl, bool forImport = false}) async { + var tuple = await canonicalize(url, + baseImporter: baseImporter, baseUrl: baseUrl, forImport: forImport); if (tuple == null) return null; var stylesheet = await importCanonical(tuple.item1, tuple.item2, tuple.item3); @@ -216,7 +227,8 @@ Relative canonical URLs are deprecated and will eventually be disallowed. /// /// Has no effect if the canonical version of [url] has not been cached. void clearCanonicalize(Uri url) { - _canonicalizeCache.remove(url); + _canonicalizeCache.remove(Tuple2(url, false)); + _canonicalizeCache.remove(Tuple2(url, true)); } /// Clears the cached parse tree for the stylesheet with the given diff --git a/lib/src/executable/watch.dart b/lib/src/executable/watch.dart index 4df60d8c9..013a5e24f 100644 --- a/lib/src/executable/watch.dart +++ b/lib/src/executable/watch.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:collection'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart'; import 'package:stream_transform/stream_transform.dart'; @@ -288,8 +289,9 @@ class _Watcher { var changed = []; for (var node in _graph.nodes.values) { var importChanged = false; - for (var url in node.upstream.keys) { - if (_name(p.url.basename(url.path)) != name) continue; + void recanonicalize(Uri url, StylesheetNode upstream, + {@required bool forImport}) { + if (_name(p.url.basename(url.path)) != name) return; _graph.clearCanonicalize(url); // If the import produces a different canonicalized URL than it did @@ -298,15 +300,25 @@ class _Watcher { Uri newCanonicalUrl; try { newCanonicalUrl = _graph.importCache - .canonicalize(url, node.importer, node.canonicalUrl) + .canonicalize(url, + baseImporter: node.importer, + baseUrl: node.canonicalUrl, + forImport: forImport) ?.item2; } catch (_) { - // If the call to canonicalize failed, do nothing. We'll surface the - // error more nicely when we try to recompile the file. + // If the call to canonicalize failed, do nothing. We'll surface + // the error more nicely when we try to recompile the file. } - importChanged = newCanonicalUrl != node.upstream[url]?.canonicalUrl; + importChanged = newCanonicalUrl != upstream?.canonicalUrl; } } + + for (var entry in node.upstream.entries) { + recanonicalize(entry.key, entry.value, forImport: false); + } + for (var entry in node.upstreamImports.entries) { + recanonicalize(entry.key, entry.value, forImport: true); + } if (importChanged) changed.add(node); } diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index 9f99db637..9613dafbd 100644 --- a/lib/src/import_cache.dart +++ b/lib/src/import_cache.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_import_cache.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: 8555cca43b8d54d392e81f33935fd379d1eb3c72 +// Checksum: 3ca2f221c3c1503c688be49d4f7501bd9be8031a // // ignore_for_file: unused_import @@ -16,6 +16,7 @@ import 'package:tuple/tuple.dart'; import 'ast/sass.dart'; import 'importer.dart'; import 'importer/result.dart'; +import 'importer/utils.dart'; import 'io.dart'; import 'logger.dart'; import 'sync_package_resolver.dart'; @@ -31,11 +32,14 @@ class ImportCache { /// The canonicalized URLs for each non-canonical URL. /// + /// The second item in each key's tuple is true when this canonicalization is + /// for an `@import` rule. Otherwise, it's for a `@use` or `@forward` rule. + /// /// This map's values are the same as the return value of [canonicalize]. /// /// This cache isn't used for relative imports, because they're /// context-dependent. - final Map> _canonicalizeCache; + final Map, Tuple3> _canonicalizeCache; /// The parsed stylesheets for each canonicalized import URL. final Map _importCache; @@ -114,18 +118,18 @@ class ImportCache { /// If any importers understand [url], returns that importer as well as the /// canonicalized URL. Otherwise, returns `null`. Tuple3 canonicalize(Uri url, - [Importer baseImporter, Uri baseUrl]) { + {Importer baseImporter, Uri baseUrl, bool forImport = false}) { if (baseImporter != null) { var resolvedUrl = baseUrl != null ? baseUrl.resolveUri(url) : url; - var canonicalUrl = _canonicalize(baseImporter, resolvedUrl); + var canonicalUrl = _canonicalize(baseImporter, resolvedUrl, forImport); if (canonicalUrl != null) { return Tuple3(baseImporter, canonicalUrl, resolvedUrl); } } - return _canonicalizeCache.putIfAbsent(url, () { + return _canonicalizeCache.putIfAbsent(Tuple2(url, forImport), () { for (var importer in _importers) { - var canonicalUrl = _canonicalize(importer, url); + var canonicalUrl = _canonicalize(importer, url, forImport); if (canonicalUrl != null) { return Tuple3(importer, canonicalUrl, url); } @@ -137,8 +141,10 @@ class ImportCache { /// Calls [importer.canonicalize] and prints a deprecation warning if it /// returns a relative URL. - Uri _canonicalize(Importer importer, Uri url) { - var result = importer.canonicalize(url); + Uri _canonicalize(Importer importer, Uri url, bool forImport) { + var result = (forImport + ? inImportRule(() => importer.canonicalize(url)) + : importer.canonicalize(url)); if (result?.scheme == '') { _logger.warn(""" Importer $importer canonicalized $url to $result. @@ -158,8 +164,9 @@ Relative canonical URLs are deprecated and will eventually be disallowed. /// /// Caches the result of the import and uses cached results if possible. Tuple2 import(Uri url, - [Importer baseImporter, Uri baseUrl]) { - var tuple = canonicalize(url, baseImporter, baseUrl); + {Importer baseImporter, Uri baseUrl, bool forImport = false}) { + var tuple = canonicalize(url, + baseImporter: baseImporter, baseUrl: baseUrl, forImport: forImport); if (tuple == null) return null; var stylesheet = importCanonical(tuple.item1, tuple.item2, tuple.item3); return Tuple2(tuple.item1, stylesheet); @@ -220,7 +227,8 @@ Relative canonical URLs are deprecated and will eventually be disallowed. /// /// Has no effect if the canonical version of [url] has not been cached. void clearCanonicalize(Uri url) { - _canonicalizeCache.remove(url); + _canonicalizeCache.remove(Tuple2(url, false)); + _canonicalizeCache.remove(Tuple2(url, true)); } /// Clears the cached parse tree for the stylesheet with the given diff --git a/lib/src/importer/utils.dart b/lib/src/importer/utils.dart index 7db6832b8..8bf19a736 100644 --- a/lib/src/importer/utils.dart +++ b/lib/src/importer/utils.dart @@ -6,43 +6,42 @@ import 'package:path/path.dart' as p; import '../io.dart'; -/// Whether the Sass interpreter is currently evaluating a `@use` rule. +/// Whether the Sass compiler is currently evaluating an `@import` rule. /// -/// The `@use` rule has slightly different path-resolution behavior than -/// `@import`: `@use` prioritizes a `.css` file with a given name at the same -/// level as `.sass` and `.scss`, while `@import` prefers `.sass` and `.scss` -/// over `.css`. It's admittedly hacky to set this globally, but `@import` will -/// eventually be removed, at which point we can delete this and have one -/// consistent behavior. -bool _inUseRule = false; +/// When evaluating `@import` rules, URLs should canonicalize to an import-only +/// file if one exists for the URL being canonicalized. Otherwise, +/// canonicalization should be identical for `@import` and `@use` rules. It's +/// admittedly hacky to set this globally, but `@import` will eventually be +/// removed, at which point we can delete this and have one consistent behavior. +bool _inImportRule = false; -/// Runs [callback] in a context where [resolveImportPath] uses `@use` semantics -/// rather than `@import` semantics. -T inUseRule(T callback()) { - var wasInUseRule = _inUseRule; - _inUseRule = true; +/// Runs [callback] in a context where [resolveImportPath] uses `@import` +/// semantics rather than `@use` semantics. +T inImportRule(T callback()) { + var wasInImportRule = _inImportRule; + _inImportRule = true; try { return callback(); } finally { - _inUseRule = wasInUseRule; + _inImportRule = wasInImportRule; } } -/// Like [inUseRule], but asynchronous. -Future inUseRuleAsync(Future callback()) async { - var wasInUseRule = _inUseRule; - _inUseRule = true; +/// Like [inImportRule], but asynchronous. +Future inImportRuleAsync(Future callback()) async { + var wasInImportRule = _inImportRule; + _inImportRule = true; try { return await callback(); } finally { - _inUseRule = wasInUseRule; + _inImportRule = wasInImportRule; } } /// Resolves an imported path using the same logic as the filesystem importer. /// -/// This tries to fill in extensions and partial prefixes and check if a directory default. If no file can be -/// found, it returns `null`. +/// This tries to fill in extensions and partial prefixes and check for a +/// directory default. If no file can be found, it returns `null`. String resolveImportPath(String path) { var extension = p.extension(path); if (extension == '.sass' || extension == '.scss' || extension == '.css') { @@ -96,7 +95,7 @@ String _exactlyOne(List paths) { paths.map((path) => " " + p.prettyUri(p.toUri(path))).join("\n"); } -/// If [_inUseRule] is `false`, invokes callback and returns the result. +/// If [_inImportRule] is `true`, invokes callback and returns the result. /// /// Otherwise, returns `null`. -T _ifInImport(T callback()) => _inUseRule ? null : callback(); +T _ifInImport(T callback()) => _inImportRule ? callback() : null; diff --git a/lib/src/stylesheet_graph.dart b/lib/src/stylesheet_graph.dart index bfbd6e15f..556f8f8e2 100644 --- a/lib/src/stylesheet_graph.dart +++ b/lib/src/stylesheet_graph.dart @@ -5,11 +5,12 @@ import 'dart:collection'; import 'package:collection/collection.dart'; +import 'package:tuple/tuple.dart'; import 'ast/sass.dart'; import 'import_cache.dart'; import 'importer.dart'; -import 'visitor/find_imports.dart'; +import 'visitor/find_dependencies.dart'; /// A graph of the import relationships between stylesheets, available via /// [nodes]. @@ -39,7 +40,8 @@ class StylesheetGraph { DateTime transitiveModificationTime(StylesheetNode node) { return _transitiveModificationTimes.putIfAbsent(node.canonicalUrl, () { var latest = node.importer.modificationTime(node.canonicalUrl); - for (var upstream in node.upstream.values) { + for (var upstream + in node.upstream.values.followedBy(node.upstreamImports.values)) { // If an import is missing, always recompile so we show the user the // error. var upstreamTime = upstream == null @@ -64,8 +66,8 @@ class StylesheetGraph { /// /// Returns `null` if the import cache can't find a stylesheet at [url]. StylesheetNode _add(Uri url, [Importer baseImporter, Uri baseUrl]) { - var tuple = _ignoreErrors( - () => importCache.canonicalize(url, baseImporter, baseUrl)); + var tuple = _ignoreErrors(() => importCache.canonicalize(url, + baseImporter: baseImporter, baseUrl: baseUrl)); if (tuple == null) return null; return addCanonical(tuple.item1, tuple.item2, tuple.item3); } @@ -90,17 +92,22 @@ class StylesheetGraph { _upstreamNodes(stylesheet, importer, canonicalUrl))); } - /// Returns a map from non-canonicalized imported URLs in [stylesheet], which - /// appears within [baseUrl] imported by [baseImporter]. - Map _upstreamNodes( + /// Returns two maps from non-canonicalized imported URLs in [stylesheet] to + /// nodes, which appears within [baseUrl] imported by [baseImporter]. + /// + /// The first map contains stylesheets depended on via `@use` and `@forward` + /// while the second map contains those depended on via `@import`. + Tuple2, Map> _upstreamNodes( Stylesheet stylesheet, Importer baseImporter, Uri baseUrl) { var active = {baseUrl}; - var upstreamUrls = - findImports(stylesheet).map((import) => Uri.parse(import.url)); - return { - for (var url in upstreamUrls) + var tuple = findDependencies(stylesheet); + return Tuple2({ + for (var url in tuple.item1) url: _nodeFor(url, baseImporter, baseUrl, active) - }; + }, { + for (var url in tuple.item2) + url: _nodeFor(url, baseImporter, baseUrl, active, forImport: true) + }); } /// Re-parses the stylesheet at [canonicalUrl] and updates the dependency graph @@ -160,9 +167,10 @@ class StylesheetGraph { /// 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 = _ignoreErrors( - () => importCache.canonicalize(url, baseImporter, baseUrl)); + Uri url, Importer baseImporter, Uri baseUrl, Set active, + {bool forImport = false}) { + var tuple = _ignoreErrors(() => importCache.canonicalize(url, + baseImporter: baseImporter, baseUrl: baseUrl, forImport: forImport)); // If an import fails, let the evaluator surface that error rather than // surfacing it here. @@ -233,29 +241,47 @@ class StylesheetNode { /// The canonical URL of [stylesheet]. final Uri canonicalUrl; - /// A map from non-canonicalized import URLs in [stylesheet] to the - /// stylesheets those imports refer to. + /// A map from non-canonicalized `@use` and `@forward` URLs in [stylesheet] to + /// the stylesheets those rules refer to. /// - /// This may have `null` values, which indicate failed imports. + /// This may have `null` values, which indicate failed loads. Map get upstream => UnmodifiableMapView(_upstream); Map _upstream; + /// A map from non-canonicalized `@import` URLs in [stylesheet] to the + /// stylesheets those imports refer to. + /// + /// This may have `null` values, which indicate failed imports. + Map get upstreamImports => + UnmodifiableMapView(_upstreamImports); + Map _upstreamImports; + /// The stylesheets that import [stylesheet]. Set get downstream => UnmodifiableSetView(_downstream); final _downstream = {}; - StylesheetNode._( - this._stylesheet, this.importer, this.canonicalUrl, this._upstream) { - for (var node in upstream.values) { + StylesheetNode._(this._stylesheet, this.importer, this.canonicalUrl, + Tuple2, Map> allUpstream) + : _upstream = allUpstream.item1, + _upstreamImports = allUpstream.item2 { + for (var node in upstream.values.followedBy(upstreamImports.values)) { if (node != null) node._downstream.add(this); } } - /// Sets [newUpstream] as the new value of [upstream] and adjusts upstream - /// nodes' [downstream] fields accordingly. - void _replaceUpstream(Map newUpstream) { - var oldUpstream = Set.of(upstream.values)..remove(null); - var newUpstreamSet = Set.of(newUpstream.values)..remove(null); + /// Updates [upstream] and [upstreamImports] from [allUpstream] and adjusts + /// upstream nodes' [downstream] fields accordingly. + /// + /// The first item in [allUpstream] replaces [upstream] while the second item + /// replaces [upstreamImports]. + void _replaceUpstream( + Tuple2, Map> allUpstream) { + var oldUpstream = {...upstream.values, ...upstreamImports.values} + ..remove(null); + var newUpstreamSet = { + ...allUpstream.item1.values, + ...allUpstream.item2.values + }..remove(null); for (var removed in oldUpstream.difference(newUpstreamSet)) { var wasRemoved = removed._downstream.remove(this); @@ -267,13 +293,14 @@ class StylesheetNode { assert(wasAdded); } - _upstream = newUpstream; + _upstream = allUpstream.item1; + _upstreamImports = allUpstream.item2; } /// Removes [this] as an upstream and downstream node from all the nodes that /// import it and that it imports. void _remove() { - for (var node in upstream.values) { + for (var node in {...upstream.values, ...upstreamImports.values}) { if (node == null) continue; var wasRemoved = node._downstream.remove(this); assert(wasRemoved); @@ -286,6 +313,12 @@ class StylesheetNode { break; } } + for (var url in node._upstreamImports.keys.toList()) { + if (node._upstreamImports[url] == this) { + node._upstreamImports[url] = null; + break; + } + } } } } diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index c465e5510..f3dd4d7d7 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -31,7 +31,6 @@ import '../functions.dart'; import '../functions/meta.dart' as meta; import '../importer.dart'; import '../importer/node.dart'; -import '../importer/utils.dart'; import '../io.dart'; import '../logger.dart'; import '../module.dart'; @@ -540,8 +539,8 @@ class _EvaluateVisitor } await _withStackFrame(stackFrame, nodeForSpan, () async { - var result = await inUseRuleAsync(() => - _loadStylesheet(url.toString(), nodeForSpan.span, baseUrl: baseUrl)); + var result = await _loadStylesheet(url.toString(), nodeForSpan.span, + baseUrl: baseUrl); var importer = result.item1; var stylesheet = result.item2; @@ -1310,7 +1309,8 @@ class _EvaluateVisitor /// Adds the stylesheet imported by [import] to the current document. Future _visitDynamicImport(DynamicImport import) { return _withStackFrame("@import", import, () async { - var result = await _loadStylesheet(import.url, import.span); + var result = + await _loadStylesheet(import.url, import.span, forImport: true); var importer = result.item1; var stylesheet = result.item2; @@ -1404,7 +1404,7 @@ class _EvaluateVisitor /// `_stylesheet.span.sourceUrl`. Future> _loadStylesheet( String url, FileSpan span, - {Uri baseUrl}) async { + {Uri baseUrl, bool forImport = false}) async { try { assert(_importSpan == null); _importSpan = span; @@ -1413,8 +1413,10 @@ class _EvaluateVisitor var stylesheet = await _importLikeNode(url); if (stylesheet != null) return Tuple2(null, stylesheet); } else { - var tuple = await _importCache.import( - Uri.parse(url), _importer, baseUrl ?? _stylesheet?.span?.sourceUrl); + var tuple = await _importCache.import(Uri.parse(url), + baseImporter: _importer, + baseUrl: baseUrl ?? _stylesheet?.span?.sourceUrl, + forImport: forImport); if (tuple != null) return tuple; } diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 4b65bc7ba..de8110b76 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: 3884b2f9f8775f0c3e49725bb2f647372219a149 +// Checksum: e3d7bea705bed417db9925546c076caed4465b2e // // ignore_for_file: unused_import @@ -40,7 +40,6 @@ import '../functions.dart'; import '../functions/meta.dart' as meta; import '../importer.dart'; import '../importer/node.dart'; -import '../importer/utils.dart'; import '../io.dart'; import '../logger.dart'; import '../module.dart'; @@ -544,8 +543,8 @@ class _EvaluateVisitor } _withStackFrame(stackFrame, nodeForSpan, () { - var result = inUseRule(() => - _loadStylesheet(url.toString(), nodeForSpan.span, baseUrl: baseUrl)); + var result = + _loadStylesheet(url.toString(), nodeForSpan.span, baseUrl: baseUrl); var importer = result.item1; var stylesheet = result.item2; @@ -1309,7 +1308,7 @@ class _EvaluateVisitor /// Adds the stylesheet imported by [import] to the current document. void _visitDynamicImport(DynamicImport import) { return _withStackFrame("@import", import, () { - var result = _loadStylesheet(import.url, import.span); + var result = _loadStylesheet(import.url, import.span, forImport: true); var importer = result.item1; var stylesheet = result.item2; @@ -1402,7 +1401,7 @@ class _EvaluateVisitor /// This first tries loading [url] relative to [baseUrl], which defaults to /// `_stylesheet.span.sourceUrl`. Tuple2 _loadStylesheet(String url, FileSpan span, - {Uri baseUrl}) { + {Uri baseUrl, bool forImport = false}) { try { assert(_importSpan == null); _importSpan = span; @@ -1411,8 +1410,10 @@ class _EvaluateVisitor var stylesheet = _importLikeNode(url); if (stylesheet != null) return Tuple2(null, stylesheet); } else { - var tuple = _importCache.import( - Uri.parse(url), _importer, baseUrl ?? _stylesheet?.span?.sourceUrl); + var tuple = _importCache.import(Uri.parse(url), + baseImporter: _importer, + baseUrl: baseUrl ?? _stylesheet?.span?.sourceUrl, + forImport: forImport); if (tuple != null) return tuple; } diff --git a/lib/src/visitor/find_dependencies.dart b/lib/src/visitor/find_dependencies.dart new file mode 100644 index 000000000..d2963ec50 --- /dev/null +++ b/lib/src/visitor/find_dependencies.dart @@ -0,0 +1,50 @@ +// 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 'package:tuple/tuple.dart'; + +import '../ast/sass.dart'; +import 'recursive_statement.dart'; + +/// Returns two lists of dependencies for [stylesheet]. +/// +/// The first is a list of URLs from all `@use` and `@forward` rules in +/// [stylesheet]. The second is a list of all imports in [stylesheet]. +Tuple2, List> findDependencies(Stylesheet stylesheet) => + _FindDependenciesVisitor().run(stylesheet); + +/// A visitor that traverses a stylesheet and records, all `@import`, `@use`, +/// and `@forward` rules it contains. +class _FindDependenciesVisitor extends RecursiveStatementVisitor { + final _usesAndForwards = []; + final _imports = []; + + Tuple2, List> run(Stylesheet stylesheet) { + visitStylesheet(stylesheet); + return Tuple2(_usesAndForwards, _imports); + } + + // These can never contain imports. + void visitEachRule(EachRule node) {} + void visitForRule(ForRule node) {} + void visitIfRule(IfRule node) {} + void visitWhileRule(WhileRule node) {} + void visitCallableDeclaration(CallableDeclaration node) {} + void visitInterpolation(Interpolation interpolation) {} + void visitSupportsCondition(SupportsCondition condition) {} + + void visitUseRule(UseRule node) { + _usesAndForwards.add(node.url); + } + + void visitForwardRule(ForwardRule node) { + _usesAndForwards.add(node.url); + } + + void visitImportRule(ImportRule node) { + for (var import in node.imports) { + if (import is DynamicImport) _imports.add(Uri.parse(import.url)); + } + } +} diff --git a/lib/src/visitor/find_imports.dart b/lib/src/visitor/find_imports.dart deleted file mode 100644 index 2a8c8d8be..000000000 --- a/lib/src/visitor/find_imports.dart +++ /dev/null @@ -1,44 +0,0 @@ -// 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 'recursive_statement.dart'; - -/// Returns a list of all [DynamicImport]s in [stylesheet]. -List findImports(Stylesheet stylesheet) => - _FindImportsVisitor().run(stylesheet); - -/// A visitor that traverses a stylesheet and records all the [DynamicImport]s -/// it contains. -class _FindImportsVisitor extends RecursiveStatementVisitor { - final _imports = []; - - List run(Stylesheet stylesheet) { - visitStylesheet(stylesheet); - return _imports; - } - - // These can never contain imports. - void visitEachRule(EachRule node) {} - void visitForRule(ForRule node) {} - void visitIfRule(IfRule node) {} - void visitWhileRule(WhileRule node) {} - void visitCallableDeclaration(CallableDeclaration node) {} - void visitInterpolation(Interpolation interpolation) {} - void visitSupportsCondition(SupportsCondition condition) {} - - void visitUseRule(UseRule node) { - _imports.add(DynamicImport(node.url.toString(), node.span)); - } - - void visitForwardRule(ForwardRule node) { - _imports.add(DynamicImport(node.url.toString(), node.span)); - } - - void visitImportRule(ImportRule node) { - for (var import in node.imports) { - if (import is DynamicImport) _imports.add(import); - } - } -} diff --git a/pubspec.yaml b/pubspec.yaml index 8f0b7a0b0..94a511c71 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: sass -version: 1.24.0 +version: 1.24.1 description: A Sass implementation in Dart. author: Sass Team homepage: https://github.com/sass/dart-sass diff --git a/test/cli/shared.dart b/test/cli/shared.dart index 0106c8d78..dd3f500a0 100644 --- a/test/cli/shared.dart +++ b/test/cli/shared.dart @@ -228,6 +228,22 @@ void sharedTests( "test.scss" ], equalsIgnoringWhitespace("a { b: c; } a { b: c; }")); }); + + // Regression test for sass/dart-sass#899 + test("with both @use and @import", () async { + await d.file("test.scss", """ + @use 'library'; + @import 'library'; + """).create(); + + await d.dir("load-path", [ + d.file("_library.scss", "a { b: regular }"), + d.file("_library.import.scss", "a { b: import-only }") + ]).create(); + + await expectCompiles(["--load-path", "load-path", "test.scss"], + equalsIgnoringWhitespace("a { b: regular; } a { b: import-only; }")); + }); }); group("with --stdin", () {