js-interop: dart2js is unable to utilize dashes in JS annotations

[kealjones-wk]

Describe the issue you're seeing When utilizing dashes in the @JSannotation it works as expected in DDC but will fail in dart2js. It fails because it adds spaces around the - and then attempts to use the dot notation when it should use the bracket notation.

Does it happen in Dartium or when compiled to JavaScript? When compiled using dart2js

• Dart SDK version: 2.2.0
• pkg/js version: "0.6.1+1"

Failing code: Example when trying to access window['material-ui']:

expected behavior: if there is a dash in the annotation it should become something like self['what-ever'].something

workaround We worked around this by assigning the window object with the dash to a non-dashed window object like so:

window.MaterialUI = window['material-ui'];

[rakudrama]

It is not really specified if the string represents a JavaScript expression or something else. The examples, like @JS('JSON.stringify') hint that is should be an expression.

DDC splits on dots and treats the parts as names of properties. dart2js generates the string "self.material-ui.DialogTitle" and then parses it as a JavaScript expression (the dash become subtraction). This is clearly wrong but would work for something like @JS('resource[13]).

@jmesserly - what should we do? The use case here seems reasonable, but we might want more that a property access path.

[jmesserly]

DDC splits on dots and treats the parts as names of properties.

yeah, I believe @jacob314 designed it so top-level names can use dots. This was mainly (only?) intended for the @JS on the library. For example @JS('goog.editor').

This is indeed a problem and my proposal doesn't specify. ES6 has things like symbolized method names[Symbol.iterator]() { ... } too, and we need a way to call those.

One challenge is figuring out what the JS parsing context would be. For example, does a member name @JS('Symbol.iterator') refer to receiver[Symbol.iterator] or does it refer to receiver.Symbol.iterator. Folks have asked for the latter. But that's not really an expression; it's being reinterpreted as a sequence of property gets applied to the receiver.

Brainstorming here:

@JS()
class MembersWithBrackets {
  @JS('["foo-bar"]')
  external get fooBar;

  @JS('[Symbol.iterator]')
  external get jsIterator;
}

@JS('["material-ui"]')
class MaterialUI {
  // ...
}

@JS('path.to["material-ui"]')
class MaterialUIWithPath {
  // ...
}

Pros:

• it's like ES6
• it supports property paths

Cons:

• requires us to parse some subset of JS (and spec the subset)
• feels clunky?

Interested in your thoughts on this.

[jacob314]

I don't think users would get in trouble using this extended syntax so it seems fine to include in the slightly clunky form. There isn't really any harm in the clunkiness as it will be rarely used but when it is used it would be really useful.

Potentially if you get extension methods that can use package:js/js_util.dart or a similar API that lets you get arbitrary properties you might not need to extend the @JS syntax at all as the custom syntax would only be needed for really rare cases.

[jmesserly]

Potentially if you get extension methods that can use package:js/js_util.dart or a similar API that lets you get arbitrary properties you might not need to extend the @js syntax at all as the custom syntax would only be needed for really rare cases.

That's a good point, we don't need this for members, because a static helper can work around it:

@JS()
@staticExtensions
class MembersWithBrackets {
  get fooBar => js_util.getProperty(this, "foo-bar")
  get jsIterator => js_util.getProperty(this, JSSymbol.iterator);
}

@JS('Symbol')
class JSSymbol {
  external static get iterator;
}

For classes it's tricky. We could change the methods from static to instance methods:

@JS()
@anonymous
class MaterialUI {
  external ReactClass get button;
  // ...
}

// Note: js_util.global doesn't exist, but it probably should.
// (Otherwise dart:js is the only easy way to get the JS global object.)
MaterialUI get materialUI => js_util.getProperty(js_util.global, "material-ui");

That one seems suboptimal IMO, but at least it can be expressed.