Update TypeScript to 5.7.2 #1082
Conversation
🦋 Changeset detectedLatest commit: da3e822 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1082 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 97 97
Lines 1392 1392
Branches 351 351
=======================================
Hits 1390 1390
Misses 1 1
Partials 1 1
Continue to review full report in Codecov by Sentry.
|
|
Size Change: 0 B Total Size: 4.63 kB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks for doing this. The changes look good to me except for one. I have left a question/suggestion inline; but to summarize, I think we need to be more explicit about which modules/globals conflict so that future us can properly understand the implications of future updates and know when certain config values may no longer be relevant.
| // - node_modules/@google-cloud/kms/node_modules/long/index.d.ts:446:1 - error | ||
| // TS1203: Export assignment cannot be used when targeting ECMAScript modules. | ||
| // Consider using 'export default' or another module format instead. |
There was a problem hiding this comment.
This one is not a conflict. Maybe the .d.ts files will be different in a more recent version. I don't want to make changes that could modify the runtime behavior.
There was a problem hiding this comment.
praise: Thanks. This is really helpful. It won't catch any new issues that come up, but it gives us something to understand.
tsconfig-common.json
Outdated
| // - node_modules/@typescript-eslint/utils/node_modules/@typescript-eslint/typescript-estree/dist/ts-estree/ts-nodes.d.ts:19:3551 - error | ||
| // TS2694: Namespace 'ts' has no exported member 'InputFiles'. | ||
| // - node_modules/@typescript-eslint/utils/node_modules/@typescript-eslint/typescript-estree/dist/ts-estree/ts-nodes.d.ts:19:3567 - error | ||
| // TS2694: Namespace 'ts' has no exported member 'UnparsedSource'. |
There was a problem hiding this comment.
@typescript-eslint/typescript-estree provides ambient type definitions that conflict with the TypeScript types for TypeScript itself.
| // - node_modules/@google-cloud/kms/node_modules/long/index.d.ts:446:1 - error | ||
| // TS1203: Export assignment cannot be used when targeting ECMAScript modules. | ||
| // Consider using 'export default' or another module format instead. |
There was a problem hiding this comment.
praise: Thanks. This is really helpful. It won't catch any new issues that come up, but it gives us something to understand.
Summary:
Wonder Blocks is updating Storybook to 8.x which requires using "moduleResolution": "bundler" in its tsconfig, see https://stackoverflow.com/questions/72193784/why-doesnt-the-exports-field-of-npm-work-in-typescript for details. To fix this, I decided to update TypeScript in Wonder Blocks. Instead of updating to the minimally required version I figured I may as well upgrade to the most recent version since I'm upgrading anyways.
Although we probably don't need to update TypeScript in all of our repos at the same time (unless we start using new syntax), we may as well.
In theory, this shouldn't affect the output of the code since we use babel for compilation.
Issue: None
Test plan: