diff --git a/CHANGELOG.md b/CHANGELOG.md index f38ea879e..1696968b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` diff --git a/lib/src/importer/async.dart b/lib/src/importer/async.dart index 3d2591bac..cc61c8fb3 100644 --- a/lib/src/importer/async.dart +++ b/lib/src/importer/async.dart @@ -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. @@ -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 diff --git a/lib/src/importer/node/implementation.dart b/lib/src/importer/node/implementation.dart index 019395c19..823ec5914 100644 --- a/lib/src/importer/node/implementation.dart +++ b/lib/src/importer/node/implementation.dart @@ -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. @@ -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 _includePaths; @@ -50,7 +55,7 @@ class NodeImporter { final List _importers; NodeImporter( - this._context, Iterable includePaths, Iterable importers) + this._options, Iterable includePaths, Iterable importers) : _includePaths = List.unmodifiable(_addSassPath(includePaths)), _importers = List.unmodifiable(importers.cast()); @@ -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); } @@ -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); } @@ -193,13 +200,21 @@ class NodeImporter { } /// Calls an importer that may or may not be asynchronous. - Future _callImporterAsync( - JSFunction importer, String url, String previousString) async { + Future _callImporterAsync(JSFunction importer, String url, + String previousString, bool forImport) async { var completer = Completer(); - 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; + } } diff --git a/lib/src/importer/node/interface.dart b/lib/src/importer/node/interface.dart index fb65865b9..e280d27bc 100644 --- a/lib/src/importer/node/interface.dart +++ b/lib/src/importer/node/interface.dart @@ -5,7 +5,7 @@ import 'package:tuple/tuple.dart'; class NodeImporter { - NodeImporter(Object context, Iterable includePaths, + NodeImporter(Object options, Iterable includePaths, Iterable importers); Tuple2? load(String url, Uri? previous, bool forImport) => diff --git a/lib/src/importer/utils.dart b/lib/src/importer/utils.dart index 6e5d51fef..b7eac3ac2 100644 --- a/lib/src/importer/utils.dart +++ b/lib/src/importer/utils.dart @@ -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'; @@ -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 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 inImportRuleAsync(Future 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 callback()) => + runZoned(callback, zoneValues: {#_inImportRule: true}); /// Resolves an imported path using the same logic as the filesystem importer. /// @@ -95,7 +79,7 @@ String? _exactlyOne(List 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 callback()) => _inImportRule ? callback() : null; +T? _ifInImport(T callback()) => fromImport ? callback() : null; diff --git a/lib/src/node.dart b/lib/src/node.dart index 89c8e6dfa..5a430dac2 100644 --- a/lib/src/node.dart +++ b/lib/src/node.dart @@ -204,7 +204,8 @@ List _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) { @@ -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) { @@ -288,29 +288,26 @@ NodeImporter _parseImporter(RenderOptions options, DateTime start) { } var includePaths = List.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.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]. diff --git a/lib/src/node/render_context.dart b/lib/src/node/render_context.dart index 8fccdabf4..d42db52f5 100644 --- a/lib/src/node/render_context.dart +++ b/lib/src/node/render_context.dart @@ -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() diff --git a/lib/src/parse/sass.dart b/lib/src/parse/sass.dart index db03e8949..3fc0d9943 100644 --- a/lib/src/parse/sass.dart +++ b/lib/src/parse/sass.dart @@ -414,7 +414,7 @@ class SassParser extends StylesheetParser { do { containsTab = false; containsSpace = false; - nextIndentation = 0; + nextIndentation = 0; while (true) { var next = scanner.peekChar(); diff --git a/pubspec.yaml b/pubspec.yaml index 9ff25ca83..0aaff2eaa 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -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 diff --git a/test/dart_api/from_import_importer.dart b/test/dart_api/from_import_importer.dart new file mode 100644 index 000000000..bfab408ab --- /dev/null +++ b/test/dart_api/from_import_importer.dart @@ -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); +} diff --git a/test/dart_api/importer_test.dart b/test/dart_api/importer_test.dart index 7ffae8f9c..430ec00e2 100644 --- a/test/dart_api/importer_test.dart +++ b/test/dart_api/importer_test.dart @@ -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'; @@ -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)]); + }); + }); } diff --git a/test/node_api/importer_test.dart b/test/node_api/importer_test.dart index 2a2c47f95..a0cb89c49 100644 --- a/test/node_api/importer_test.dart +++ b/test/node_api/importer_test.dart @@ -556,6 +556,48 @@ void main() { })))); }); }); + + group("includes a fromImport field that's", () { + test("true for an @import", () { + renderSync(RenderOptions( + data: '@import "foo"', + importer: allowInteropCaptureThis( + expectAsync3((RenderContext this_, _, __) { + expect(this_.fromImport, isTrue); + return NodeImporterResult(contents: ''); + })))); + }); + + test("false for a @use", () { + renderSync(RenderOptions( + data: '@use "foo"', + importer: allowInteropCaptureThis( + expectAsync3((RenderContext this_, _, __) { + expect(this_.fromImport, isFalse); + return NodeImporterResult(contents: ''); + })))); + }); + + test("false for a @forward", () { + renderSync(RenderOptions( + data: '@forward "foo"', + importer: allowInteropCaptureThis( + expectAsync3((RenderContext this_, _, __) { + expect(this_.fromImport, isFalse); + return NodeImporterResult(contents: ''); + })))); + }); + + test("false for meta.load-css()", () { + renderSync(RenderOptions( + data: '@use "sass:meta"; @include meta.load-css("foo")', + importer: allowInteropCaptureThis( + expectAsync3((RenderContext this_, _, __) { + expect(this_.fromImport, isFalse); + return NodeImporterResult(contents: ''); + })))); + }); + }); }); group("gracefully handles an error when", () {