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

Publish Modern Javascript instead of Typescript #2644

Open
1 task
jorenbroekema opened this issue Nov 29, 2024 · 2 comments
Open
1 task

Publish Modern Javascript instead of Typescript #2644

jorenbroekema opened this issue Nov 29, 2024 · 2 comments

Comments

@jorenbroekema
Copy link
Contributor

jorenbroekema commented Nov 29, 2024

What version of starlight are you using?

0.29.2

What version of astro are you using?

4.16.16

What package manager are you using?

npm

What operating system are you using?

Windows 11 + WSL2 (Ubuntu 20.04)

What browser are you using?

Chrome

Describe the Bug

Whenever I'm upgrading Starlight, I always seem to be running into type issues:

image

These cannot be ignored by using skipLibCheck because that flag only disables the type validation of .d.ts files in node_modules.

Typescript doesn't care that a .ts file is coming from node_modules, if you're using it, it needs to compile it and verify the types.

I think the fundamental problem of this library is the fact that it's publishing .ts files to NPM rather than compiling it to .js + .d.ts and publishing that. This renders skipLibCheck useless and means that in order to prevent type issues, I have to be on the Typescript version that this library was published with. If this library was built in typescript 5.5.5 and 5.7.2 (current) is more restrictive somewhere, it means that I have to deal with the type issues of this library in my project. What's worse is, not just different typescript versions can cause these issues, if I have a more restrictive TSConfig than you guys do e.g. I'm using noImplicitAny but you guys are not, then I will also get type issues as a consumer about implicit any in my installation of your library.

Furthermore, things such as virtual import paths defined by module declarations inside handwritten .d.ts files only work for this library locally, but do not work in the context of this library being installed in some node_modules folder for the consumer.

So while I would appreciate it if the type issues were fixed, it would be a lot smarter to publish JS/D.TS files in the future to prevent these issues from arising to begin with.

To reproduce some of the type issues as an example, go to the stackblitz and run npm run lint:types. This also highlights that for example my "strict" and "noImplicitAny" settings are causing extra issues in starlight dependency, because it seems that the tsconfig that starlight uses is also more lenient than mine. This again emphasizes the fundamental problem of publishing TypeScript files: you force your consumer to adhere to your TS Config settings, because TypeScript will compile and verify types of Starlight using the settings of the consumer.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/withastro-astro-jtj49r

Participation

  • I am willing to submit a pull request for this issue.
@HiDeoo
Copy link
Member

HiDeoo commented Nov 29, 2024

Thanks for the feedback.

This is mostly the same issue as #983 or #1891 (which are only discussions/feature requests) and it's definitely something we want to fix. I attempted to fix this in the past multiple times but some changes in Astro now make it a bit easier and I plan to get back to it although it's something that I've pushed back due to other priorities.

Once the Astro 5 support is added to Starlight, I'm personally hoping to get back to this.

@jorenbroekema
Copy link
Contributor Author

Aaah cool, sorry I didn't check the discussions for something that was opened wrt this 👍🏻 for now I can get around it with patch-package + typescript ignores in the TS files, in case someone else needs a workaround for this in the meantime

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

2 participants