-
Notifications
You must be signed in to change notification settings - Fork 31
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
Track @types/*
packages
#6966
Track @types/*
packages
#6966
Conversation
Size Change: -720 kB (-24%) 🎉 Total Size: 2.31 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we see this working in terms of telling us when things are wrong? The comment is really useful, but what could be really really useful is having alerting for when we regress, or even a weekly rundown?
Agree, also really needs an investment of time to resolve some of the wider issues at play here - visibility is great but it doesn't fix anything. I think when everyone is ready to upgrade to React 18 there will scope to eliminate at least a few of the mismatches. |
839aeeb
to
df51e7c
Compare
(Sorry I (Joe) just edited this by mistake) Max said something like: I think its worthwhile spending some time fixing the existing issues. One of the problems we have currently is that we can't add a test to check for mismatches as there are already a lot of them. One way to get this to pass in CI would be to add a list of “known issues”, for example: /* if the mismatches is in this list, warn instead of error */
const ignores = [
ajv,
compression,
he,
jest,
jsdom,
react,
react-dom,
webpack-node-externals,
] |
Mismatches in @types packages significantly reduce any type safety gained by using TypeScript. Create a regularly updating issue that tracks all `major.minor` versions for packages with a matching `@types/*`.
df51e7c
to
453ded8
Compare
Use tilde ranges for packages with a type definition, as this is what the contract from DefinitelyTyped expects https://github.com/DefinitelyTyped/DefinitelyTyped#how-do-definitely-typed-package-versions-relate-to-versions-of-the-corresponding-library https://github.com/npm/node-semver#tilde-ranges-123-12-1
I’ve now merged the tilde range CI action from #6971, @joecowton1 & @jamesgorrie, which addresses these concerns. |
⚡️ Lighthouse report for the changes in this PRLighthouse tested 2 URLs Report for Article
Report for Front
|
Okay thanks! In that case I'll add the |
What does this change?
Create a regularly updating issue (#6965) that tracks all
major.minor
versions for packages with a matching@types/*
.Why?
Mismatches in @types packages significantly reduce any type safety gained by using TypeScript.
Screenshots