Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange behavior of @JS () in Dart 2.0 #35185

Closed
DisDis opened this issue Nov 15, 2018 · 8 comments
Closed

Strange behavior of @JS () in Dart 2.0 #35185

DisDis opened this issue Nov 15, 2018 · 8 comments
Labels
closed-as-intended Closed as the reported issue is expected behavior

Comments

@DisDis
Copy link

DisDis commented Nov 15, 2018

Dart VM version: 2.0.0 (Unknown timestamp) on "linux_x64"

Create classes:

// https://github.com/DisDis/test_message_bus_js/blob/master/lib/test_message_js.dart
@JS()
@anonymous
class TestMessage1JS implements TestMessage1{
  num fieldInt1;
  String fieldStr1;
  SubObjTestMessage1 subObjTestMessage1;
}

@JS()
@anonymous
class SubObjTestMessage1JS implements SubObjTestMessage1 {
  List<String> fieldArr1Str;
  num fieldInt2;
}
// https://github.com/DisDis/test_message_bus_js/blob/master/lib/src/test_message_interface.dart
abstract class TestMessage1{
  num fieldInt1;
  String fieldStr1;
  SubObjTestMessage1 subObjTestMessage1;
}

abstract class SubObjTestMessage1{
  num fieldInt2;
  List<String> fieldArr1Str;
}

Run the main code:

@JS('JSON.parse')
external dynamic parse(String value);

main(){
 TestMessage1 tm2Js = parse('{"fieldInt1":3,"fieldStr1":"json text","subObjTestMessage1":{"fieldInt2":4,"fieldArr1Str":["1json","2json"]}}');
 _jsCallback(tm2Js);
}

// Cut

_jsCallback(dynamic msg){
  print('[Dart] fieldStr1: ${msg.fieldStr1}');
  print('[Dart] fieldInt1: ${msg.fieldInt1}');
  print('[Dart] "msg" is TestMessage1: ${msg is TestMessage1}');
  print('[Dart] "msg" is SubObjTestMessage1: ${msg is SubObjTestMessage1}');
}

Output:

[Dart] fieldStr1: json text
[Dart] fieldInt1: 3
[Dart] "msg" is TestMessage1: true
[Dart] "msg" is SubObjTestMessage1: true

Why does the code work correctly in Dart2Js? (in DDC you need to make a fix "(parse(...) as TestMessage1JS) as TestMessage1")

Why ("msg" is TestMessage1 == true) and ("msg" is SubObjTestMessage1 == true) in Dart2Js?
Can we not write 'getter/setter' and 'external' for all fields(fieldInt1, fieldStr1, etc) as in the example?

We would like to use this approach for parse responses from the server. Is this a bug or an undocumented @JS behavior?

Different examples of @JS work in Dart2.0 https://github.com/DisDis/test_message_bus_js

@DisDis
Copy link
Author

DisDis commented Nov 15, 2018

@matanlurey @jmesserly Maybe you know the answers?

@matanlurey
Copy link
Contributor

A couple things stand out:

  • All members on an @JS() class must be abstract and external.

The fact it works otherwise is a bug:

@JS()
@anonymous
abstract class TestMessage1JS implements TestMessage1{
  external factory TestMessage1JS({
    num fieldInt1,
    String fieldStr1,
  });
  external num get fieldInt1;
  external String get fieldStr1;
}
  • An @JS() interop method cannot return List<String>, only a List<dynamic>.

@DisDis
Copy link
Author

DisDis commented Nov 15, 2018

@matanlurey But the bug makes working with js data and server response very simple. It directly solves the whole experience with jsonSerialization/Deserialization.
Strange that ddc/dart2js does not find errors at compile time

@matanlurey
Copy link
Contributor

Whether it does or does not, it isn't supposed to work that way, hence why it does not always work.

@matanlurey matanlurey added the closed-as-intended Closed as the reported issue is expected behavior label Nov 15, 2018
@matanlurey
Copy link
Contributor

#35084 will likely make the above an error. I would track that.

@jmesserly
Copy link

All members on an @JS() class must be abstract and external.

fields are (supposed to be) an exception; they're treated as external in a @JS() class. I believe this was an issue in the Dart grammar, it didn't support external for fields (IMO it should).

Strange that ddc/dart2js does not find errors at compile time

Yeah that is tracked by: #32929 ... we need error checking for JS interop. Also is checks need to be consistent (currently they aren't consistent between dartdevc/dart2js)

Here is the tracking bug for improvements to JS interop: #35084 ... I'm working on it, it will be a major overhaul with lots of improvements, including error checking, well defined behavior for is checks.

@alexey-delikanov
Copy link

alexey-delikanov commented Nov 20, 2018

@jmesserly One more question on this issue. Is it ok to omit all members except factory in JS annotated class if I have the same members listed in interface I want to expose to client code?

@JS()
@anonymous
abstract class TestJS implements Test {
  external factory TestJS({
    num fieldInt1,
    Function func1
  });

  // Notice that all other members are omitted
}
abstract class Test {
  num fieldInt1;
  void func1(String str);

  // Notice that members are not marked as external
}

I have tested this code and it works correctly, but is it an expected behaviour?

Thank you.

@jmesserly
Copy link

Is it ok to omit all members except factory in JS annotated class if I have the same members listed in interface I want to expose to client code?

It's better to put them on an @JS() class, or at least mark Test as @JS(). In your example, the compiler doesn't know calls to Test are JS interop calls. It might happen to work by accident, but there's probably a lot things that don't work if you do it that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-as-intended Closed as the reported issue is expected behavior
Projects
None yet
Development

No branches or pull requests

4 participants