dart-archive / js_facade_gen

Generates package:js Javascript interop facades for arbitrary TypeScript libraries
Apache License 2.0
161 stars 29 forks source link

`maybeEmitFakeConstructor` considered harmful #82

Open ditman opened 4 years ago

ditman commented 4 years ago

Generated fakeConstructor$ do not work and can't be called in dart2js, hence they probably shouldn't be generated anymore.

This should be removed.

ditman commented 4 years ago

cc/ @natebosch who knows way more about this than I do.

ditman commented 4 years ago

This is a PR where the autogenerated code had to be removed: https://github.com/flutter/plugins/pull/2602

natebosch commented 4 years ago

I have never seen the errors this code claims to solve. @jacob314 - do you know the specifics? How can I repro?

My first preference would be to remove this altogether, I don't understand the problem it solves.

If we can't remove this altogether we can consider adding an external keyword.

jacob314 commented 4 years ago

they were never intended to be called. They were only around to placate the anlayzer for the hard to predict cases where you had a sublcass with a constructor but the base class didn't have the appropriate constructor. I forget the exact details.

natebosch commented 4 years ago

Let's drop them. I haven't seen a case where they are necessary. If someone complains we can revisit.

jacob314 commented 4 years ago

sgtm

ditman commented 4 years ago

I can get this done tomorrow. I think I'm one of the main users of the script for now :P

jacob314 commented 4 years ago

Happy to review any PR requests you send my way :)