From 8a344e343f60f488b817f359b76d3def77bb9f14 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 13 May 2021 17:27:04 -0700 Subject: [PATCH] Add `this.fromImport` for JS importers See sass/sass#3055 See webpack-contrib/sass-loader#905 --- CHANGELOG.md | 11 ++++++ lib/src/importer/node/implementation.dart | 27 +++++++++----- lib/src/importer/node/interface.dart | 2 +- lib/src/node.dart | 44 +++++++++++------------ lib/src/node/render_context.dart | 4 ++- lib/src/parse/sass.dart | 2 +- pubspec.yaml | 2 +- test/node_api/importer_test.dart | 42 ++++++++++++++++++++++ 8 files changed, 98 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f38ea879e..c75fcaac9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +## 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 + ## 1.32.13 * **Potentially breaking bug fix:** Null values in `@use` and `@forward` diff --git a/lib/src/importer/node/implementation.dart b/lib/src/importer/node/implementation.dart index 019395c19..9af892575 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,17 @@ 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) => RenderContext( + options: _options as RenderContextOptions, fromImport: fromImport); } 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/node.dart b/lib/src/node.dart index 89c8e6dfa..ca07b1f7a 100644 --- a/lib/src/node.dart +++ b/lib/src/node.dart @@ -204,7 +204,7 @@ 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)); var fiber = options.fiber; if (fiber != null) { @@ -261,9 +261,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 +287,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/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", () {