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

Add TS typings #78

Open
Pandapip1 opened this issue Jun 21, 2023 · 5 comments
Open

Add TS typings #78

Pandapip1 opened this issue Jun 21, 2023 · 5 comments
Assignees
Labels
ready to implement Feature has reached rough consensus among editors and is ready to be implemented

Comments

@Pandapip1
Copy link
Member

Pretty simple. Add .d.ts files for eipw-lint-js.

@SamWilsn SamWilsn added the ready to implement Feature has reached rough consensus among editors and is ready to be implemented label Jun 28, 2023
@Ricy137
Copy link

Ricy137 commented May 20, 2024

I'm interested in the issue, can I be assigned @SamWilsn ?

Also, please correct me if I got it wrong, the issue is due to the .d.ts file generated by wasm-pack isn't specific enough( filled with any type), so I need to update the automatically generated .d.ts file to more specifc types.

My question is currently the js package is generated automatically with wasm-pack and the js package as well as the targeted .d.ts file isn't in the repo currently (the repo has codes to generate the js package but not the final js package codes). So where shall I update the .d.ts file to? Thanks 🙏 .

@SamWilsn
Copy link
Collaborator

I'm interested in the issue, can I be assigned @SamWilsn ?

Of course!

Also, please correct me if I got it wrong, the issue is due to the .d.ts file generated by wasm-pack isn't specific enough( filled with any type)

Correct!

My question is currently the js package is generated automatically with wasm-pack and the js package as well as the targeted .d.ts file isn't in the repo currently (the repo has codes to generate the js package but not the final js package codes). So where shall I update the .d.ts file to?

It's a bit hairy (or at least it was the last time I looked.) I've done something similar over here, but it's a very manual process. The wasm-bindgen reference has a section on it.

This Stack Overflow answer has some options for doing it automatically, but I haven't investigated them much. ts-rs in particular seems weird in that it generates the bindings during cargo test. On the other hand, tsify doesn't seem as active of a project, but it works with wasm-bindgen.

@Ricy137
Copy link

Ricy137 commented May 26, 2024

It's a bit hairy (or at least it was the last time I looked.) I've done something similar over here, but it's a very manual process. The wasm-bindgen reference has a section on it.

This Stack Overflow answer has some options for doing it automatically, but I haven't investigated them much. ts-rs in particular seems weird in that it generates the bindings during cargo test. On the other hand, tsify doesn't seem as active of a project, but it works with wasm-bindgen.

Thanks for your guides, they're very helpful! I've actually successfully implemented tsify and generated the intended .d.ts file. But this would involves modifications on eipw-lint, so I think manually adding the types generated by tsify would be a better approach. I've forked the repo and you can see my latest process here.

However, I failed to bind the types customarily defined with the lint and format function generated by wasm-bindgen. I got that I can use codes like here to use these customarily defined types in rust codes, but to combine these types in the lint and format functions require further modification for the related rust codes... This is where I'm blocked ( not knowing much about rust and doubt my inexperienced modifications would introduce unexpected negative effects).

Can you provide some help to break the blocks I met? And you can find all my codes here ( the master branch is where I want to manually add types, tsify/ts-rs branch is where I implemented tsify/ts-rs ). Thanks!

@SamWilsn
Copy link
Collaborator

Oh wow! You've gone way further down this road than I ever have.

But this would involves modifications on eipw-lint

I'm not opposed to modifications. How extensive are they? I tried to look through your diff, but I think you reformatted the code, so it's a bit hard to follow.

However, I failed to bind the types customarily defined with the lint and format function generated by wasm-bindgen.

Those might be easier to just do by hand. sources should be something like string[], options would be something like Opts | null, and the return value would be SnippetDef[] I think? wasm-bindgen takes care of the Option as far as I am aware.

@Ricy137
Copy link

Ricy137 commented May 30, 2024

and the return value would be SnippetDef[] I think?

This is the tricky part I met. I can't just code as Result<Vec<SnippetJS>, JsError> or I would met the trait IntoJsResult is not implemented for Result<Vec<SnippetJS>, wasm_bindgen::JsError> error. TBH, I've been tearing my hair out trying to find a workaround or bypass to fix the error.

Also after changing the type declaration of lint and format, there would be type mismatch issues in test. Since OptsJS and SnippetJS are generated by wasm_bindgen, they didn't implement Deserialize. So I can't use from_value e.t. to convert the type, instead, I did the conversion manually here, the convert_to_js_object function is a helper functionto retaining original plain JavaScript object structure, and the convert_to_js_object is to convert Value type to OptsJS type.

How do you think of current codes. ?

Thanks 🙏 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to implement Feature has reached rough consensus among editors and is ready to be implemented
Projects
None yet
Development

No branches or pull requests

3 participants