Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typescript) Getters correctly define the inner results. #2909

Merged
merged 2 commits into from May 31, 2022

Commits on May 23, 2022

  1. fix(typescript) Getters correctly define the inner results.

    This patch fixes
    rustwasm#2721. Let's consider
    the following example:
    
    ```rust
    pub struct Foo {
        /// Hello `first`.
        pub first: u32,
    
        /// Hello `second`.
        #[wasm_bindgen(readonly)]
        pub second: u32,
    }
    ```
    
    It outputs the following `.d.ts` file:
    
    ```typescript
    export class Foo {
      free(): void;
    /**
    * Hello `first`.
    */
      first: number;
    /**
    * Hello `second`.
    */
      readonly second: void;
    }
    ```
    
    What's wrong here is that `second` has the type `void`. It should be
    `number`.
    
    What's happening? For `Foo.first`, a getter and a setter are
    generated. The getter never has a return type (spoiler: that's the
    bug), but the setter has a correct return type somehow (it's infererd
    from its arguments, combined with a unit descriptor, see
    `wasm_bindgen_cli_support::wit::Context::struct_`). When the getter
    and the setter are processed, they both end up calling
    `wasm_bindgen_cli_support::js::ExportedClass::push_accessor_ts`. This
    function overwrites the return type for the same field everytime it is
    called. So when called for the getter, the return type is missing, and
    when called for the setter with a type, the bug is fixed.
    
    For `Foo.second`, it's different. There is no setter generated because
    the field is marked as `readonly`. So the bug appears.
    
    To fix that, I've updated the getter `Function` descriptor, so that it
    has an `inner_ret` value. This is passed to an `Adapter.inner_results`
    at some point in the code, which is finally passed by
    `JsFunction::process` to `JsFunction::typescript_signature` to
    generate the typescript signature. And suddently, it's fixed!
    
    However, we got:
    
    ```typescript
    export class Foo {
      free(): void;
    /**
    * Hello `first`.
    * @returns {number}
    */
      first: number;
    /**
    * Hello `second`.
    * @returns {number}
    */
      readonly second: number;
    }
    ```
    
    We're making progress!
    
    Nonetheless, the documentation is wrong now: The fields don't return
    anything. This problem comes from the way `js_docs` is computed
    globally for all export kinds in
    `wasm_bindgen_cli_support::js::Context::generate_adapter`. This patch
    also updates that to compute `js_docs` lately, for each branch. It is
    specialized in the `AuxExportKind::Getter` and `AuxExportKind::Setter`
    branches.
    
    I hope this is the correct approach. I'm not sure about the difference
    between `Adapter.results` and `Adapter.inner_results` as there is no
    documentation unfortunately (kudos for every other super-well
    documented place though).
    Hywan committed May 23, 2022
    Copy the full SHA
    0dc54e0 View commit details
    Browse the repository at this point in the history

Commits on May 30, 2022

  1. Copy the full SHA
    f43c0b9 View commit details
    Browse the repository at this point in the history