-
Notifications
You must be signed in to change notification settings - Fork 307
fix: Update tsconfigs to resolve IDE type errors #1150
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: fe3b9b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Stably Runner - Test Suite - 'Smoke Test'Test Suite Run Result: 🟢 Success (4/4 tests passed) [dashboard] This comment was generated from stably-runner-action |
@@ -0,0 +1,33 @@ | |||
{ | |||
"compilerOptions": { |
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.
Is it possible for us to share the tsconfig base across api and app using this config, or are they fairly diverged?
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.
The api and common-utils configs had a lot of overlap, so I created a tsconfig.base to share between them (great idea!). There was a lot less overlap between app and the others though, so I didn't think it would be worth it to create a base config that all three packages could share
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.
Nice, should I try it locally? It looks good to me from my scan of the changes
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.
Yeah, if you could confirm the IDE type issues are gone for you locally as well, that would be great!
5bd4d46
to
d033d10
Compare
d033d10
to
d4c0b0b
Compare
Change looks good! Good improvement. Please commit the changeset |
d4c0b0b
to
fe3b9b1
Compare
@wrn14897 Thanks, added the changeset |
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.
Fixed for me locally, nice work!
Summary
This PR updates the structure of our tsconfigs so that IDEs (namely, VSCode) can successfully typecheck test files (which were previously excluded by the default
tsconfig
files in each package.The new structure is as follows:
tsconfig.json
. This is where the bulk of the configuration is, and is the file read by VSCode. It includes both source and test files.tsconfig.json
each extend from the sametsonfig.base.json
in the root directory, since they had quite a bit of shared configuration. The app package's tsconfig is sufficiently different that sharing a config between app and api was less helpful.tsconfig.build.json
file. This extends thetsconfig.json
and excludes test files. This file is used during builds (next build
for the app package andtsc
for the api package)tsup
, which does not run typechecking, so there is no need for atsconfig.build.json
in the common utils package. Thetsconfig.json
in the common utils package is used for linting, and both source and test files will be linted.Testing
IDE errors on test files are gone:

The image builds locally using

make build-local
:That image runs locally:

