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 TypeScript type declaration file #255

Open
saschanaz opened this issue Jan 4, 2019 · 16 comments
Open

Add TypeScript type declaration file #255

saschanaz opened this issue Jan 4, 2019 · 16 comments

Comments

@saschanaz
Copy link
Member

Adding index.d.ts in the root directory will enable other packages to automatically read it to properly typecheck any use of webidl2.js.

Previously I had to contribute to DefinitelyTyped like DefinitelyTyped/DefinitelyTyped#25092 but it's harder to maintain as it has a separate versioning system.

@marcoscaceres What do you think, as you previously stated a concern about adding TS support? #55 (comment)

@marcoscaceres
Copy link
Member

marcoscaceres commented Jan 4, 2019

I'm very comfortable with our TS support because we use the JSDoc style. What I'm uncomfortable with is writing pure TypeScript code and transpiling it. So, I'd be totally fine with as index.d.ts file so long as code in the project remains ES.

@saschanaz
Copy link
Member Author

saschanaz commented Mar 16, 2019

Waiting for microsoft/TypeScript#7546...

@ExE-Boss
Copy link
Contributor

microsoft/TypeScript#7546 has been resolved and is supported as of TypeScript 3.7.

@saschanaz
Copy link
Member Author

Hey @ExE-Boss, you have been quite active for editing @types/webidl2. Are you interested in generating them right from the sources? I haven't been too active in this area so your help would be very precious.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Sep 3, 2020

Well, I tried it already, but I ran into microsoft/TypeScript#4433.

@saschanaz
Copy link
Member Author

saschanaz commented Sep 3, 2020

Oh, TIL! Generating separate module definitions doesn't sound like a blocker for me though. My immediate blocker is that I have no way to generate .d.ts without also generating .js files. (Well I can just remove them after build but there should be an option not to generate them 🤔)

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Sep 3, 2020

emitDeclarationOnly does exactly that.

@saschanaz
Copy link
Member Author

saschanaz commented Sep 3, 2020

Thanks, and this is now interesting.

> npx tsc --declaration --allowJs --outDir types --emitDeclarationOnly                 
C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:82931
                throw e;
                ^

Error: Debug Failure. Unhandled class member kind! 33587200
    at serializePropertySymbol (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33558:41)
    at serializePropertySymbolForInterface (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33562:28)
    at C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33217:104
    at Object.flatMap (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:388:25)
    at serializeInterface (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33217:38)
    at serializeSymbolWorker (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33146:25)
    at serializeSymbol (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33096:38)
    at C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33077:25
    at Map.forEach (<anonymous>)
    at visitSymbolTable (C:\Users\Kagami\Documents\GitHub\webidl2.js\node_modules\typescript\lib\tsc.js:33076:33)

Maybe worth filing a bug... (Edit: microsoft/TypeScript#40385)

@weswigham
Copy link

If you wanna workaround the issue for now (I have a PR out with a fix) - the interface merge(s) in supplement.d.ts are what's causing the declaration emitter to crash ❤️

@saschanaz
Copy link
Member Author

saschanaz commented Sep 9, 2020

Thanks @weswigham! Should I remove the supplement for now and instead define them in JSDoc? What would be the alternatives?

Will the merging still be supported?

@weswigham
Copy link

Yeah, it'll work once the patch I have out is in.

@marcoscaceres
Copy link
Member

Earlier, I made a comment about our use of TS in this project. Over the last week, I've finally had a chance to really deep dive into TS. I now have a much better understanding of what it does and what it's actually for. Going forwards, I just wanted to go on the record that I'd feel quite comfortable with us adopting TS fully if we ever want to refactor this project (though I'd probably stay away from a few of the experimental things and just stick to the static analysis feature set of TS).

@saschanaz
Copy link
Member Author

We still need more @ts-check on our codes so adding it for all files would be the first step for better TS use.

@saschanaz
Copy link
Member Author

Somehow still doesn't work as expected, filed microsoft/TypeScript#47358

@cyraid
Copy link

cyraid commented Sep 9, 2024

@saschanaz Did you get the reply on that bug report for the workaround? Is it possible to use here? tsc --noEmit false --declaration [...]

@saschanaz
Copy link
Member Author

I think that works but the result should be refined to match the current DT types. Probably we can copypaste some of it.

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

No branches or pull requests

5 participants