Skip to content
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

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

keyz
Copy link
Contributor

@keyz keyz commented Jan 4, 2024

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:

  • The default tsconfig.json contains "noEmit": true and "include": ["**/*.ts"]. This is used by typechecking and type-aware typescript-eslint; VSCode also picks up this file by default.
  • A separate tsconfig.build.json for compilation, invoked via tsc -p tsconfig.build.json (or TypeScript project references in a monorepo).

Here's a real-world monorepo example:

@keyz keyz changed the title Allow user to configure a custom tsconfig path Allow user to specify a custom tsconfig path Jan 4, 2024
@@ -0,0 +1,9 @@
{
"extends": "../tsconfig.custom.json",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here

@keyz
Copy link
Contributor Author

keyz commented Jan 10, 2024

Friendly bump @isaacs

@isaacs
Copy link
Owner

isaacs commented Jan 10, 2024

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?

@keyz
Copy link
Contributor Author

keyz commented Jan 10, 2024

Sounds good to me! Gonna update the PR in a bit

@keyz keyz changed the title Allow user to specify a custom tsconfig path Allow user to specify a custom project (tsconfig path) Jan 10, 2024
src/tsconfig.ts Outdated
Comment on lines 47 to 49
: /* c8 ignore start */
join('..', config.project),
/* c8 ignore stop */
Copy link
Contributor Author

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.

Copy link
Owner

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

Copy link
Owner

@isaacs isaacs left a 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
Comment on lines 47 to 49
: /* c8 ignore start */
join('..', config.project),
/* c8 ignore stop */
Copy link
Owner

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

src/valid-project.ts Outdated Show resolved Hide resolved
@isaacs isaacs mentioned this pull request Jan 12, 2024
@keyz
Copy link
Contributor Author

keyz commented Jan 12, 2024

Thanks @isaacs! I updated the file existence check, but I'm gonna defer the test/tsconfig.ts to you if that's ok -- think that might require a few changes to the test setup.

@isaacs isaacs closed this in d8b2d10 Jan 16, 2024
@isaacs isaacs merged commit d8b2d10 into isaacs:main Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants