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] JS staticInterop works poorly with variadic arguments, runtime failures #49785

Open matanlurey opened 2 years ago

matanlurey commented 2 years ago

Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_arm64"


I might have tried to be "too clever" for my own good, but here is what I tried and what happens:

import 'package:js/js.dart';

@JS()
@staticInterop
external JSWindow get window;

@JS()
@staticInterop
abstract class JSWindow {}

extension JSWindow$ on JSWindow {
  external JSConsole get console;
}

@JS('console')
@staticInterop
abstract class JSConsole {}

extension JSConsole$ on JSConsole {
  @JS('log')
  external void log1(Object? a);

  @JS('log')
  external void log2(Object? a, Object? b);

  @JS('log')
  external void log3(Object? a, Object? b, Object? c);

  @JS('log')
  external void log4(Object? a, Object? b, Object? c, Object? d);

  @JS('log')
  external void log5(Object? a, Object? b, Object? c, Object? d, Object? e);
}

Everything compiles (no errors), but at runtime I get:

Uncaught TypeError: dart.global.log is not a function

The generated DDC JS code looks like this:

dart.global.log(dom['JSWindow$|get#console'](dart.global.window), a);

I find this pretty bizarre personally. It works fine like this:

extension JSConsole$ on JSConsole {
  external void log(Object? a);
}

... but then I can only provide a single argument. I tried using optional arguments:

extension JSConsole$ on JSConsole {
  external void log(Object? a, [Object? b, Object? c]);
}

... but then I get null printed in the console for every unused argument.

sigmundch commented 2 years ago

@matanlurey - what version of the tools are you using to run these examples? We haven't been testing things out in webdev much because this is fairly an experimental feature on the side at the moment. But using a local build of ddc and dart2js the examples with logN work for me, also trying it in dartpad works.

We currently implement the extensions as a lowering in the compilers, so both compilers behave the same, but the error you see seems like an error I'd expect when the lowering was not enabled (which could be the case in an older version of the SDK). Unfortunately, I don't believe there is an SDK lower bound on package:js for this.

More generally, we have talked about being more flexible with JS functions and being less strict about arity than we are with Dart functions, for example, to support use cases like https://github.com/dart-lang/sdk/issues/48186.

matanlurey commented 2 years ago

@matanlurey - what version of the tools are you using to run these examples?

You mean the SDK?

Dart SDK version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_arm64"

If you're asking about webdev, 2.7.10.

We haven't been testing things out in webdev much because this is fairly an experimental feature on the side at the moment. But using a local build of ddc and dart2js the examples with logN work for me, also trying it in dartpad works.

I guess I'm confused why webdev would change the implementation. Is it possible you're on a newer/bleeding DDC?

We currently implement the extensions as a lowering in the compilers, so both compilers behave the same, but the error you see seems like an error I'd expect when the lowering was not enabled (which could be the case in an older version of the SDK). Unfortunately, I don't believe there is an SDK lower bound on package:js for this.

Sounds good. It's not blocking any work, this is a personal project, I was just surprised.

Happy to wait for the next stable release.

More generally, we have talked about being more flexible with JS functions and being less strict about arity than we are with Dart functions, for example, to support use cases like #48186.

Nice! I ran into this problem as well with window.requestAnimationFrame - I wanted to ignore the timestamp argument.

sigmundch commented 2 years ago

Thanks!

Technically webdev wouldn't affect things, you are correct, that said, in the past we have seen issues in some modular configurations that don't show up in others (e.g. the incremental front-end server shows issues with a transformer that makes the incremental compiler state invalid for hot restart) - so, as a result, there are issues that sometimes pop up only under certain environments.

I can't quite explain why you saw the runtime errors with dart.global.log. The SDK 2.17.6 should have the compiler transformations in place for @staticInterop so that is surprising. Do you see the same issues when compiling with dart2js?

matanlurey commented 2 years ago

I haven't tried, and probably won't if the DDC bits don't work yet. No worries :)