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.21k stars 1.57k forks source link

[dart2js] JavaScriptObject tree-shaken too eagerly #51307

Open ditman opened 1 year ago

ditman commented 1 year ago

Consider this program:

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

@JS()
@staticInterop
class MyClass {}
extension MyClassExtension on MyClass {
  external int get value;
  external String get other;
}

void main() {
  final MyClass one = js_util.jsify(<String, Object>{
    'value': 1,
    'other': 'Woah',
  }) as MyClass;

  final MyClass two = js_util.jsify(<String, Object>{
    'value': 2,
    'other': 'Foo',
  }) as MyClass;

  // If the next line is commented out, the app crashes.
  print(one.value); // expected: 1
  print(one == one); // expected: true
  print(one == two); // expected: false
}

If the line print(one.value); is commented out, the app crashes with:

Uncaught Error: TypeError: Instance of 'Object': type 'Object' is not a subtype of type 'JavaScriptObject'

(I think the equality operator should be able to handle the case above.)

Environment

Tested this in the current version of DartPad, with the following versions:

flutter: 3.7.1
dart: 2.19.1
package:js 0.6.5

See dartpad gist

sigmundch commented 1 year ago

It appears this is a tree-shaking problem. When you comment out the line printing print(one.value) it is removing the only use of the static interop type. And as a result dart2js is not noticing that the entire application still needs to keep around the JavaScriptObject type and interceptor.

In particular, the error message you saw is happening earlier in the program, in the cast to as MyClass. I believe dart2js shouldn't be tree-shaking the base type here.

Another similar example, without ==:

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

@JS()
@staticInterop
class MyClass {}

extension MyClassExtension on MyClass {
  external int get value;
  external String get other;
}

@JS()
@staticInterop
@anonymous
class F {
  external factory F();
}

void main() {
  // F();
  final jsOne = js_util.jsify(<String, Object>{'value': 1});
  print(jsOne.runtimeType);

  final MyClass one = jsOne as MyClass;
  //print(one.value);
}

The call to .runtimeType crashes with the program above, however it would return JSObject when we don't tree-shake the interceptor for JavaScript values. Either uncommenting the call to create F() or the call like you had before to read .value would be enough for dart2js to keep it around.

ditman commented 1 year ago

Ah yes, good call! I assumed mine was failing in the equality rather than the cast, because DartPad wasn't giving me any line number or anything, I should have attempted to run this locally!

@sigmundch should this issue be renamed so it reflects more accurately what you explained? or is this WAI?

sigmundch commented 1 year ago

Went ahead and updated the title, not WAI :smile:, we should definitely fix this.

That said, this only triggers on very small programs that don't use the static interop class for anything other than Object methods, so this is less likely to cause issues in larger applications.

ditman commented 1 year ago

@sigmundch yes, I was helping an internal customer with some issue they were having and came across this issue.

I'm not sure if this is the same issue that the user was having (because they were just introducing staticInterop into their app), or something "similar", because I started a repro from scratch rather than attempting to minimize the actual code from the user.

(I'll try to repro the issue our customer was having more thoroughly in a separate issue)