-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: upgrade TypeScript to 4.4 #134
Conversation
Codecov Report
@@ Coverage Diff @@
## master #134 +/- ##
==========================================
+ Coverage 95.77% 95.79% +0.02%
==========================================
Files 70 70
Lines 1799 1809 +10
Branches 196 223 +27
==========================================
+ Hits 1723 1733 +10
Misses 75 75
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
packages/entity-database-adapter-knex/src/__integration-tests__/PostgresInvalidSetup-test.ts
Outdated
Show resolved
Hide resolved
orderBy: | ||
| { | ||
columnName: string; | ||
order: OrderByOrdering; | ||
}[] | ||
| undefined; | ||
offset: number | undefined; | ||
limit: number | undefined; |
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.
For APIs it's usually nicer to be able to omit a field than to need to write { unusedOption: undefined }
. Depending on how this code is called into, it might be better to write: { orderBy?: {...} | undefined }
to allow both.
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.
This is an internal-only type that is the internal representation of QuerySelectionModifiers, the main difference being that it contains the translated column name. Because it's used internally only we can ensure that all constructions of it contain explicit undefined (only place that creates one is convertToTableQueryModifiers
in this file).
8d7e782
to
2b1852d
Compare
🥳 |
Why
We're a few versions behind. This comes with quite a few new features, but the ones that affect this repo or are likely to affect it are:
noPropertyAccessFromIndexSignature
: stricter rules around property access on not-well-specified object types. Helps with property misspellings mostly.noUncheckedIndexedAccess
: stricter index access checks where array indexing can return undefined. This is much more in alignment with this codebase's style than unchecked, but I can see how it may not be appropriate for all codebases.exactOptionalPropertyTypes
: Makes assigning undefined to an object property value need to specify it as a possible RHS value rather than the odd left-hand?
operator. This is more along the lines of how I think about typesystems working with JS objects, and is how this codebase was written in most cases (fixes a couple). This helps with a rare but potential bug due to a difference between a property existing and being assigned to undefined and not existing.override
(next PR): https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-3.html#override-and-the---noimplicitoverride-flaguseUnknownInCatchVariables
in strict: Since JS can throw non-errors this is more correct, and even if we don't expect/allow first-party code to throw non-errors, libraries may still do it. This helps us catch odd bugs where that may be the case.In addition to those, it is faster in general.
How
Upgrade the package. Ensure all tests and docs generation pass.
Test Plan
Wait for CI.