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

Ensure no implicit @types/ includes #912

Merged
merged 3 commits into from
Jul 12, 2024
Merged

Conversation

voxpelli
Copy link
Collaborator

To avoid the errors seen in #899 as a result of #897 this is a companion follow up to the #898 that @Emiyaaaaa made that also removes any implicitly includes @types/ package.

When no types is specified in the tsconfig.json, then typescript will pull in all types in ./node_modules/@types/, no matter if those appeared there due to us including it or because it's been hoisted from one of our dependencies.

Since we specify no @types/ dependencies all such types are hoisted from our dependencies and seems to currently amount to:

Skärmavbild 2024-07-12 kl  01 56 14

Most importantly this means that we have been pulling in a random @types/node and implicitly relied on it (seemingly by relying on xo which relies on eslint-import-resolver-webpack ➡️ webpack ➡️ terser-webpack-plugin ➡️ jest-worker ➡️ @types/node@*)

This PR sets "types": [] to ensure no such implicit types are included, which uncovers the issue mentioned in #899 which it solves the same way as the dom specific types were solved.

Related: #908

@sindresorhus
Copy link
Owner

CI is failing

@voxpelli
Copy link
Collaborator Author

@sindresorhus Fixed it now by adding an ignore comment in that specific place

Not sure how to best handle tests of stuff that's only available in some contexts. Similar issue as #908 has.

For now I think ignoring is okay as this PR is mostly to ensure no new mistakes are being made

@sindresorhus sindresorhus merged commit 8040abc into main Jul 12, 2024
8 checks passed
@sindresorhus sindresorhus deleted the no-implicit-type-includes branch July 12, 2024 12:05
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.

2 participants