js_facade_gen icon indicating copy to clipboard operation
js_facade_gen copied to clipboard

Can't use google chart api facade due to missing constructor.

Open sureshg opened this issue 8 years ago • 4 comments

Generated the js-interop facade for Google visualization api and is missing the imp factory constructors for google chart types. All it has is the fakeConstructor$ instead one with HTMLElement . This makes the generated facade useless. As a workaround, I had to manually add the factory constructors for most of the chart types.

@JS("google.visualization.PieChart")
class PieChart extends CoreChartBase {
  // Manually added this factory constructor.
  external factory PieChart(Element element);
  // @Ignore
  PieChart.fakeConstructor$() : super.fakeConstructor$();

Sample App

I am not sure why we need the fakeConstructor$, is it just to satisfy the inheritance hierarchy?

Also, one of the other major pain point is missing type info from the generated facade.

  /*external void draw(DataTable data, PieChartOptions options);*/
  /*external void draw(DataView data, PieChartOptions options);*/
  external void draw(
      dynamic /*DataTable|DataView*/ data, PieChartOptions options);

Since dart doesn't support method overriding or union types, this dynamic conversion make it less type safe. Is there any plan to fix it?

sureshg avatar Dec 29 '16 08:12 sureshg

@jacob314 thoughts?

kevmoo avatar Jan 04 '17 17:01 kevmoo

Any update would be greatly appreciated. Thanks!

sureshg avatar Jan 10 '17 06:01 sureshg

Issue #1. Ignore the fakeConstructor$. It was there originally to avoid analyzer warnings and is not the reason why the actual PieChart constructor is missing. I have a CL in progress that fixes the actual issue which is that we aren't matching the JS semantics that constructors from base classes also exist on subclasses.

Issue #2. Answer depends on what happens with supporting those features generally in Dart code. If Dart supports unions and overloads we will just use that syntax. If it becomes clear Dart will not support them we will look to add custom syntax that only applies for JS interop facades likely using the comment syntax you see now.

jacob314 avatar Jan 10 '17 22:01 jacob314

jfyi, some of the other TS conversion tools are handling the issue 2 nicely (i mean preserving the type info) .

Eg: TS to Kotlin

external open class LineChart : CoreChartBase {
    open fun draw(data: DataTable, options: LineChartOptions): Unit = definedExternally
    open fun draw(data: DataView, options: LineChartOptions): Unit = definedExternally
}

I hope dart can also solve this issue elegentely 😉

sureshg avatar Mar 23 '17 19:03 sureshg