dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.04k stars 1.55k forks source link

[ddc] Incorrect JavaScript generated for renamed extension members on JS interop classes in dart:html #49767

Open rileyporter opened 2 years ago

rileyporter commented 2 years ago

The JavaScript generated by DDC for renamed extension members on JS interop classes when the classes are inside dart:html is incorrect. Instead of an extension member access like html['JSClassExt|get#externalGetter'](htmlObj), the JavaScript generated is on dart.global like dart.global.OtherNameGetter(htmlObj). It seems like it's only an issue for external renamed extension members, and doesn't affect non-external extension members or external members that don't have a different @JS name. It also doesn't affect top level members.

As an example to repro, add this code to dart:html:

import 'dart:_js_annotations' as foo;

@foo.JS('OtherNameTopLevel')
external JSClass get renamedTopLevelExternalMember;

@foo.JS()
external JSClass get topLevelExternalMember;

@foo.JS()
@foo.staticInterop
class JSClass {}

extension JSClassExt on JSClass {
  @foo.JS('OtherNameField')
  external String renamedExternalField;

  external String externalField;

  @foo.JS('OtherNameSetter')
  external set renamedExternalSetter(String arg);

  external set externalSetter(String arg);

  set nonExternalSetter(String arg) {
    throw UnimplementedError();
  }

  @foo.JS('OtherNameGetter')
  external String get renamedExternalGetter;

  external String get externalGetter;

  String get nonExternalGetter => 'foo';

  @foo.JS('OtherNameMethod')
  external void renamedExternalMethod();

  external void externalMethod();

  void nonExternalMethod() => throw UnimplementedError();
}

And then provide the same classes outside of dart:html, e.g. in this main.dart:

import 'dart:html' as html;
import 'package:js/js.dart';

@JS('OtherNameTopLevel')
external NonHtmlJSClass get renamedTopLevelExternalMember;

@JS()
external NonHtmlJSClass get topLevelExternalMember;

@JS()
@staticInterop
class NonHtmlJSClass {}

extension NonHtmlJSClassExt on NonHtmlJSClass {
  @JS('OtherNameField')
  external String renamedExternalField;

  external String externalField;

  @JS('OtherNameSetter')
  external set renamedExternalSetter(String arg);

  external set externalSetter(String arg);

  set nonExternalSetter(String arg) {
    throw UnimplementedError();
  }

  @JS('OtherNameGetter')
  external String get renamedExternalGetter;

  external String get externalGetter;

  String get nonExternalGetter => 'foo';

  @JS('OtherNameMethod')
  external void renamedExternalMethod();

  external void externalMethod();

  void nonExternalMethod() => throw UnimplementedError();
}

void main() {
  var htmlObj = html.JSClass();
  htmlObj.renamedExternalSetter = 'a';
  htmlObj.externalSetter = 'b';
  htmlObj.nonExternalSetter = 'c';
  print(htmlObj.renamedExternalGetter);
  print(htmlObj.externalGetter);
  print(htmlObj.nonExternalGetter);
  htmlObj.renamedExternalMethod();
  htmlObj.externalMethod();
  htmlObj.nonExternalMethod();

  var obj = NonHtmlJSClass();
  obj.renamedExternalField = 'a';
  obj.externalField = 'b';
  obj.renamedExternalSetter = 'a';
  obj.externalSetter = 'b';
  obj.nonExternalSetter = 'c';
  print(obj.renamedExternalGetter);
  print(obj.externalGetter);
  print(obj.nonExternalGetter);
  obj.renamedExternalMethod();
  obj.externalMethod();
  obj.nonExternalMethod();
}

This generates the following main method in JS:

 main.main = function main$() {
    let htmlObj = _interceptors.JavaScriptObject.as(new dart.global.JSClass());
    dart.global.OtherNameSetter(htmlObj, "a");                 // incorrect
    html['JSClassExt|set#externalSetter'](htmlObj, "b");
    html['JSClassExt|set#nonExternalSetter'](htmlObj, "c");
    core.print(dart.global.OtherNameGetter(htmlObj));        // incorrect
    core.print(html['JSClassExt|get#externalGetter'](htmlObj));
    core.print(html['JSClassExt|get#nonExternalGetter'](htmlObj));
    dart.global.OtherNameMethod(htmlObj);                      // incorrect
    html['JSClassExt|externalMethod'](htmlObj);
    html['JSClassExt|nonExternalMethod'](htmlObj);
    html['JSClassExt|externalMethod'](dart.global.topLevelExternalMember);
    html['JSClassExt|externalMethod'](dart.global.OtherNameTopLevel);

    let obj = _interceptors.JavaScriptObject.as(new dart.global.NonHtmlJSClass());
    main['NonHtmlJSClassExt|set#renamedExternalField'](obj, "a");
    main['NonHtmlJSClassExt|set#externalField'](obj, "b");
    main['NonHtmlJSClassExt|set#renamedExternalSetter'](obj, "a");
    main['NonHtmlJSClassExt|set#externalSetter'](obj, "b");
    main['NonHtmlJSClassExt|set#nonExternalSetter'](obj, "c");
    core.print(main['NonHtmlJSClassExt|get#renamedExternalGetter'](obj));
    core.print(main['NonHtmlJSClassExt|get#externalGetter'](obj));
    core.print(main['NonHtmlJSClassExt|get#nonExternalGetter'](obj));
    main['NonHtmlJSClassExt|renamedExternalMethod'](obj);
    main['NonHtmlJSClassExt|externalMethod'](obj);
    main['NonHtmlJSClassExt|nonExternalMethod'](obj);
    main['NonHtmlJSClassExt|externalMethod'](dart.global.topLevelExternalMember);
    main['NonHtmlJSClassExt|externalMethod'](dart.global.OtherNameTopLevel);
  };

It could be that this is true for any extensions of JS interop classes in dart:* libraries, and might be due to this isJsMember method?

nshahan commented 1 year ago

Setting low priority unless this comes up as blocking for you.

rileyporter commented 1 year ago

Sounds good, I'll let you know if it becomes blocking. It's not blocking for now and I'm working on moving the new dart:html code outside of the sdk, so it may never become blocking.

KealJones commented 1 year ago

I am hitting this issue on a personal project. Its quite annoying. Everything works fine in dart2js but ddc is impossible to use.

nshahan commented 1 year ago

@KealJones This issue was is about unexpected compilation output under a very specific set of circumstances. In your personal project are you editing and compiling the dart:html library to add uses of JS interop and extension methods? If not could you file a separate issue with reproduction instructions?