Support user generated custom native JS classes.

tests/html/js_typed_interop_test.dart

Index: tests/html/js_typed_interop_test.dart
diff --git a/tests/html/js_typed_interop_test.dart b/tests/html/js_typed_interop_test.dart
index c8c02600743cdb4c345d992d4f709891ccfc16f7..2e566360a08de179ec68ca4d6ab7cf08bc469661 100644
--- a/tests/html/js_typed_interop_test.dart
+++ b/tests/html/js_typed_interop_test.dart
@@ -2,7 +2,8 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-library jsArrayTest;
+@JsName()

[sra1, 2015/09/17 19:52:57]

It looks odd to me when there is no name in @JsName()

The annotation expresses different things - that something is implemented in
JavaScript and certain attributes of that implementation.
Perhaps there should be an annotation for each thing.

@JsLibrary()   
@JsName('JSON')  // optional
library ...

maybe a short annotation with named optional arguments would be more expressive
and also extensible. (e.g. Maybe we will want to add a way inform the compiler
how to call, say Math.max, with multiple arguments.)

@Js()

@Js(name: 'JSON')

@Js(access: "JSON.stringify")
external String stringify(obj);

[Siggi Cherem (dart-lang), 2015/09/18 20:34:10]

I had a similar reaction - @JsName() seems like it's missing something.

One question: what other annotations do we expect to have in addition to this
one? Are we planning to have code-gen of stubs like Justin and Alexander did a
while back, or does that become completely unnecessary in the future?

That might have an influence on the names we pick, to make sure we clearly
distinguish between those and the interop annotations here.

I like @Js() and @Js('name.in.a.string'), but I'd also be OK with different
annotations for different kind of elements:

@JsLibrary('JSON')
library ...

@JsClass('Foo')
class A {}

@JsFunction('foo.bar')
external _fooBar();

If `Js` seems too generic, other ideas are: @JsApi(), @FromJs()

[Jacob, 2015/10/01 00:47:33]

switched to @Js

there will probably be other annotations but not many. 

Modifying the behavior of a JS class is now an explicit non-goal as it seems
hard to implement efficiently without wrappers in Dart2Js and the goal is to
expose existing JS libraries not to beautify them.

[alexandre.ardhuin, 2015/10/01 08:51:05]

:( That means the dart libs that proxified Js lib will not be able to follow the
Dart style guide without adding dart wrappers.

For instance in
https://developers.google.com/maps/documentation/javascript/3.exp/reference#Map
you can see a lot of `getXxx()` that would be more darty with `get xxx;`
(https://www.dartlang.org/articles/style-guide/#do-use-a-getter-for-operations...).

[Jacob, 2015/10/02 20:08:15]

There are some significant tradeoffs at play that lead to this design. Mapping
get xxx to getXxx for some JS classes comes with a real performance and/or code
size hit. In addition, this is a feature needed largely for older JS libraries
designed before ES5 was widely available. I think it is better to provide fast
and automate-able importing of JS libraries to Dart rather than trying to make
JS libraries look like they were written by Dart users.

Even for libraries with "ugly" getXxx members some end users will prefer keeping
the methods as getXxx as that makes it much easier to reuse JS samples. For
dart:html we found that ugly is often better than pretty and harder to search
for/less familiar.


The Maps API looks that way likely because they needed to support ES3.  If you
want to make a version of the API designed with ES5+ in mind, those methods
would likely be getters.
If you want the Maps API to have a more modern shape, use the same technique you
would writing regular JavaScript and monkey patch the JavaScript properties you
want to add. 

If a particular JS library really needs to be more darty to be productively used
from Dart, monkey patch it on the JS side to be more darty the same way you
would modernize the JS library for JS users.
Note: in the future there may be an option to do this monkey patching in a
typesafe way using ES6 symbols in the way the Dart Dev Compiler currently makes
JS Array support the Dart List API.  At some point we may expose a similar
mechanism in Dart2Js to enable the case you describe. In any case, there is
still the real cost of adding these unlikely to be treeshaken methods. The
material difference in system behavior is really just whether the new properties
are visible from JS or hidden from JS users.

[alexandre.ardhuin, 2015/10/02 20:29:56]

Thanks for this explanation. It makes sense.
I'll take a look at monkey patching (with symbols) and ES6 proxies (once the
implementation in the main browsers has improved).

+library js_typed_interop_test;
 
 import 'dart:html';
 import 'dart:js';
@@ -14,12 +15,31 @@ _injectJs() {
   document.body.append(new ScriptElement()
     ..type = 'text/javascript'
     ..innerHtml = r"""
+  var Foo = {
+    multiplyDefault2: function(a, b) {
+      if (arguments.length >= 2) return a *b;
+      return a * 2;
+    }
+  };
+
   var foo = {
     x: 3,
     z: 40, // Not specified in typed Dart API so should fail in checked mode.
     multiplyByX: function(arg) { return arg * this.x; },
     // This function can be torn off without having to bind this.
-    multiplyBy2: function(arg) { return arg * 2; }
+    multiplyBy2: function(arg) { return arg * 2; },
+    callClosureWithArg1: function(closure, arg) {
+      return closure(arg);
+    },
+    callClosureWithArg2: function(closure, arg1, arg2) {
+      return closure(arg1, arg2);
+    },
+    callClosureWithArgAndThis: function(closure, arg) {
+      return closure.apply(this, [arg]);
+    },
+    getBar() {
+      return bar;
+    }
   };
 
   var foob = {
@@ -30,7 +50,19 @@ _injectJs() {
 
   var bar = {
     x: "foo",
-    multiplyByX: true
+    multiplyByX: true,
+    getFoo() {
+      return foo;
+    }
+  };
+
+  function ClassWithConstructor(a, b) {
+    this.a = a;
+    this.b = b;
+  };
+
+  ClassWithConstructor.prototype = {
+    getA: function() { return this.a;}
   };
 
   var selection = ["a", "b", "c", foo, bar];  
@@ -38,36 +70,86 @@ _injectJs() {
 """);
 }
 
-abstract class Foo {
-  int get x;
-  set x(int v);
-  num multiplyByX(num y);
-  num multiplyBy2(num y);
+// TODO(jacobr): depend on package:js instead of defining this annotation here.
+class JsName {
+  const JsName([this.name]);
+  final String name;
+}
+
+class RegularClass {
+  factory RegularClass(a) {
+    return new RegularClass.fooConstructor(a);
+  }
+  RegularClass.fooConstructor(this.a);
+  var a;
 }
 
-abstract class Foob extends Foo {
-  final String y;
+@JsName()
+class ClassWithConstructor {
+  external ClassWithConstructor(aParam, bParam);
+  external getA();
+  external get a;
+  external get b;
 }
 
-abstract class Bar {
-  String get x;
-  bool get multiplyByX;
+@JsName()
+class Foo extends JavaScriptObject {
+  external int get x;
+  external set x(int v);
+  external num multiplyByX(num y);
+  external num multiplyBy2(num y);
+  external callClosureWithArgAndThis(Function closure, arg);
+  external callClosureWithArg1(Function closure, arg1);
+  external callClosureWithArg2(Function closure, arg1, arg2);
+  external Bar getBar();
+  external static int multiplyDefault2(int a, [b]);
+}
+
+@JsName()
+class ExampleLiteral
+{
+  external factory ExampleLiteral({int x, String y, num z});
+
+  external int get x;
+  external String get y;
+  external num get z;
+}
+
+@JsName('Foob')
+class Foob extends Foo {
+  external String get y;
+}
+
+@JsName('Bar')
+class Bar extends JavaScriptObject {
+  external String get x;
+  external bool get multiplyByX;
+  external Foo getFoo();
 }
 
 class Baz {}
 
-// This class shows the pattern used by APIs such as jQuery that add methods
-// to Arrays.
-abstract class Selection implements List {
-  num doubleLength();
+// This class shows the pattern used by APIs like jQuery that add methods
+// to array like objects.
+abstract class Selection implements JsArray {
+  external num doubleLength();
 }
 
-Foo get foo => context['foo'];
-Foob get foob => context['foob'];
-Bar get bar => context['bar'];
-Selection get selection => context['selection'];
+// No @JsName is required for these external methods as the library is
+// annotated with JsName.
+external Foo get foo;
+external Foob get foob;
+external Bar get bar;
+external Selection get selection;
+
+addWithDefault(a, [b = 100]) => a + b;
+
+@JsName("JSON.stringify")
+external String stringify(obj);
 
 main() {
+  var x = new RegularClass(42);
+  print(x.toString());
   // Call experimental API to register Dart interfaces implemented by
   // JavaScript classes.
   registerJsInterfaces([Foo, Foob, Bar, Selection]);
@@ -76,6 +158,30 @@ main() {
 
   useHtmlConfiguration();
 
+  group('object literal', () {
+    test('simple', () {
+      var l = new ExampleLiteral(x: 3, y: "foo");
+      expect(l.x, equals(3));
+      expect(l.y, equals("foo"));
+      expect(l.z, isNull);
+      expect(stringify(l), equals('{"x":3,"y":"foo"}'));
+      l = new ExampleLiteral(z: 100);
+      expect(l.x, isNull);
+      expect(l.y, isNull);
+      expect(l.z, equals(100));
+      expect(stringify(l), equals('{"z":100}'));
+    });
+  });
+
+  group('constructor', () {
+    test('simple', () {
+      var o = new ClassWithConstructor("foo", "bar");
+      expect(o.a, equals("foo"));
+      expect(o.b, equals("bar"));
+      expect(o.getA(), equals("foo"));
+    });
+  });
+
   group('property', () {
     test('get', () {
       expect(foo.x, equals(3));
@@ -83,15 +189,15 @@ main() {
       expect(foob.y, equals("why"));
 
       // Exists in JS but not in API.
-      expect(() => foo.z, throws);
+      expect(() => (foo as dynamic).zSomeInvalidName, throws);
       expect(bar.multiplyByX, isTrue);
     });
     test('set', () {
       foo.x = 42;
       expect(foo.x, equals(42));
       // Property tagged as read only in typed API.
-      expect(() => foob.y = "bla", throws);
-      expect(() => foo.unknownName = 42, throws);
+      expect(() => (foob as dynamic).y = "bla", throws);
+      expect(() => (foo as dynamic).unknownName = 42, throws);
     });
   });
 
@@ -105,22 +211,74 @@ main() {
 
     test('tearoff', () {
       foo.x = 10;
-      // TODO(jacobr): should we automatically bind "this" for tearoffs of JS
-      // objects?
-      JsFunction multiplyBy2 = foo.multiplyBy2;
+      Function multiplyBy2 = foo.multiplyBy2;
       expect(multiplyBy2(5), equals(10));
+      Function multiplyByX = foo.multiplyByX;
+      // Tearing off a JS closure doesn't bind this.
+      // You will need to use the new method tearoff syntax to bind this.
+      expect(multiplyByX(4), isNaN);
+    });
+  });
+
+  group('static method', () {
+    test('call from dart', () {
+/*      expect(Foo.multiplyDefault2(6, 7), equals(42));
+      expect(Foo.multiplyDefault2(6), equals(12));*/
+      Function tearOffMethod = Foo.multiplyDefault2;
+      expect(tearOffMethod(6, 6), equals(36));
+      expect(tearOffMethod(6), equals(12));
+    });
+  });
+
+  group('closure', () {
+    test('call from js', () {
+      localClosure(x) => x * 10;
+      var wrappedLocalClosure = allowInterop(localClosure);
+      expect(
+          identical(allowInterop(localClosure), wrappedLocalClosure), isTrue);
+      expect(foo.callClosureWithArg1(wrappedLocalClosure, 10), equals(100));
+      expect(foo.callClosureWithArg1(wrappedLocalClosure, "a"),
+          equals("aaaaaaaaaa"));
+      expect(foo.callClosureWithArg1(allowInterop(addWithDefault), 10),
+          equals(110));
+      expect(foo.callClosureWithArg2(allowInterop(addWithDefault), 10, 20),
+          equals(30));
+      addThisXAndArg(Foo that, int arg) {
+        return foo.x + arg;
+      }
+      var wrappedCaptureThisClosure = allowInteropCaptureThis(addThisXAndArg);
+      foo.x = 20;
+      expect(foo.callClosureWithArgAndThis(wrappedCaptureThisClosure, 10),
+          equals(30));
+      foo.x = 50;
+      expect(foo.callClosureWithArgAndThis(wrappedCaptureThisClosure, 10),
+          equals(60));
+      expect(
+          identical(allowInteropCaptureThis(addThisXAndArg),
+              wrappedCaptureThisClosure),
+          isTrue);
+    });
+  });
+
+  group('chain calls', () {
+    test("method calls", () {
+      // In dart2js make sure we still use interceptors when making nested
+      // calls to objects.
+      var bar = foo.getBar().getFoo().getBar().getFoo().getBar();
+      expect(bar.x, equals("foo"));
     });
   });
 
   group('type check', () {
     test('js interfaces', () {
-      expect(foo is JsObject, isTrue);
-      // Cross-casts are allowed.
+      // Is checks return true for all  JavaScript interfaces.
       expect(foo is Bar, isTrue);
-      expect(selection is JsArray, isTrue);
+      expect(foo is Foob, isTrue);
+
+      expect(selection is List, isTrue);
 
       // We do know at runtime whether something is a JsArray or not.
-      expect(foo is JsArray, isFalse);
+      expect(foo is List, isFalse);
     });
 
     test('dart interfaces', () {
@@ -128,12 +286,4 @@ main() {
       expect(selection is List, isTrue);
     });
   });
-
-  group("registration", () {
-    test('repeated fails', () {
-      // The experimental registerJsInterfaces API has already been called so
-      // it cannot be called a second time.
-      expect(() => registerJsInterfaces([Baz]), throws);
-    });
-  });
 }