Skip to content

Commit

Permalink
Better handle filesystem importers when load paths aren't necessary (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
nex3 authored Mar 27, 2024
1 parent 9302b35 commit c8d0643
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 28 deletions.
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,31 @@
* Add support for nesting in plain CSS files. This is not processed by Sass at
all; it's emitted exactly as-is in the CSS.

* In certain circumstances, the current working directory was unintentionally
being made available as a load path. This is now deprecated. Anyone relying on
this should explicitly pass in `.` as a load path or `FilesystemImporter('.')`
as the current importer.

* Add linux-riscv64 and windows-arm64 releases.

### Command-Line Interface

* Fix a bug where absolute `file:` URLs weren't loaded for files compiled via
the command line unless an unrelated load path was also passed.

* Fix a bug where `--update` would always update files that were specified via
absolute path unless an unrelated load path was also passed.

### Dart API

* Add `FilesystemImporter.noLoadPath`, which is a `FilesystemImporter` that can
load absolute `file:` URLs and resolve URLs relative to the base file but
doesn't load relative URLs from a load path.

* `FilesystemImporter.cwd` is now deprecated. Either use
`FilesystemImporter.noLoadPath` if you weren't intending to rely on the load
path, or `FilesystemImporter('.')` if you were.

## 1.72.0

* Support adjacent `/`s without whitespace in between when parsing plain CSS
Expand Down
3 changes: 2 additions & 1 deletion bin/sass.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'package:sass/src/executable/options.dart';
import 'package:sass/src/executable/repl.dart';
import 'package:sass/src/executable/watch.dart';
import 'package:sass/src/import_cache.dart';
import 'package:sass/src/importer/filesystem.dart';
import 'package:sass/src/io.dart';
import 'package:sass/src/logger/deprecation_handling.dart';
import 'package:sass/src/stylesheet_graph.dart';
Expand Down Expand Up @@ -45,7 +46,7 @@ Future<void> main(List<String> args) async {
}

var graph = StylesheetGraph(ImportCache(
importers: options.pkgImporters,
importers: [...options.pkgImporters, FilesystemImporter.noLoadPath],
loadPaths: options.loadPaths,
// This logger is only used for handling fatal/future deprecations
// during parsing, and is re-used across parses, so we don't want to
Expand Down
5 changes: 5 additions & 0 deletions lib/src/deprecation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ enum Deprecation {
deprecatedIn: '1.62.3',
description: 'Passing null as alpha in the ${isJS ? 'JS' : 'Dart'} API.'),

fsImporterCwd('fs-importer-cwd',
deprecatedIn: '1.73.0',
description:
'Using the current working directory as an implicit load path.'),

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

Expand Down
35 changes: 23 additions & 12 deletions lib/src/evaluation_context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'dart:async';
import 'package:source_span/source_span.dart';

import 'deprecation.dart';
import 'logger.dart';

/// An interface that exposes information about the current Sass evaluation.
///
Expand All @@ -17,13 +18,16 @@ abstract interface class EvaluationContext {
///
/// Throws [StateError] if there isn't a Sass stylesheet currently being
/// evaluated.
static EvaluationContext get current {
if (Zone.current[#_evaluationContext] case EvaluationContext context) {
return context;
} else {
throw StateError("No Sass stylesheet is currently being evaluated.");
}
}
static EvaluationContext get current =>
currentOrNull ??
(throw StateError("No Sass stylesheet is currently being evaluated."));

/// The current evaluation context, or `null` if none exists.
static EvaluationContext? get currentOrNull =>
switch (Zone.current[#_evaluationContext]) {
EvaluationContext context => context,
_ => null
};

/// Returns the span for the currently executing callable.
///
Expand All @@ -50,13 +54,20 @@ abstract interface class EvaluationContext {
/// This may only be called within a custom function or importer callback.
/// {@category Compile}
void warn(String message, {bool deprecation = false}) =>
EvaluationContext.current
.warn(message, deprecation ? Deprecation.userAuthored : null);
switch (EvaluationContext.currentOrNull) {
var context? =>
context.warn(message, deprecation ? Deprecation.userAuthored : null),
_ when deprecation => (const Logger.stderr())
.warnForDeprecation(Deprecation.userAuthored, message),
_ => (const Logger.stderr()).warn(message)
};

/// Prints a deprecation warning with [message] of type [deprecation].
void warnForDeprecation(String message, Deprecation deprecation) {
EvaluationContext.current.warn(message, deprecation);
}
void warnForDeprecation(String message, Deprecation deprecation) =>
switch (EvaluationContext.currentOrNull) {
var context? => context.warn(message, deprecation),
_ => (const Logger.stderr()).warnForDeprecation(deprecation, message)
};

/// Runs [callback] with [context] as [EvaluationContext.current].
///
Expand Down
4 changes: 2 additions & 2 deletions lib/src/executable/compile_stylesheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ Future<void> _compileStylesheetWithoutErrorHandling(ExecutableOptions options,
try {
if (source != null &&
destination != null &&
!graph.modifiedSince(
p.toUri(source), modificationTime(destination), importer)) {
!graph.modifiedSince(p.toUri(p.absolute(source)),
modificationTime(destination), importer)) {
return;
}
} on FileSystemException catch (_) {
Expand Down
74 changes: 65 additions & 9 deletions lib/src/importer/filesystem.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,30 +5,86 @@
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;

import '../deprecation.dart';
import '../evaluation_context.dart';
import '../importer.dart';
import '../io.dart' as io;
import '../syntax.dart';
import '../util/nullable.dart';
import 'utils.dart';

/// An importer that loads files from a load path on the filesystem.
/// An importer that loads files from a load path on the filesystem, either
/// relative to the path passed to [FilesystemImporter.new] or absolute `file:`
/// URLs.
///
/// Use [FilesystemImporter.noLoadPath] to _only_ load absolute `file:` URLs and
/// URLs relative to the current file.
///
/// {@category Importer}
@sealed
class FilesystemImporter extends Importer {
/// The path relative to which this importer looks for files.
final String _loadPath;
///
/// If this is `null`, this importer will _only_ load absolute `file:` URLs
/// and URLs relative to the current file.
final String? _loadPath;

/// Whether loading from files from this importer's [_loadPath] is deprecated.
final bool _loadPathDeprecated;

/// Creates an importer that loads files relative to [loadPath].
FilesystemImporter(String loadPath) : _loadPath = p.absolute(loadPath);
FilesystemImporter(String loadPath)
: _loadPath = p.absolute(loadPath),
_loadPathDeprecated = false;

FilesystemImporter._deprecated(String loadPath)
: _loadPath = p.absolute(loadPath),
_loadPathDeprecated = true;

/// Creates an importer that _only_ loads absolute `file:` URLs and URLs
/// relative to the current file.
FilesystemImporter._noLoadPath()
: _loadPath = null,
_loadPathDeprecated = false;

/// Creates an importer relative to the current working directory.
static final cwd = FilesystemImporter('.');
/// A [FilesystemImporter] that loads files relative to the current working
/// directory.
///
/// Historically, this was the best default for supporting `file:` URL loads
/// when the load path didn't matter. However, adding the current working
/// directory to the load path wasn't always desirable, so it's no longer
/// recommended. Instead, either use [FilesystemImporter.noLoadPath] if the
/// load path doesn't matter, or `FilesystemImporter('.')` to explicitly
/// preserve the existing behavior.
@Deprecated(
"Use FilesystemImporter.noLoadPath or FilesystemImporter('.') instead.")
static final cwd = FilesystemImporter._deprecated('.');

/// Creates an importer that _only_ loads absolute `file:` URLsand URLs
/// relative to the current file.
static final noLoadPath = FilesystemImporter._noLoadPath();

Uri? canonicalize(Uri url) {
if (url.scheme != 'file' && url.scheme != '') return null;
return resolveImportPath(p.join(_loadPath, p.fromUri(url)))
.andThen((resolved) => p.toUri(io.canonicalize(resolved)));
String? resolved;
if (url.scheme == 'file') {
resolved = resolveImportPath(p.fromUri(url));
} else if (url.scheme != '') {
return null;
} else if (_loadPath case var loadPath?) {
resolved = resolveImportPath(p.join(loadPath, p.fromUri(url)));

if (resolved != null && _loadPathDeprecated) {
warnForDeprecation(
"Using the current working directory as an implicit load path is "
"deprecated. Either add it as an explicit load path or importer, or "
"load this stylesheet from a different URL.",
Deprecation.fsImporterCwd);
}
} else {
return null;
}

return resolved.andThen((resolved) => p.toUri(io.canonicalize(resolved)));
}

ImporterResult? load(Uri url) {
Expand All @@ -53,5 +109,5 @@ class FilesystemImporter extends Importer {
basename == p.url.withoutExtension(canonicalBasename);
}

String toString() => _loadPath;
String toString() => _loadPath ?? '<absolute file importer>';
}
13 changes: 13 additions & 0 deletions test/cli/shared/update.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:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:test_descriptor/test_descriptor.dart' as d;
import 'package:test_process/test_process.dart';
Expand Down Expand Up @@ -148,6 +149,18 @@ void sharedTests(Future<TestProcess> runSass(Iterable<String> arguments)) {
await d.file("out.css", "x {y: z}").validate();
});

// Regression test for #2203
test("whose sources weren't modified with an absolute path", () async {
await d.file("test.scss", "a {b: c}").create();
await d.file("out.css", "x {y: z}").create();

var sass = await update(["${p.absolute(d.path('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();
Expand Down
4 changes: 0 additions & 4 deletions test/dart_api/logger_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,6 @@ void main() {
mustBeCalled();
}));
});

test("throws an error outside a callback", () {
expect(() => warn("heck"), throwsStateError);
});
});
}

Expand Down

0 comments on commit c8d0643

Please sign in to comment.