giniedp / webidl2ts

Converts Web IDL to Typescript (.d.ts)
MIT License
61 stars 16 forks source link

attributes missing, only provides getter and setter #3

Closed netpoetica closed 3 years ago

netpoetica commented 3 years ago

In this case, the WebIDL looks like

interface Vector2 {
    void Vector2(float x, float y);
    attribute float x;
    attribute float y;
};

and the TS type is like

 class Vector2 {
        constructor(x: number, y: number);
        get_x(): number;
        set_x(x: number): void;
        get_y(): number;
        set_y(y: number): void;
 }

but the actual object has more than just a getter and setter:

Vector2 {ptr: 5248120}
ptr: 5248120
x: 10
y: 20
__proto__: WrapperObject
constructor: ƒ Vector2(x, y)
get_x: ƒ ()
get_y: ƒ ()
set_x: ƒ (arg0)
set_y: ƒ (arg0)
__class__: ƒ Vector2(x, y)
__destroy__: ƒ ()
x: (...)
y: (...)
get x: ƒ ()
set x: ƒ (arg0)
get y: ƒ ()
set y: ƒ (arg0)
__proto__: WrapperObject

And the live object can definitely be used like vec.x = 10 or console.log(vec.x). Shouldn't the TS type contain x and y as number type, no?

netpoetica commented 3 years ago

I will make a PR for this, I am just curious if there is a reason why it you didn't do it - maybe it is undefined behavior, something like this @giniedp

giniedp commented 3 years ago

I am not sure any more why i left them out. My initial implementation was based on the binding description of ammo.js https://github.com/kripken/ammo.js/ where it states

Native JavaScript getters and setters could give a slightly nicer API here, however their performance is potentially problematic.

So maybe i just focused on the getters and setters.

Official documentation has no mention about performance. https://emscripten.org/docs/porting/connecting_cpp_and_javascript/WebIDL-Binder.html#attributes

Regardless of the potential performance penalty i agree with you, the properties should also be part of the generated type.

giniedp commented 3 years ago

should be fixed now. Thanks for the report.