From 671338317f5c795e1199b70e041d38178161bb11 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Wed, 29 Nov 2017 16:44:39 -0800 Subject: [PATCH] Add support for async importers to the JS render() function Closes #9 --- README.md | 13 +-- lib/src/importer/node/implementation.dart | 84 +++++++++++++----- lib/src/importer/node/interface.dart | 6 +- lib/src/node.dart | 100 +++++++++++++++------- lib/src/node/utils.dart | 10 +++ lib/src/visitor/async_evaluate.dart | 2 +- lib/src/visitor/evaluate.dart | 2 +- test/node_api/api.dart | 12 +++ test/node_api/importer_test.dart | 47 ++++++++++ test/node_api/utils.dart | 10 ++- 10 files changed, 220 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 5c7f7198a..7e2773b14 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,12 @@ That's it! When installed via NPM, Dart Sass supports a JavaScript API that aims to be compatible with [Node Sass](https://github.com/sass/node-sass#usage). Full compatibility is a work in progress, but Dart Sass currently supports the -`render()` and `renderSync()` functions with the following options: +`render()` and `renderSync()` functions. Note however that **`renderSync()` is +much faster than `render()`**, due to the overhead of asynchronous callbacks. +It's highly recommended that users use `renderSync()` unless they absolutely +require support for asynchronous importers. + +Both `render()` and `renderSync()` support the following options: * [`file`](https://github.com/sass/node-sass#file) * [`data`](https://github.com/sass/node-sass#data) @@ -122,13 +127,9 @@ compatibility is a work in progress, but Dart Sass currently supports the * [`indentType`](https://github.com/sass/node-sass#indenttype) * [`indentWidth`](https://github.com/sass/node-sass#indentwidth) * [`linefeed`](https://github.com/sass/node-sass#linefeed) +* [`importer`](https://github.com/sass/node-sass#importer--v200---experimental) * Only the `"expanded"` value of [`outputStyle`](https://github.com/sass/node-sass#outputstyle) is supported. -* [`importer`][importer option] is supported, but only for importers that return - values synchronously. The `done()` callback is currently not passed to any - importers, even when running the asynchronous `render()` function. - -[importer option]: https://github.com/sass/node-sass#importer--v200---experimental The following options are not yet supported, but are intended: diff --git a/lib/src/importer/node/implementation.dart b/lib/src/importer/node/implementation.dart index 504497124..435ee6486 100644 --- a/lib/src/importer/node/implementation.dart +++ b/lib/src/importer/node/implementation.dart @@ -2,6 +2,9 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:async'; + +import 'package:js/js.dart'; import 'package:tuple/tuple.dart'; import '../../io.dart'; @@ -10,7 +13,7 @@ import '../../node/utils.dart'; import '../../util/path.dart'; import '../utils.dart'; -typedef _Importer(String url, String prev); +typedef _Importer(String url, String prev, [void done(result)]); /// An importer that encapsulates Node Sass's import logic. /// @@ -67,26 +70,30 @@ class NodeImporter { previous.scheme == 'file' ? p.fromUri(previous) : previous.toString(); for (var importer in _importers) { var value = call2(importer, _context, urlString, previousString); - if (value == null) continue; - if (isJSError(value)) throw value; - - NodeImporterResult result; - try { - result = value as NodeImporterResult; - } on CastError { - // is reports a different result than as here. I can't find a minimal - // reproduction, but it seems likely to be related to sdk#26838. - return null; - } - - if (result.file != null) { - var resolved = _resolvePath(result.file, previous); - if (resolved != null) return resolved; - - throw "Can't find stylesheet to import."; - } else { - return new Tuple2(result.contents ?? '', url); - } + if (value != null) return _handleImportResult(url, previous, value); + } + + return null; + } + + /// Asynchronously loads the stylesheet at [url]. + /// + /// The [previous] URL is the URL of the stylesheet in which the import + /// appeared. Returns the contents of the stylesheet and the URL to use as + /// [previous] for imports within the loaded stylesheet. + Future> loadAsync(Uri url, Uri previous) async { + if (url.scheme == '' || url.scheme == 'file') { + var result = _resolvePath(p.fromUri(url), previous); + if (result != null) return result; + } + + // The previous URL is always an absolute file path for filesystem imports. + var urlString = url.toString(); + var previousString = + previous.scheme == 'file' ? p.fromUri(previous) : previous.toString(); + for (var importer in _importers) { + var value = await _callImporterAsync(importer, urlString, previousString); + if (value != null) return _handleImportResult(url, previous, value); } return null; @@ -129,4 +136,39 @@ class NodeImporter { ? null : new Tuple2(readFile(resolved), p.toUri(resolved)); } + + /// Converts an [_Importer]'s return [value] to a tuple that can be returned + /// by [load]. + Tuple2 _handleImportResult(Uri url, Uri previous, Object value) { + if (isJSError(value)) throw value; + + NodeImporterResult result; + try { + result = value as NodeImporterResult; + } on CastError { + // is reports a different result than as here. I can't find a minimal + // reproduction, but it seems likely to be related to sdk#26838. + return null; + } + + if (result.file != null) { + var resolved = _resolvePath(result.file, previous); + if (resolved != null) return resolved; + + throw "Can't find stylesheet to import."; + } else { + return new Tuple2(result.contents ?? '', url); + } + } + + /// Calls an importer that may or may not be asynchronous. + Future _callImporterAsync( + _Importer importer, String urlString, String previousString) async { + var completer = new Completer(); + + var result = call3(importer, _context, urlString, previousString, + allowInterop(completer.complete)); + if (isUndefined(result)) return await completer.future; + return result; + } } diff --git a/lib/src/importer/node/interface.dart b/lib/src/importer/node/interface.dart index ac887f3b3..c64dff001 100644 --- a/lib/src/importer/node/interface.dart +++ b/lib/src/importer/node/interface.dart @@ -2,13 +2,17 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:async'; + import 'package:tuple/tuple.dart'; -typedef _Importer(String url, String prev); +typedef _Importer(String url, String prev, [void done(result)]); class NodeImporter { NodeImporter(Object context, Iterable includePaths, Iterable<_Importer> importers); Tuple2 load(Uri url, Uri previous) => null; + + Future> loadAsync(Uri url, Uri previous) => null; } diff --git a/lib/src/node.dart b/lib/src/node.dart index 4876da701..7f65dcc41 100644 --- a/lib/src/node.dart +++ b/lib/src/node.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:collection/collection.dart'; import 'package:js/js.dart'; @@ -20,7 +22,7 @@ import 'util/path.dart'; import 'value/number.dart'; import 'visitor/serialize.dart'; -typedef _Importer(String url, String prev); +typedef _Importer(String url, String prev, [void done(result)]); /// The entrypoint for Node.js. /// @@ -46,37 +48,19 @@ void main() { /// [render]: https://github.com/sass/node-sass#options void _render(RenderOptions options, void callback(RenderError error, RenderResult result)) { - try { - callback(null, _doRender(options)); - } on SassException catch (error) { - callback(_wrapException(error), null); - } catch (error) { - callback(newRenderError(error.toString(), status: 3), null); - } -} - -/// Converts Sass to CSS. -/// -/// This attempts to match the [node-sass `renderSync()` API][render] as closely -/// as possible. -/// -/// [render]: https://github.com/sass/node-sass#options -RenderResult _renderSync(RenderOptions options) { - try { - return _doRender(options); - } on SassException catch (error) { - jsThrow(_wrapException(error)); - } catch (error) { - jsThrow(newRenderError(error.toString(), status: 3)); - } - throw "unreachable"; + _renderAsync(options).then((result) { + callback(null, result); + }, onError: (error, stackTrace) { + if (error is SassException) { + callback(_wrapException(error), null); + } else { + callback(newRenderError(error.toString(), status: 3), null); + } + }); } -/// Converts Sass to CSS. -/// -/// Unlike [_render] and [_renderSync], this doesn't do any special handling for -/// Dart exceptions. -RenderResult _doRender(RenderOptions options) { +/// Converts Sass to CSS asynchronously. +Future _renderAsync(RenderOptions options) async { var start = new DateTime.now(); CompileResult result; if (options.data != null) { @@ -85,7 +69,7 @@ RenderResult _doRender(RenderOptions options) { "options.data and options.file may not both be set."); } - result = compileString(options.data, + result = await compileStringAsync(options.data, nodeImporter: _parseImporter(options, start), indented: options.indentedSyntax ?? false, style: _parseOutputStyle(options.outputStyle), @@ -94,7 +78,7 @@ RenderResult _doRender(RenderOptions options) { lineFeed: _parseLineFeed(options.linefeed), url: 'stdin'); } else if (options.file != null) { - result = compile(options.file, + result = await compileAsync(options.file, nodeImporter: _parseImporter(options, start), indented: options.indentedSyntax, style: _parseOutputStyle(options.outputStyle), @@ -114,6 +98,58 @@ RenderResult _doRender(RenderOptions options) { includedFiles: result.includedFiles.toList()); } +/// Converts Sass to CSS. +/// +/// This attempts to match the [node-sass `renderSync()` API][render] as closely +/// as possible. +/// +/// [render]: https://github.com/sass/node-sass#options +RenderResult _renderSync(RenderOptions options) { + try { + var start = new DateTime.now(); + CompileResult result; + if (options.data != null) { + if (options.file != null) { + throw new ArgumentError( + "options.data and options.file may not both be set."); + } + + result = compileString(options.data, + nodeImporter: _parseImporter(options, start), + indented: options.indentedSyntax ?? false, + style: _parseOutputStyle(options.outputStyle), + useSpaces: options.indentType != 'tab', + indentWidth: _parseIndentWidth(options.indentWidth), + lineFeed: _parseLineFeed(options.linefeed), + url: 'stdin'); + } else if (options.file != null) { + result = compile(options.file, + nodeImporter: _parseImporter(options, start), + indented: options.indentedSyntax, + style: _parseOutputStyle(options.outputStyle), + useSpaces: options.indentType != 'tab', + indentWidth: _parseIndentWidth(options.indentWidth), + lineFeed: _parseLineFeed(options.linefeed)); + } else { + throw new ArgumentError( + "Either options.data or options.file must be set."); + } + var end = new DateTime.now(); + + return newRenderResult(result.css, + entry: options.file ?? 'data', + start: start.millisecondsSinceEpoch, + end: end.millisecondsSinceEpoch, + duration: end.difference(start).inMilliseconds, + includedFiles: result.includedFiles.toList()); + } on SassException catch (error) { + jsThrow(_wrapException(error)); + } catch (error) { + jsThrow(newRenderError(error.toString(), status: 3)); + } + throw "unreachable"; +} + /// Converts a [SassException] to a [RenderError]. RenderError _wrapException(SassException exception) { var trace = exception is SassRuntimeException diff --git a/lib/src/node/utils.dart b/lib/src/node/utils.dart index aec5bbdf7..6fd2d2ee0 100644 --- a/lib/src/node/utils.dart +++ b/lib/src/node/utils.dart @@ -23,6 +23,11 @@ void jsThrow(error) => _jsThrow.call(error); final _jsThrow = new JSFunction("error", "throw error;"); +/// Returns whether or not [value] is the JS `undefined` value. +bool isUndefined(value) => _isUndefined.call(value) as bool; + +final _isUndefined = new JSFunction("value", "return value === undefined;"); + @JS("Error") external Function get _JSError; @@ -33,3 +38,8 @@ bool isJSError(value) => instanceof(value, _JSError) as bool; R call2( R function(A1 arg1, A2 arg2), Object thisArg, A1 arg1, A2 arg2) => (function as JSFunction).apply(thisArg, [arg1, arg2]) as R; + +/// Invokes [function] with [thisArg] as `this`. +R call3(R function(A1 arg1, A2 arg2, A3 arg3), Object thisArg, + A1 arg1, A2 arg2, A3 arg3) => + (function as JSFunction).apply(thisArg, [arg1, arg2, arg3]) as R; diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index ff7079a49..432a2277c 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -737,7 +737,7 @@ class _EvaluateVisitor /// /// Returns the [Stylesheet], or `null` if the import failed. Future _importLikeNode(DynamicImport import) async { - var result = _nodeImporter.load(import.url, _baseUrl); + var result = await _nodeImporter.loadAsync(import.url, _baseUrl); if (result == null) return null; var contents = result.item1; diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 96e1c46e9..3dd4d2887 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/synchronize.dart for details. // -// Checksum: ef8fa3966d7580d8511d8d8430a8f65cd9cb9018 +// Checksum: ae64ba442752642066f0e9e038f5f2c1bbfa866b import 'dart:math' as math; diff --git a/test/node_api/api.dart b/test/node_api/api.dart index 7dc634b29..1946c5073 100644 --- a/test/node_api/api.dart +++ b/test/node_api/api.dart @@ -22,9 +22,21 @@ export 'package:sass/src/node/render_result.dart'; /// The Sass module. final sass = _require(p.absolute("build/npm/sass.dart")); +/// A `null` that's guaranteed to be represented by JavaScript's `undefined` +/// value, not by `null`. @JS() external Object get undefined; +/// A `null` that's guaranteed to be represented by JavaScript's `null` value, +/// not by `undefined`. +/// +/// We have to use eval here because otherwise dart2js will inline the null +/// value and then optimize it away. +final Object jsNull = _eval("null"); + +@JS("eval") +external Object _eval(String js); + @JS("process.chdir") external void chdir(String directory); diff --git a/test/node_api/importer_test.dart b/test/node_api/importer_test.dart index 88e915028..dae562c93 100644 --- a/test/node_api/importer_test.dart +++ b/test/node_api/importer_test.dart @@ -13,6 +13,7 @@ import 'package:test/test.dart'; import 'package:sass/src/io.dart'; import 'package:sass/src/util/path.dart'; +import 'package:sass/src/node/utils.dart'; import 'package:sass/src/value/number.dart'; import '../ensure_npm_package.dart'; @@ -534,4 +535,50 @@ void main() { " stdin 1:9 root stylesheet")); }); }); + + group("render()", () { + test("supports asynchronous importers", () { + expect( + render(new RenderOptions( + data: "@import 'foo'", + importer: allowInterop((_, __, done) { + new Future.delayed(Duration.ZERO).then((_) { + done(new NodeImporterResult(contents: 'a {b: c}')); + }); + }))), + completion(equalsIgnoringWhitespace('a { b: c; }'))); + }); + + test("supports asynchronous errors", () { + expect( + renderError(new RenderOptions( + data: "@import 'foo'", + importer: allowInterop((_, __, done) { + new Future.delayed(Duration.ZERO).then((_) { + done(new JSError('oh no')); + }); + }))), + completion(toStringAndMessageEqual("oh no\n" + " stdin 1:9 root stylesheet"))); + }); + + test("supports synchronous importers", () { + expect( + render(new RenderOptions( + data: "@import 'foo'", + importer: allowInterop((_, __, ___) => + new NodeImporterResult(contents: 'a {b: c}')))), + completion(equalsIgnoringWhitespace('a { b: c; }'))); + }); + + test("supports synchronous null returns", () { + expect( + renderError(new RenderOptions( + data: "@import 'foo'", + importer: allowInterop((_, __, ___) => jsNull))), + completion( + toStringAndMessageEqual("Can't find stylesheet to import.\n" + " stdin 1:9 root stylesheet"))); + }); + }); } diff --git a/test/node_api/utils.dart b/test/node_api/utils.dart index 20977b664..f00b82ce5 100644 --- a/test/node_api/utils.dart +++ b/test/node_api/utils.dart @@ -37,10 +37,11 @@ Matcher toStringAndMessageEqual(String text) => predicate((error) { /// Returns the result of rendering via [options] as a string. Future render(RenderOptions options) { var completer = new Completer(); - sass.render(options, allowInterop((error, result) { + sass.render(options, + allowInterop(Zone.current.bindBinaryCallback((error, result) { expect(error, isNull); completer.complete(UTF8.decode(result.css)); - })); + }))); return completer.future; } @@ -48,10 +49,11 @@ Future render(RenderOptions options) { /// error. Future renderError(RenderOptions options) { var completer = new Completer(); - sass.render(options, allowInterop((error, result) { + sass.render(options, + allowInterop(Zone.current.bindBinaryCallback((error, result) { expect(result, isNull); completer.complete(error); - })); + }))); return completer.future; }