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

*ImportCache.canonicalize: Deprecate base importer without URL #2213

Closed
wants to merge 1 commit into from
Closed
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
20 changes: 20 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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: 3 additions & 2 deletions lib/src/embedded/logger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'package:stack_trace/stack_trace.dart';

import '../deprecation.dart';
import '../logger.dart';
import '../util/nullable.dart';
import '../utils.dart';
import 'compilation_dispatcher.dart';
import 'embedded_sass.pb.dart' hide SourceSpan;
Expand All @@ -34,7 +33,9 @@ final class EmbeddedLogger extends LoggerWithDeprecationType {
..type = LogEventType.DEBUG
..message = message
..span = protofySpan(span)
..formatted = (span.start.sourceUrl.andThen(p.prettyUri) ?? '-') +
..formatted = (isRealUrl(span.start.sourceUrl)
? p.prettyUri(span.start.sourceUrl)
: '-') +
':${span.start.line + 1} ' +
(_color ? '\u001b[1mDebug\u001b[0m' : 'DEBUG') +
': $message\n');
Expand Down
7 changes: 4 additions & 3 deletions lib/src/executable/compile_stylesheet.dart
Original file line number Diff line number Diff line change
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 Expand Up @@ -105,6 +104,7 @@ Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
logger: options.logger,
importCache: importCache,
importer: FilesystemImporter.cwd,
url: p.toUri(p.absolute('(stdin)${syntax.extension}')),
style: options.style,
quietDeps: options.quietDeps,
verbose: options.verbose,
Expand All @@ -130,6 +130,7 @@ Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
logger: options.logger,
importCache: graph.importCache,
importer: FilesystemImporter.cwd,
url: p.toUri(p.absolute('(stdin)${syntax.extension}')),
style: options.style,
quietDeps: options.quietDeps,
verbose: options.verbose,
Expand Down
3 changes: 1 addition & 2 deletions lib/src/executable/repl.dart
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
5 changes: 3 additions & 2 deletions lib/src/logger/stderr.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ final class StderrLogger implements Logger {

void debug(String message, SourceSpan span) {
var result = StringBuffer();
var url =
span.start.sourceUrl == null ? '-' : p.prettyUri(span.start.sourceUrl);
var url = isRealUrl(span.start.sourceUrl)
? p.prettyUri(span.start.sourceUrl)
: '-';
result.write('$url:${span.start.line + 1} ');
result.write(color ? '\u001b[1mDebug\u001b[0m' : 'DEBUG');
result.write(': $message');
Expand Down
18 changes: 5 additions & 13 deletions lib/src/stylesheet_graph.dart
Original file line number Diff line number Diff line change
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
11 changes: 11 additions & 0 deletions lib/src/syntax.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;

/// An enum of syntaxes that Sass can parse.
Expand All @@ -27,6 +28,16 @@ enum Syntax {
/// The name of the syntax.
final String _name;

/// The file extension used for this syntax, including the period.
///
/// :nodoc:
@internal
String get extension => switch (this) {
Syntax.sass => '.sass',
Syntax.scss => '.scss',
Syntax.css => 'css'
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be .css ?

};

const Syntax(this._name);

String toString() => _name;
Expand Down
8 changes: 7 additions & 1 deletion lib/src/util/source_map_buffer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,13 @@ class SourceMapBuffer implements StringBuffer {
if (entry.target.offset == target.offset) return;
}

_entries.add(Entry(source, target, null));
_entries.add(Entry(
isRealUrl(source.sourceUrl)
? source
: SourceLocation(source.offset,
sourceUrl: null, line: source.line, column: source.column),
target,
null));
}

void clear() =>
Expand Down
20 changes: 15 additions & 5 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:math' as math;

import 'package:charcode/charcode.dart';
import 'package:collection/collection.dart';
import 'package:path/path.dart' as p;
import 'package:source_span/source_span.dart';
import 'package:stack_trace/stack_trace.dart';
import 'package:string_scanner/string_scanner.dart';
Expand Down Expand Up @@ -207,11 +208,11 @@ int mapHash(Map<Object, Object> map) =>
///
/// By default, the frame's URL is set to `span.sourceUrl`. However, if [url] is
/// passed, it's used instead.
Frame frameForSpan(SourceSpan span, String member, {Uri? url}) => Frame(
url ?? span.sourceUrl ?? _noSourceUrl,
span.start.line + 1,
span.start.column + 1,
member);
Frame frameForSpan(SourceSpan span, String member, {Uri? url}) {
url ??= span.sourceUrl;
return Frame(isRealUrl(url) ? url! : _noSourceUrl, span.start.line + 1,
span.start.column + 1, member);
}

/// Returns the variable name (including the leading `$`) from a [span] that
/// covers a variable declaration, which includes the variable name as well as
Expand Down Expand Up @@ -443,6 +444,15 @@ void attachTrace(Object error, StackTrace trace) {
StackTrace? getTrace(Object error) =>
error is String || error is num || error is bool ? null : _traces[error];

/// Returns whether [url] is both non-null and not a fake URL to represent
/// standard input for the Sass CLI.
bool isRealUrl(Uri? url) =>
url != null &&
url != _noSourceUrl &&
!(url.scheme == 'file' &&
const {'(stdin).sass', '(stdin).scss', '(stdin).css'}
.contains(p.url.basename(url.path)));

/// Parses a function signature of the format allowed by Node Sass's functions
/// option and returns its name and declaration.
///
Expand Down
Loading
Loading