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

[dart2wasm] Issue with dynamic data type is not very intuitive as expected #56019

Closed konsultaner closed 4 months ago

konsultaner commented 4 months ago

I've been migrating my package connectanum-dart to support the dart2wasm compiler. It works quite well after fixing some issues with other packages and on issue on dart itself. I found out that the last remaining issues where all caused by unsing the dynamic datatype.

Whenever I use a dynamic checking it for a certain type lets say a custom class Bean. My tests failed. So

var x = returnsADynamic();
return x.toJS; // This will fail

needs to be changed to

var x = returnsADynamic();
return (x as Bean).toJS; // This will not fail

One major issue was using the Completer class where I had multiple classes being returned as an error like this:

try {
   var x = await call(); // returning a completer.future
} on ErrorClass1 catch (e1) {
...
} on ErrorClass2 catch (e2) {

}

I had to rewrite it to use the then() without await to get it to run under wasm.

dart-github-bot commented 4 months ago

Labels: area-dart2wasm, type-bug Summary: The dart2wasm compiler exhibits unexpected behavior when working with dynamic data types. Specifically, type checks on dynamic variables fail, requiring explicit type casts, and Completer usage within try...catch blocks necessitates rewriting to use then() without await.

osa1 commented 4 months ago

Re: the first issue, you can't use toJS on dynamic because toJS is an extension method. Extension methods are basically static methods with syntactic sugar to make them look like normal methods. See https://dart.dev/language/extension-methods#static-types-and-dynamic.

Does toJS on dynamic work in dart2js? It shouldn't work.

Re: the await issue, I'm not sure what the issue is. Could you show the full code?

konsultaner commented 4 months ago

Ok, that makes finding issues pretty tough. Why doesn't dynamic not just throw a compiletime error or the IDE should at least warn the developer to not use extension methods on dynamic types. It took me several hours to understand the issue and it is far away from being obvious.


Future<void> getCompleter() async {
    var c = Completer
    // some time Passes by
    c.completeExceptionally(ErrorBean1);
}

try {
    await getCompleter();
} on ErrorBean1 catch (e) {
   // handle it, but you wont
}
mraleph commented 4 months ago

@konsultaner There are lint rules which can help you with that: e.g. https://dart.dev/tools/linter-rules/avoid_dynamic_calls. See also https://dart.dev/tools/analysis#enabling-additional-type-checks

konsultaner commented 4 months ago

Now it just clicked. Dynamic can't produce a compile time exception 🙈

konsultaner commented 4 months ago

This commit

shows the exact same issue with the try catch and how I fixed it.

osa1 commented 4 months ago

This commit shows the exact same issue with the try catch and how I fixed it.

That looks like a bug, but I'm unable to reproduce it. I tried this:

Future<void> main() async {
  await(test());
}

class A {}

class B {}

bool runtimeFalse() => int.parse('1') == 0;

bool runtimeTrue() => int.parse('1') == 1;

Future<void> test() async {
  try {
    final test = await f();
    print('After f()');
  } on A catch (a) {
    print("Caught A");
  } on B catch (b) {
    print("Caught B");
  }
}

Future<int> f() async {
  if (runtimeFalse()) {
    throw A();
  } else if (runtimeTrue()) {
    throw B();
  }
  return 123;
}

This works as expected.

Which version of Dart are you using? Is there a test command I can run to reproduce the error in the old version of the code?

konsultaner commented 4 months ago

@osa1 seems like it has already been fixed with the latest version of dart. I updated it yesterday. Seems to work now. rolling back to the old version of my code just works fine! Thx!