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

Allow web-sys to emit correct typescript declarations from webidl #1998

Conversation

bennetthardwick
Copy link
Contributor

This change adds the NamedAnyref adapter-type / descriptor, which behaves the same as an Anyref but also contains some information about the name. This allows web-sys to output the correct names for types instead of falling back to any.

If this feature is the kind of thing that the rustwasm team would like to merge - I'll make sure it's test covered. At the moment I just wanted to get it working and make sure it's something worth working on.

This is currently a breaking change but could be hidden behind a feature flag in web-sys.

@bennetthardwick bennetthardwick requested review from alexcrichton, Pauan and fitzgen and removed request for alexcrichton and Pauan February 13, 2020 23:02
@Pauan
Copy link
Contributor

Pauan commented Feb 14, 2020

I don't think we should be worrying too much about TypeScript breaking changes, since TypeScript's type system is so loose, and it's always possible for people to use as to arbitrarily convert from one type to another.

@bennetthardwick
Copy link
Contributor Author

That makes sense 👍 My only concern that was since the types were any before, there's a high chance that existing projects have been providing the wrong types, but it sounds like that's not too bad.

@alexcrichton
Copy link
Contributor

Thanks for this! Could this be sure to add some tests as well to exercise it and ensure it works?

@bennetthardwick
Copy link
Contributor Author

Hey @alexcrichton, I've added a few typescript tests. I couldn't find anywhere that explicitly tests stuff like ast::ImportKind through adapter2ts, so I figured just making some in the typescript-tests crate would be best.

Also since all of the webidl stuff is using this, the webidl-tests crate is making sure that it all works (at least in terms of wasm).

Please let me know if there's anywhere else I should write some tests 😄

@alexcrichton
Copy link
Contributor

Looks great to me, thanks!

@alexcrichton alexcrichton merged commit ec1b945 into rustwasm:master Feb 19, 2020
@bennetthardwick bennetthardwick deleted the feature/add-typescript-names-to-webidl branch February 19, 2020 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants