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

chore: generate ts declaration files #11

Merged
merged 2 commits into from
Sep 25, 2024
Merged

chore: generate ts declaration files #11

merged 2 commits into from
Sep 25, 2024

Conversation

filahf
Copy link
Contributor

@filahf filahf commented Sep 24, 2024

EDIT: Now follows #9 but with the --skipLibCheck flag

We discussed typeVersions in #8 and tsc --emitDeclaration in #9.

The tsc method introduces another dependency (which would break in CI). It also outputs the entire base64-encoded string as the type, for example:

declare const _default = 'data:application/json;base64,eyJnbH....'
export default _default

I’m unsure if this could cause performance issues with the tsserver, but it seems somewhat redundant.

This PR ensures that each file gets a corresponding declaration file with the following content:

declare const _default = 'base64'
export default _default

Result:
image

@abernier
Copy link
Member

abernier commented Sep 24, 2024

The tsc method introduces another dependency (which would break in CI)

but it passed with --skipLibCheck : #10 (comment)

it also outputs the entire base64-encoded string as the type

I guess it is ok, I would not touch and let tsc handle this

=> I would have prefered we stick with the previous tsx --emitDeclarationOnly solution (rather than generating it ourselves)

NB: And image seems funny :/

@filahf
Copy link
Contributor Author

filahf commented Sep 25, 2024

Alright, will fix! 👍

@abernier abernier merged commit d8709a9 into pmndrs:main Sep 25, 2024
Copy link

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@abernier
Copy link
Member

abernier commented Sep 25, 2024

ok it's merged, but @filahf I guess you are right about your concern for the whole string:

declare const _default = 'data:application/json;base64,eyJnbH....'

it should double the size of the package, even if types will be stripped at the end, so not so bad

let's see...

@filahf
Copy link
Contributor Author

filahf commented Sep 26, 2024

Great, I like the idea of putting the script outside of the makefile.

Yea it shouldn't be a problem bundle-wise but I'll try to do some perf testing in a real project with tsc --generateTrace

@abernier
Copy link
Member

#12

@filahf can you test it? (I tried but still get TS-warnings)
image

@filahf
Copy link
Contributor Author

filahf commented Sep 26, 2024

#12

@filahf can you test it? (I tried but still get TS-warnings)

I never got a plain single index.d.ts to work either. I had to specify the exports/typeVersions field to make ts happy (#8)

@abernier
Copy link
Member

#12
@filahf can you test it? (I tried but still get TS-warnings)

I never got a plain single index.d.ts to work either. I had to specify the exports/typeVersions field to make ts happy (#8)

weird no? it should work in my mind...

@filahf
Copy link
Contributor Author

filahf commented Sep 26, 2024

yea I agree. there is an open issue for this here microsoft/TypeScript#38638

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants