-
Notifications
You must be signed in to change notification settings - Fork 218
build: typescript project references #1077
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
build: typescript project references #1077
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1077 +/- ##
=======================================
Coverage 79.50% 79.50%
=======================================
Files 77 77
Lines 2279 2279
Branches 577 577
=======================================
Hits 1812 1812
Misses 391 391
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e440863
to
b9ff262
Compare
|
c1f764f
to
056fd2c
Compare
I may be wrong, but even though it does not throw errors when I run the |
|
ed01644
to
c2d3fd8
Compare
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.
I'm unsure what is wrong with @hono/node-ws
... it looks like it might be an existing issue? Though I can't reproduce it outside of tsup
😩
I noticed there were a few older issues like honojs/hono#3327, might be related?
"compilerOptions": { | ||
"rootDir": "src", | ||
"outDir": "dist", | ||
"tsBuildInfoFile": "dist/tsconfig.build.tsbuildinfo", |
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.
tsBuildInfoFile
is used along with incremental
(enabled with composite: true
) to provide some performance improvements when building the project. They don't have to go in the dist
directory, it's just where the typescript-eslint
project puts them
"rootDir": "src", | ||
"outDir": "dist", | ||
"tsBuildInfoFile": "dist/tsconfig.build.tsbuildinfo", | ||
"emitDeclarationOnly": false |
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.
We can change this as we use tsup
to build the output.
One benefit of having tcs
emit declaration files during the build is it can generate declaration maps. They enable features like "Go to Definition" in tools like vscode. There are some conversations as to if these files are worth publishing to npm, tsup
also mentions it in their documentation.
There are also a few issues about this:
"include": ["src/**/*.ts"], | ||
"exclude": ["**/*.test.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.
Only include src files in the build, while excluding .test.ts
files.
The big benefit of this is it prevents test related globals from appearing as valid references in src files, eg; describe
, test
, expect
from vitest
"extends": "../../tsconfig.base.json", | ||
"compilerOptions": { | ||
"outDir": "../../dist/out-tsc/packages/ajv-validator", | ||
"types": ["vitest/globals"] |
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.
Allow .test.ts
files to reference vitest/globals
"outDir": "../../dist/out-tsc/packages/ajv-validator", | ||
"types": ["vitest/globals"] | ||
}, | ||
"include": ["**/*.test.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.
Only include .test.ts
files in this project
"references": [ | ||
{ | ||
"path": "./tsconfig.build.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.
Along with a reference to the project that includes the src files
"composite": false, | ||
"jsx": "react", | ||
"types": ["@cloudflare/workers-types", "node", "ws"] |
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.
tsup
struggles to understand composite projects, so it's simpler to just disable them
It also needs a reference to any types needed during build, though the jsx
configuration can probably just go in tsconfig.base.json
🤔
Hey @BarryThePenguin The |
Oops. Accidentally closed. Reopened. |
3c14afa
to
4ce55d1
Compare
Awesome, looks good now 👍🏻 Thanks 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.
LGTM!
Looks good! I'll merge this now. Thank you! |
* build: typescript project references * chore: remove duplicate keys
This is heavily based on the https://github.com/typescript-eslint/typescript-eslint monorepo. The TypeScript documentation for Project References is also useful.
There are 3 main tsconfig files:
tsconfig.base.json
the base configuration that everything else extends fromtsconfig.json
that references all the workspaces in the monorepotsconfig.tsup.json
that is used bytsup
when building a single workspaceThen each workspace has it's own
tsconfig.json
file that is referenced by the roottsconfig.json
to include the following files:package/*/tsconfig.build.json
used to build/typecheck the workspace filespackage/*/tsconfig.spec.json
used to typecheck test filesThe benefits of using project references include:
yarn typecheck
command to check the types across the whole monorepoyarn typecheck
to an individual workspaces, which is useful for keeping CI quick@hono/zod-openapi
and@hono/zod-validator
projectService
configuration.This is a lot of extra configuration, so I understand if the overhead of managing it all outweighs the benefits of running typescript across the entire monorepo.
The alternative, which is basically what exists now, would be just running typescript in individual workspaces. Though it doesn't come with the benefits that I mentioned above..