Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Better handle filesystem importers when load paths aren't necessary
Browse files Browse the repository at this point in the history
nex3 committed Mar 21, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 772280a commit 726296f
Showing 5 changed files with 89 additions and 12 deletions.
25 changes: 24 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,30 @@
## 1.72.1
## 1.73.0

* 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
3 changes: 2 additions & 1 deletion bin/sass.dart
Original file line number Diff line number Diff line change
@@ -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';
@@ -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
4 changes: 4 additions & 0 deletions lib/src/deprecation.dart
Original file line number Diff line number Diff line change
@@ -69,6 +69,10 @@ 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),

67 changes: 58 additions & 9 deletions lib/src/importer/filesystem.dart
Original file line number Diff line number Diff line change
@@ -5,30 +5,79 @@
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
/// recommneded. 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?) {
if (_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);
}

resolved = resolveImportPath(p.join(loadPath, p.fromUri(url)));
} else {
return null;
}

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

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

String toString() => _loadPath;
String toString() => _loadPath ?? '<absolute file importer>';
}
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: sass
version: 1.72.1-dev
version: 1.73.0-dev
description: A Sass implementation in Dart.
homepage: https://github.com/sass/dart-sass

0 comments on commit 726296f

Please sign in to comment.