Skip to content

Commit

Permalink
*ImportCache.canonicalize: Deprecate base importer without URL
Browse files Browse the repository at this point in the history 
See #2208
  • Loading branch information
@nex3
nex3 committed Apr 9, 2024
1 parent 1137797 commit b6bf5bb
Show file tree
Hide file tree
Showing 15 changed files with 192 additions and 107 deletions.
20 changes: 20 additions & 0 deletions CHANGELOG.md
View file
@@ -1,3 +1,23 @@
## 1.75.0

### JS API

* Passing an `importer` argument without a corresponding canonical `url` to
`compileString()` and `compileStringAsync()` will now emit a deprecation named
`importer-without-url`. This was always forbidden by the TypeScript types, and
so was officially undefined behavior, but the previous behavior (passing
relative URLs as-is to the `importer` before passing them to the `importers`
list) will be preserved until Dart Sass 2.0.0. See
https://sass-lang.com/d/importer-without-url for more information.

### Dart API

* Passing an `importer` argument without a corresponding canonical `url` to
`compileStringToResult()`, `compileStringToResultAsync()`, `compileString()`,
and `compileStringAsync()` will now emit a deprecation named
`importer-without-url`. See https://sass-lang.com/d/importer-without-url for
more information.

## 1.74.1

* No user-visible changes.
Expand Down
10 changes: 10 additions & 0 deletions lib/src/async_compile.dart
View file
Expand Up @@ -118,6 +118,16 @@ Future<CompileResult> compileStringAsync(String source,
futureDeprecations: {...?futureDeprecations},
limitRepetition: !verbose);

// Allow NoOpImporter because various first-party callers use that to opt out
// of the also-deprecated FilesystemImporter.cwd behavior.
if (importer != null && importer is! NoOpImporter && url == null) {
logger.warnForDeprecation(
Deprecation.importerWithoutUrl,
"Passing an importer to compileString* without a canonical URL is "
"deprecated and will be an error in future versions of Sass. Use the "
"importers argument for non-relative loads.");
}

var stylesheet =
Stylesheet.parse(source, syntax ?? Syntax.scss, url: url, logger: logger);

Expand Down
20 changes: 8 additions & 12 deletions lib/src/async_import_cache.dart
View file
Expand Up @@ -154,6 +154,8 @@ final class AsyncImportCache {
}

if (baseImporter != null && url.scheme == '') {
// TODO(sass/sass#3831): Throw an ArgumentError here if baseImporter is
// passed without a baseUrl as well.
var relativeResult = await putIfAbsentAsync(
_relativeCanonicalizeCache,
(
Expand Down Expand Up @@ -182,17 +184,11 @@ final class AsyncImportCache {

/// Calls [importer.canonicalize] and prints a deprecation warning if it
/// returns a relative URL.
///
/// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl]
/// before passing it to [importer].
Future<AsyncCanonicalizeResult?> _canonicalize(
AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport,
{bool resolveUrl = false}) async {
var resolved =
resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url;
AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport) async {
var canonicalize = forImport
? () => inImportRule(() => importer.canonicalize(resolved))
: () => importer.canonicalize(resolved);
? () => inImportRule(() => importer.canonicalize(url))
: () => importer.canonicalize(url);

var passContainingUrl = baseUrl != null &&
(url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme));
Expand All @@ -203,15 +199,15 @@ final class AsyncImportCache {
if (result.scheme == '') {
_logger.warnForDeprecation(
Deprecation.relativeCanonical,
"Importer $importer canonicalized $resolved to $result.\n"
"Importer $importer canonicalized $url to $result.\n"
"Relative canonical URLs are deprecated and will eventually be "
"disallowed.");
} else if (await importer.isNonCanonicalScheme(result.scheme)) {
throw "Importer $importer canonicalized $resolved to $result, which "
throw "Importer $importer canonicalized $url to $result, which "
"uses a scheme declared as non-canonical.";
}

return (importer, result, originalUrl: resolved);
return (importer, result, originalUrl: url);
}

/// Tries to import [url] using one of this cache's importers.
Expand Down
12 changes: 11 additions & 1 deletion lib/src/compile.dart
View file
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_compile.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: ab2c6fa2588988a86abdbe87512134098e01b39e
// Checksum: 7e80ff5e991b0815e71b91b23a869222f12cf90b
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -127,6 +127,16 @@ CompileResult compileString(String source,
futureDeprecations: {...?futureDeprecations},
limitRepetition: !verbose);

// Allow NoOpImporter because various first-party callers use that to opt out
// of the also-deprecated FilesystemImporter.cwd behavior.
if (importer != null && importer is! NoOpImporter && url == null) {
logger.warnForDeprecation(
Deprecation.importerWithoutUrl,
"Passing an importer to compileString* without a canonical URL is "
"deprecated and will be an error in future versions of Sass. Use the "
"importers argument for non-relative loads.");
}

var stylesheet =
Stylesheet.parse(source, syntax ?? Syntax.scss, url: url, logger: logger);

Expand Down
5 changes: 5 additions & 0 deletions lib/src/deprecation.dart
View file
Expand Up @@ -74,6 +74,11 @@ enum Deprecation {
description:
'Using the current working directory as an implicit load path.'),

importerWithoutUrl('importer-without-url',
deprecatedIn: '1.75.0',
description:
'Passing a base importer without a base URL to compileString*.'),

@Deprecated('This deprecation name was never actually used.')
calcInterp('calc-interp', deprecatedIn: null),

Expand Down
5 changes: 2 additions & 3 deletions lib/src/executable/compile_stylesheet.dart
View file
Expand Up @@ -68,13 +68,12 @@ Future<(int, String, String?)?> compileStylesheet(ExecutableOptions options,
Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
StylesheetGraph graph, String? source, String? destination,
{bool ifModified = false}) async {
var importer = FilesystemImporter.cwd;
if (ifModified) {
try {
if (source != null &&
destination != null &&
!graph.modifiedSince(p.toUri(p.absolute(source)),
modificationTime(destination), importer)) {
!graph.modifiedSince(
p.toUri(p.absolute(source)), modificationTime(destination))) {
return;
}
} on FileSystemException catch (_) {
Expand Down
3 changes: 1 addition & 2 deletions lib/src/executable/repl.dart
View file
Expand Up @@ -22,9 +22,8 @@ Future<void> repl(ExecutableOptions options) async {
var repl = Repl(prompt: '>> ');
var logger = TrackingLogger(options.logger);
var evaluator = Evaluator(
importer: FilesystemImporter.cwd,
importCache: ImportCache(
importers: options.pkgImporters,
importers: [...options.pkgImporters, FilesystemImporter('.')],
loadPaths: options.loadPaths,
logger: logger),
logger: logger);
Expand Down
22 changes: 9 additions & 13 deletions lib/src/import_cache.dart
View file
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_import_cache.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: d157b83599dbc07a80ac6cb5ffdf5dde03b60376
// Checksum: 01bc58f71fe5862d20d4c9ab7d8c7ba1eb612b7f
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -154,6 +154,8 @@ final class ImportCache {
}

if (baseImporter != null && url.scheme == '') {
// TODO(sass/sass#3831): Throw an ArgumentError here if baseImporter is
// passed without a baseUrl as well.
var relativeResult = _relativeCanonicalizeCache.putIfAbsent(
(
url,
Expand All @@ -179,17 +181,11 @@ final class ImportCache {

/// Calls [importer.canonicalize] and prints a deprecation warning if it
/// returns a relative URL.
///
/// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl]
/// before passing it to [importer].
CanonicalizeResult? _canonicalize(
Importer importer, Uri url, Uri? baseUrl, bool forImport,
{bool resolveUrl = false}) {
var resolved =
resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url;
Importer importer, Uri url, Uri? baseUrl, bool forImport) {
var canonicalize = forImport
? () => inImportRule(() => importer.canonicalize(resolved))
: () => importer.canonicalize(resolved);
? () => inImportRule(() => importer.canonicalize(url))
: () => importer.canonicalize(url);

var passContainingUrl = baseUrl != null &&
(url.scheme == '' || importer.isNonCanonicalScheme(url.scheme));
Expand All @@ -200,15 +196,15 @@ final class ImportCache {
if (result.scheme == '') {
_logger.warnForDeprecation(
Deprecation.relativeCanonical,
"Importer $importer canonicalized $resolved to $result.\n"
"Importer $importer canonicalized $url to $result.\n"
"Relative canonical URLs are deprecated and will eventually be "
"disallowed.");
} else if (importer.isNonCanonicalScheme(result.scheme)) {
throw "Importer $importer canonicalized $resolved to $result, which "
throw "Importer $importer canonicalized $url to $result, which "
"uses a scheme declared as non-canonical.";
}

return (importer, result, originalUrl: resolved);
return (importer, result, originalUrl: url);
}

/// Tries to import [url] using one of this cache's importers.
Expand Down
18 changes: 5 additions & 13 deletions lib/src/stylesheet_graph.dart
View file
Expand Up @@ -41,12 +41,8 @@ class StylesheetGraph {
/// 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]) {
bool modifiedSince(Uri url, DateTime since) {
DateTime transitiveModificationTime(StylesheetNode node) {
return _transitiveModificationTimes.putIfAbsent(node.canonicalUrl, () {
var latest = node.importer.modificationTime(node.canonicalUrl);
Expand All @@ -63,22 +59,18 @@ class StylesheetGraph {
});
}

var node = _add(url, baseImporter, baseUrl);
var node = _add(url);
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 result = _ignoreErrors(() => importCache.canonicalize(url,
baseImporter: baseImporter, baseUrl: baseUrl));
if (result case (var importer, var canonicalUrl, :var originalUrl)) {
StylesheetNode? _add(Uri url) {
if (_ignoreErrors(() => importCache.canonicalize(url))
case (var importer, var canonicalUrl, :var originalUrl)) {
addCanonical(importer, canonicalUrl, originalUrl);
return nodes[canonicalUrl];
} else {
Expand Down
41 changes: 18 additions & 23 deletions lib/src/visitor/async_evaluate.dart
View file
Expand Up @@ -85,6 +85,8 @@ Future<EvaluateResult> evaluateAsync(Stylesheet stylesheet,
Logger? logger,
bool quietDeps = false,
bool sourceMap = false}) =>
// TODO(sass/sass#3831): Throw an ArgumentError here if importer is passed
// but stylesheet doesn't have a URL.
_EvaluateVisitor(
importCache: importCache,
nodeImporter: nodeImporter,
Expand All @@ -100,28 +102,23 @@ final class AsyncEvaluator {
/// The visitor that evaluates each expression and statement.
final _EvaluateVisitor _visitor;

/// The importer to use to resolve `@use` rules in [_visitor].
final AsyncImporter? _importer;

/// Creates an evaluator.
///
/// Arguments are the same as for [evaluateAsync].
AsyncEvaluator(
{AsyncImportCache? importCache,
AsyncImporter? importer,
Iterable<AsyncCallable>? functions,
Logger? logger})
: _visitor = _EvaluateVisitor(
importCache: importCache, functions: functions, logger: logger),
_importer = importer;
importCache: importCache, functions: functions, logger: logger);

Future<void> use(UseRule use) => _visitor.runStatement(_importer, use);
Future<void> use(UseRule use) => _visitor.runStatement(use);

Future<Value> evaluate(Expression expression) =>
_visitor.runExpression(_importer, expression);
_visitor.runExpression(expression);

Future<void> setVariable(VariableDeclaration declaration) =>
_visitor.runStatement(_importer, declaration);
_visitor.runStatement(declaration);
}

/// A visitor that executes Sass code to produce a CSS tree.
Expand Down Expand Up @@ -605,17 +602,15 @@ final class _EvaluateVisitor
}
}

Future<Value> runExpression(AsyncImporter? importer, Expression expression) =>
withEvaluationContext(
_EvaluationContext(this, expression),
() => _withFakeStylesheet(importer, expression,
() => _addExceptionTrace(() => expression.accept(this))));
Future<Value> runExpression(Expression expression) => withEvaluationContext(
_EvaluationContext(this, expression),
() => _withFakeStylesheet(
expression, () => _addExceptionTrace(() => expression.accept(this))));

Future<void> runStatement(AsyncImporter? importer, Statement statement) =>
withEvaluationContext(
_EvaluationContext(this, statement),
() => _withFakeStylesheet(importer, statement,
() => _addExceptionTrace(() => statement.accept(this))));
Future<void> runStatement(Statement statement) => withEvaluationContext(
_EvaluationContext(this, statement),
() => _withFakeStylesheet(
statement, () => _addExceptionTrace(() => statement.accept(this))));

/// Asserts that [value] is not `null` and returns it.
///
Expand All @@ -627,12 +622,12 @@ final class _EvaluateVisitor
throw StateError("Can't access $name outside of a module.");
}

/// Runs [callback] with [importer] as [_importer] and a fake [_stylesheet]
/// with [nodeWithSpan]'s source span.
Future<T> _withFakeStylesheet<T>(AsyncImporter? importer,
/// Runs [callback] with a fake [_stylesheet] with [nodeWithSpan]'s source
/// span.
Future<T> _withFakeStylesheet<T>(
AstNode nodeWithSpan, FutureOr<T> callback()) async {
var oldImporter = _importer;
_importer = importer;
_importer = null;

assert(__stylesheet == null);
_stylesheet = Stylesheet(const [], nodeWithSpan.span);
Expand Down

0 comments on commit b6bf5bb

Please sign in to comment.