-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
refactor: unifies spread types in their proper places #2067
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
great work @wellwelwel |
This comment was marked as resolved.
This comment was marked as resolved.
@sidorares, I'm thinking of closing this PR. Instead, I'm considering bringing here a Milestone that I have into a private repository where I test everything before bringing it here. Then, reach exactly at the same place as this PR, but in stages.
|
See #2072 |
It's a large TypeScript refactoring that includes every single
*.ts
file in the entire project.Why?
Many types are confusing because they are redefined several times, for example:
Pool
types are spread and defined inindex.d.ts
,typings/mysql/index.d.ts
andtypings/mysql/lib/Pool.d.ts
at the same timeConnection
,ConnectionOptions
,execute
,query
, etc.execute
andquery
are complex and have to do several ctrl + c and v around the typesTo consider
Connection
? It's simple: go toConnection.d.ts
file 👨🏻💻CI
npm run lint:code
now checks for every*.ts
file (including the tests)What does this refactoring change?
✅ These changes resolves 100% of
eslint
errors in*.ts
✅ Unifies the types in their proper places:
Pool
as example: now, all typings are defined intypings/mysql/lib/Pool.d.ts
execute
andquery
overloads too. Now both can be reused:execute
orquery
, it will automatically affect all types that need it (Connection
,PoolConnection
, etc.) 🤹🏻♀️✅ Increments
NodeNext
TypeScript compatibility by requires the.js
in local imports for*.ts
files✅ Resolves circular imports in every
*.ts
file✅ Patterns all imports as
import any from './'
import any = require('./')
ℹ️ Removes older TypeScript tests that are unused
@types/mocha
,@types/chai
,mocha
andchai
dependenciesLastly
It's the new index.d.ts: