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.09k stars 1.56k forks source link

JS compilers ignore default values of optional parameters of external factories #33395

Open lexaknyazev opened 6 years ago

lexaknyazev commented 6 years ago

Dart SDK 2.0.0-dev.60.0

@JS()
library js_bug;

import 'package:js/js.dart';

@JS()
@anonymous
class MyClass {
  external factory MyClass({String foo = 'fooValue', String bar});
}

@JS('console.log')
external void log(v);

void main() {
  log(new MyClass(bar: 'barValue'));
}

Compiled with DDC (fragment):

  main.main = function() {
    dart.global.console.log({bar: "barValue"});  // <---- foo is missing
  };

Compiled with Dart2JS (fragment):

d:function(){var z={bar:"barValue"} // <---- foo is missing
self.console.log(z)},
matanlurey commented 6 years ago

I think this blanket is not supported, since @JS() is supposed to be a direct mapping to JS-code, which obviously won't have Dart-specific default values (in fact they might have there own based on undefined, which this would break).

We should probably cover this in stuff that @jmesserly wants to specify for JS interop if so.

lexaknyazev commented 6 years ago

in fact they might have there own based on undefined, which this would break

Fair point. But defining non-external factories should be safe, right? This example shows the same result as the one above.

@JS()
@anonymous
class MyClass {
  external factory MyClass({String foo, String bar});

  factory MyClass.defaultFoo({String foo = 'fooValue', String bar}) =>
      new MyClass(foo: foo, bar: bar);
}

@JS('console.log')
external void log(v);

void main() {
  log(new MyClass.defaultFoo(bar: 'barValue'));
}
matanlurey commented 6 years ago

I don't think JS interop officially supports Dart-only code on JS members (it might work).

leonsenft commented 2 years ago

Could the compilers fail on default arguments in this context since they're not going to work as one would expected?

It looks like this is proposed in https://github.com/dart-lang/sdk/issues/41375.