-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Allow user to specify a custom project (tsconfig path) #43
Conversation
@@ -0,0 +1,9 @@ | |||
{ | |||
"extends": "../tsconfig.custom.json", |
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.
here
Friendly bump @isaacs |
Sorry, been a busy holiday season, just getting back to it. I think the only suggestion I have right now is it should maybe be called "project" to match the setting in ts-node and tsc? What do you think? |
Sounds good to me! Gonna update the PR in a bit |
src/tsconfig.ts
Outdated
: /* c8 ignore start */ | ||
join('..', config.project), | ||
/* c8 ignore stop */ |
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.
cc @isaacs -- are you ok with this? The behavior is being tested via the integration test (basic-custom-project
). Unclear whether there's a good way to test it at unit test level.
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 should be covered by the tests in test/tsconfig.ts
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 seems to work as intended, no major issues with the code. Does need a test, and the check against invalid settings could be better. You can update it or I can soon :)
src/tsconfig.ts
Outdated
: /* c8 ignore start */ | ||
join('..', config.project), | ||
/* c8 ignore stop */ |
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 should be covered by the tests in test/tsconfig.ts
Thanks @isaacs! I updated the file existence check, but I'm gonna defer the |
ad16ed0
to
d8b2d10
Compare
Related: #38
This PR adds a new
project
config for tshy to extend from. With this new option, you can have separate tsconfigs for typechecking (linting) and compilation, where tshy will only be using the compilation config. Let me know how you feel about this approach!Why?
It's common to have multiple tsconfig files co-exist in a project, where:
tsconfig.json
contains"noEmit": true
and"include": ["**/*.ts"]
. This is used by typechecking and type-awaretypescript-eslint
; VSCode also picks up this file by default.tsconfig.build.json
for compilation, invoked viatsc -p tsconfig.build.json
(or TypeScript project references in a monorepo).Here's a real-world monorepo example:
tsconfig.json
(default),tsconfig.build.json
, and type-aware eslint configtsconfig.build.json
,tsconfig.typecheck.json
, and the script to invoke compilation