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

feat: use tsc to generate typings #679

Closed
wants to merge 1 commit into from
Closed

Conversation

daKmoR
Copy link
Member

@daKmoR daKmoR commented Aug 1, 2019

alternative to #678

using a pre-alpha version of tsc

  • typings look better 👍
  • however jsDoc gets lost 😢
  • exports are invalid (missing from '../path/to/file.js')

@benjamind
Copy link
Contributor

Very promising though, despite the exports being broken. We're so close!

@justinfagnani
Copy link

justinfagnani commented Aug 2, 2019

At the risk of derailing a PR conversation, can I ask why the open-wc source just doesn't use TypeScript directly?

@daKmoR daKmoR force-pushed the feat/autoGenerateTypesTsc branch from ba9cbc8 to 93b44ff Compare August 2, 2019 09:32
@daKmoR
Copy link
Member Author

daKmoR commented Aug 2, 2019

woop woop new version at microsoft/TypeScript#32372 (comment) and it seems to work perfectly now 🎉

@daKmoR
Copy link
Member Author

daKmoR commented Aug 2, 2019

@benjamind could you verify if the types are now fully helpful? 🤗

@benjamind
Copy link
Contributor

I can verify that I pulled in the @openwc/testing-helpers package from this branch and it compiled against my typescript project without issue! Types appear to be strongly applied, and I managed to remove all of my hand crafted .d.ts files for that package.

I then added @openwc/semantic-dom-diff to the compilation and got some usable-ish files. Since its used as a chai plugin i think we need some additional types there to correctly expose its functionality on the Chai Assertion namespace, but I think thats just a thing thats lacking from the jsdoc and not an issue with this tsc generation.

Overall I think this is an excellent approach (second only to native Typescript which would be my preference), but this gives us very usable output for Typescript and I think will mean we can after some more testing remove the allowJs approach from our recommended typing configuration which is a great win for Typescript users.

@LarsDenBakker
Copy link
Member

LarsDenBakker commented Aug 4, 2019

@justinfagnani we're not doing that because we want to maintain a low barrier for people to contribute to the project.

@justinfagnani
Copy link

@LarsDenBakker I understand that concern, but from my experience as a maintainer of a lot of TypeScript-based projects I've found that TS has possibly more often than not made it easier for contributors because of the excellent tooling that helps newcomers navigate the codebase and catch errors early. We've seen comments like "that was easier/more helpful than I expected". I'm personally hesitant to contribute to non-TS projects these days :)

@daKmoR
Copy link
Member Author

daKmoR commented Aug 15, 2019

closed in favor of #697

@daKmoR daKmoR closed this Aug 15, 2019
@daKmoR daKmoR deleted the feat/autoGenerateTypesTsc branch November 27, 2019 21:20
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.

4 participants