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

Add the ability for importers to detect @import #1309

Merged
merged 3 commits into from
May 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
## 1.33.0

### JS API

* The `this` context for importers now has a `fromImport` field, which is `true`
if the importer is being invoked from an `@import` and `false` otherwise.
Importers should only use this to determine whether to load [import-only
files].

[import-only files]: https://sass-lang.com/documentation/at-rules/import#import-only-files

### Dart API

* Add an `Importer.fromImport` getter, which is `true` if the current
`Importer.canonicalize()` call comes from an `@import` rule and `false`
otherwise. Importers should only use this to determine whether to load
[import-only files].

## 1.32.13

* **Potentially breaking bug fix:** Null values in `@use` and `@forward`
Expand Down
19 changes: 19 additions & 0 deletions lib/src/importer/async.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

import 'dart:async';

import 'package:meta/meta.dart';

import 'result.dart';
import 'utils.dart' as utils;

/// An interface for importers that resolves URLs in `@import`s to the contents
/// of Sass files.
Expand All @@ -20,6 +23,22 @@ import 'result.dart';
///
/// Subclasses should extend [AsyncImporter], not implement it.
abstract class AsyncImporter {
/// Whether the current [canonicalize] invocation comes from an `@import`
/// rule.
///
/// 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.
///
/// [import-only file]: https://sass-lang.com/documentation/at-rules/import#import-only-files
///
/// Subclasses should only access this from within calls to [canonicalize].
/// Outside of that context, its value is undefined and subject to change.
@protected
@nonVirtual
bool get fromImport => utils.fromImport;

/// If [url] is recognized by this importer, returns its canonical format.
///
/// Note that canonical URLs *must* be absolute, including a scheme. Returning
Expand Down
31 changes: 23 additions & 8 deletions lib/src/importer/node/implementation.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import '../../node/function.dart';
import '../../node/importer_result.dart';
import '../../node/utils.dart';
import '../../util/nullable.dart';
import '../../node/render_context.dart';
import '../utils.dart';

/// An importer that encapsulates Node Sass's import logic.
Expand Down Expand Up @@ -40,8 +41,12 @@ import '../utils.dart';
/// 4. Filesystem imports relative to an `includePaths` path.
/// 5. Filesystem imports relative to a `SASS_PATH` path.
class NodeImporter {
/// The `this` context in which importer functions are invoked.
final Object _context;
/// The options for the `this` context in which importer functions are
/// invoked.
///
/// This is typed as [Object] because the public interface of [NodeImporter]
/// is shared with the VM, which can't handle JS interop types.
final Object _options;

/// The include paths passed in by the user.
final List<String> _includePaths;
Expand All @@ -50,7 +55,7 @@ class NodeImporter {
final List<JSFunction> _importers;

NodeImporter(
this._context, Iterable<String> includePaths, Iterable<Object> importers)
this._options, Iterable<String> includePaths, Iterable<Object> importers)
: _includePaths = List.unmodifiable(_addSassPath(includePaths)),
_importers = List.unmodifiable(importers.cast());

Expand Down Expand Up @@ -78,7 +83,8 @@ class NodeImporter {
// The previous URL is always an absolute file path for filesystem imports.
var previousString = _previousToString(previous);
for (var importer in _importers) {
var value = call2(importer, _context, url, previousString);
var value =
call2(importer, _renderContext(forImport), url, previousString);
if (value != null) {
return _handleImportResult(url, previous, value, forImport);
}
Expand All @@ -103,7 +109,8 @@ class NodeImporter {
// The previous URL is always an absolute file path for filesystem imports.
var previousString = _previousToString(previous);
for (var importer in _importers) {
var value = await _callImporterAsync(importer, url, previousString);
var value =
await _callImporterAsync(importer, url, previousString, forImport);
if (value != null) {
return _handleImportResult(url, previous, value, forImport);
}
Expand Down Expand Up @@ -193,13 +200,21 @@ class NodeImporter {
}

/// Calls an importer that may or may not be asynchronous.
Future<Object?> _callImporterAsync(
JSFunction importer, String url, String previousString) async {
Future<Object?> _callImporterAsync(JSFunction importer, String url,
String previousString, bool forImport) async {
var completer = Completer<Object>();

var result = call3(importer, _context, url, previousString,
var result = call3(importer, _renderContext(forImport), url, previousString,
allowInterop(completer.complete));
if (isUndefined(result)) return await completer.future;
return result;
}

/// Returns the [RenderContext] in which to invoke importers.
RenderContext _renderContext(bool fromImport) {
var context = RenderContext(
options: _options as RenderContextOptions, fromImport: fromImport);
context.options.context = context;
return context;
}
}
2 changes: 1 addition & 1 deletion lib/src/importer/node/interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import 'package:tuple/tuple.dart';

class NodeImporter {
NodeImporter(Object context, Iterable<String> includePaths,
NodeImporter(Object options, Iterable<String> includePaths,
Iterable<Object> importers);

Tuple2<String, String>? load(String url, Uri? previous, bool forImport) =>
Expand Down
34 changes: 9 additions & 25 deletions lib/src/importer/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// MIT-style license that can be found in the LICENSE file or at
// https://opensource.org/licenses/MIT.

import 'dart:async';

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

import '../io.dart';
Expand All @@ -13,30 +15,12 @@ import '../io.dart';
/// 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 `@import`
/// semantics rather than `@use` semantics.
T inImportRule<T>(T callback()) {
var wasInImportRule = _inImportRule;
_inImportRule = true;
try {
return callback();
} finally {
_inImportRule = wasInImportRule;
}
}
bool get fromImport => Zone.current[#_inImportRule] as bool? ?? false;

/// Like [inImportRule], but asynchronous.
Future<T> inImportRuleAsync<T>(Future<T> callback()) async {
var wasInImportRule = _inImportRule;
_inImportRule = true;
try {
return await callback();
} finally {
_inImportRule = wasInImportRule;
}
}
/// Runs [callback] in a context where [inImportRule] returns `true` and
/// [resolveImportPath] uses `@import` semantics rather than `@use` semantics.
T inImportRule<T>(T callback()) =>
runZoned(callback, zoneValues: {#_inImportRule: true});
Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason why this is now run in a zone, or is it just a cleaner way to update fromImport?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making the variable Zone-scoped means that, if multiple Sass compilations are running in parallel, they won't stomp on one another's values for this variable. It wasn't necessary before because this was only used by FileSystemImporter which was synchronous, but user-defined importers aren't necessarily.


/// Resolves an imported path using the same logic as the filesystem importer.
///
Expand Down Expand Up @@ -95,7 +79,7 @@ String? _exactlyOne(List<String> paths) {
paths.map((path) => " " + p.prettyUri(p.toUri(path))).join("\n");
}

/// If [_inImportRule] is `true`, invokes callback and returns the result.
/// If [fromImport] is `true`, invokes callback and returns the result.
///
/// Otherwise, returns `null`.
T? _ifInImport<T>(T callback()) => _inImportRule ? callback() : null;
T? _ifInImport<T>(T callback()) => fromImport ? callback() : null;
45 changes: 21 additions & 24 deletions lib/src/node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ List<AsyncCallable> _parseFunctions(RenderOptions options, DateTime start,
'Invalid signature "$signature": ${error.message}', error.span);
}

var context = _contextWithOptions(options, start);
var context = RenderContext(options: _contextOptions(options, start));
context.options.context = context;

var fiber = options.fiber;
if (fiber != null) {
Expand Down Expand Up @@ -261,9 +262,8 @@ NodeImporter _parseImporter(RenderOptions options, DateTime start) {
importers = [options.importer as JSFunction];
}

var context = importers.isNotEmpty
? _contextWithOptions(options, start)
: const Object();
var contextOptions =
importers.isNotEmpty ? _contextOptions(options, start) : Object();

var fiber = options.fiber;
if (fiber != null) {
Expand All @@ -288,29 +288,26 @@ NodeImporter _parseImporter(RenderOptions options, DateTime start) {
}

var includePaths = List<String>.from(options.includePaths ?? []);
return NodeImporter(context, includePaths, importers);
return NodeImporter(contextOptions, includePaths, importers);
}

/// Creates a `this` context that contains the render options.
RenderContext _contextWithOptions(RenderOptions options, DateTime start) {
/// Creates the [RenderContextOptions] for the `this` context in which custom
/// functions and importers will be evaluated.
RenderContextOptions _contextOptions(RenderOptions options, DateTime start) {
var includePaths = List<String>.from(options.includePaths ?? []);
var context = RenderContext(
options: RenderContextOptions(
file: options.file,
data: options.data,
includePaths:
([p.current, ...includePaths]).join(isWindows ? ';' : ':'),
precision: SassNumber.precision,
style: 1,
indentType: options.indentType == 'tab' ? 1 : 0,
indentWidth: _parseIndentWidth(options.indentWidth) ?? 2,
linefeed: _parseLineFeed(options.linefeed).text,
result: RenderContextResult(
stats: RenderContextResultStats(
start: start.millisecondsSinceEpoch,
entry: options.file ?? 'data'))));
context.options.context = context;
return context;
return RenderContextOptions(
file: options.file,
data: options.data,
includePaths: ([p.current, ...includePaths]).join(isWindows ? ';' : ':'),
precision: SassNumber.precision,
style: 1,
indentType: options.indentType == 'tab' ? 1 : 0,
indentWidth: _parseIndentWidth(options.indentWidth) ?? 2,
linefeed: _parseLineFeed(options.linefeed).text,
result: RenderContextResult(
stats: RenderContextResultStats(
start: start.millisecondsSinceEpoch,
entry: options.file ?? 'data')));
}

/// Parse [style] into an [OutputStyle].
Expand Down
4 changes: 3 additions & 1 deletion lib/src/node/render_context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import 'package:js/js.dart';
@anonymous
class RenderContext {
external RenderContextOptions get options;
external bool? get fromImport;

external factory RenderContext({required RenderContextOptions options});
external factory RenderContext(
{required RenderContextOptions options, bool? fromImport});
}

@JS()
Expand Down
2 changes: 1 addition & 1 deletion lib/src/parse/sass.dart
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ class SassParser extends StylesheetParser {
do {
containsTab = false;
containsSpace = false;
nextIndentation = 0;
nextIndentation = 0;

while (true) {
var next = scanner.peekChar();
Expand Down
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.32.13
version: 1.33.0-dev
description: A Sass implementation in Dart.
author: Sass Team
homepage: https://github.com/sass/dart-sass
Expand Down
28 changes: 28 additions & 0 deletions test/dart_api/from_import_importer.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2017 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:sass/sass.dart';
import 'package:test/test.dart';

/// An [Importer] whose [canonicalize] method asserts the value of
/// [Importer.fromImport].
class FromImportImporter extends Importer {
/// The expected value of [Importer.fromImport] in the call to [canonicalize].
final bool _expected;

/// The callback to call once [canonicalize] is called.
///
/// This ensures that the test doesn't exit until [canonicalize] is called.
final void Function() _done;

FromImportImporter(this._expected) : _done = expectAsync0(() {});

Uri? canonicalize(Uri url) {
expect(fromImport, equals(_expected));
_done();
return Uri.parse('u:');
}

ImporterResult? load(Uri url) => ImporterResult("", syntax: Syntax.scss);
}
20 changes: 20 additions & 0 deletions test/dart_api/importer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'package:test/test.dart';
import 'package:sass/sass.dart';
import 'package:sass/src/exception.dart';

import 'from_import_importer.dart';
import 'test_importer.dart';
import '../utils.dart';

Expand Down Expand Up @@ -182,4 +183,23 @@ void main() {
return true;
})));
});

group("currentLoadFromImport is", () {
test("true from an @import", () {
compileString('@import "foo"', importers: [FromImportImporter(true)]);
});

test("false from a @use", () {
compileString('@use "foo"', importers: [FromImportImporter(false)]);
});

test("false from a @forward", () {
compileString('@forward "foo"', importers: [FromImportImporter(false)]);
});

test("false from meta.load-css", () {
compileString('@use "sass:meta"; @include meta.load-css("")',
importers: [FromImportImporter(false)]);
});
});
}