JS API: Validate that importer result 'contents' is a string and improve ArgumentError output (#1816)

* Validate ImporterResult 'contents' and improve ArgumentError output
* only use JS stuff in the nodejs bindings
* handle non-string contents for legacy importer too
* make it work with node 12

Author: Goodwine

CHANGELOG.md

@@ -60,6 +60,12 @@
 * Emit a deprecation warning when passing a `sassIndex` with units to
   `Value.sassIndexToListIndex()`. This will be an error in Dart Sass 2.0.0.
 
+### JS API
+
+* Importer results now validate whether `contents` is actually a string type.
+
+* Importer result argument errors are now rendered correctly.
+
 ## 1.55.0
 
 * **Potentially breaking bug fix:** Sass numbers are now universally stored as

lib/src/importer/legacy_node/implementation.dart

@@ -183,6 +183,11 @@ class NodeImporter {
 
     var file = value.file;
     var contents = value.contents;
+    if (contents != null && !isJsString(contents)) {
+      jsThrow(ArgumentError.value(contents, 'contents',
+          'must be a string but was: ${jsType(contents)}'));
+    }
+
     if (file == null) {
       return Tuple2(contents ?? '', url);
     } else if (contents != null) {

lib/src/importer/node_to_dart/async.dart

@@ -43,6 +43,11 @@ class NodeToDartAsyncImporter extends AsyncImporter {
 
     result as NodeImporterResult;
     var contents = result.contents;
+    if (!isJsString(contents)) {
+      jsThrow(ArgumentError.value(contents, 'contents',
+          'must be a string but was: ${jsType(contents)}'));
+    }
+
     var syntax = result.syntax;
     if (contents == null || syntax == null) {
       jsThrow(JsError("The load() function must return an object with contents "

lib/src/importer/node_to_dart/sync.dart

@@ -48,6 +48,11 @@ class NodeToDartImporter extends Importer {
 
     result as NodeImporterResult;
     var contents = result.contents;
+    if (!isJsString(contents)) {
+      jsThrow(ArgumentError.value(contents, 'contents',
+          'must be a string but was: ${jsType(contents)}'));
+    }
+
     var syntax = result.syntax;
     if (contents == null || syntax == null) {
       jsThrow(JsError("The load() function must return an object with contents "

lib/src/node/function.dart

@@ -6,6 +6,31 @@ import 'package:js/js.dart';
 
 @JS("Function")
 class JSFunction {
+  /// Creates a [JS function].
+  ///
+  /// The **last** argument is the function body. The other arguments become the
+  /// function's parameters.
+  ///
+  /// The function body must declare a `return` statement in order to return a
+  /// value, otherwise it returns [JSNull].
+  ///
+  /// Note: The function body must be compatible with Node 12. Null coalescing
+  /// and optional chaining features are not supported.
+  ///
+  /// Examples:
+  /// ```dart
+  /// var sum = JSFunction('a', 'b', 'return a + b');
+  /// sum.call(13, 29) as int; // 42
+  ///
+  /// var isJsString = JSFunction('a', 'return typeof a === "string"');
+  /// isJsString.call(42) as bool;   // false
+  /// isJsString.call('42') as bool; // true
+  ///
+  /// var sayHi = JSFunction('console.log("Hi!")');
+  /// sayHi.call(); // Logs "Hi!"
+  /// ```
+  ///
+  /// [JS Function]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/Function#syntax
   external JSFunction(String arg1, [String? arg2, String? arg3]);
 
   // Note that this just invokes the function with the given arguments, rather

lib/src/node/utils.dart

@@ -72,6 +72,25 @@ void jsForEach(Object object, void callback(String key, Object? value)) {
 /// returns `null`.
 Object? jsEval(String js) => JSFunction('', js).call();
 
+/// Returns whether the [object] is a JS `string`.
+bool isJsString(Object? object) => _jsTypeOf(object) == 'string';
+
+/// Returns the [object]'s `typeof` according to the JS engine.
+String _jsTypeOf(Object? object) =>
+    JSFunction("value", "return typeof value").call(object) as String;
+
+/// Returns `typeof value` if [value] is a native type, otherwise returns the
+/// [value]'s JS class name.
+String jsType(Object? value) {
+  var typeOf = _jsTypeOf(value);
+  return typeOf != 'object' ? typeOf : JSFunction('value', '''
+    if (value && value.constructor && value.constructor.name) {
+      return value.constructor.name;
+    }
+    return "object";
+  ''').call(value) as String;
+}
+
 @JS("Object.defineProperty")
 external void _defineProperty(
     Object object, String name, _PropertyDescriptor prototype);

lib/src/visitor/async_evaluate.dart

@@ -1645,6 +1645,8 @@ class _EvaluateVisitor
       }
     } on SassException catch (error, stackTrace) {
       throwWithTrace(_exception(error.message, error.span), stackTrace);
+    } on ArgumentError catch (error, stackTrace) {
+      throwWithTrace(_exception(error.toString()), stackTrace);
     } catch (error, stackTrace) {
       String? message;
       try {

lib/src/visitor/evaluate.dart

@@ -5,7 +5,7 @@
 // DO NOT EDIT. This file was generated from async_evaluate.dart.
 // See tool/grind/synchronize.dart for details.
 //
-// Checksum: c70a4193cc291f298f601a5cc371be9eac71fb74
+// Checksum: a14e075a5435c7457d1d1371d8b97dd327a66ec4
 //
 // ignore_for_file: unused_import
 
@@ -1643,6 +1643,8 @@ class _EvaluateVisitor
       }
     } on SassException catch (error, stackTrace) {
       throwWithTrace(_exception(error.message, error.span), stackTrace);
+    } on ArgumentError catch (error, stackTrace) {
+      throwWithTrace(_exception(error.toString()), stackTrace);
     } catch (error, stackTrace) {
       String? message;
       try {