-
Notifications
You must be signed in to change notification settings - Fork 23
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
Reorganize repo; TypeScript config improvements #3463
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Ugh, hub and website builds are failing, because I think we should merge this PR first, then I'll update the paths in terraform configs and redeploy manually. |
Fixed. |
|
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.
If you think this is good, happy to go with it. Your explanation sounds reasonable.
I hope this helps with the Typescript complexity, I previously found that to be pretty annoying.
1.
packages/*
is split intopackages/
,internal-packages/
andapps/
This is aligned with turborepo guidelines. (except that I decided that
internal-packages
is useful to keep separately, for clarity).In the perfect world (or a bigger org) we'd have lint checks that
packages
don't depend oninternal-packages
. But not worth it for now, can check during reviews manually instead.2. Rethinking tsconfigs
No cross-package TypeScript references
Previously, we used cross-package references:
squiggle/packages/components/tsconfig.build.json
Lines 8 to 12 in 7736492
As mentioned in turborepo guides, this is not very useful and can be harmful:
turbo build
would already take these dependencies into account, automatically, and cache the results${configDir}
rootDir
andsrcDir
in the base config (internal-packages/configs/tsconfig.base.json
). This simplifies the config files.3 -> 2 tsconfigs in most packages
Previously, in packages with tests, I used a pattern where we had three configs, e.g. in components:
noEmit
enabledsrc
(since we don't want to build and package test files; also because building everything in the package resulted in the awkwarddist/src/
paths)Now I've managed to get the same result just two files:
src/
Notes:
tsBuildInfoFile
are configured explicitly; this is for aesthetical reasons, mostly, and I might change my mind in the future on how useful it is; explained here:squiggle/internal-packages/configs/tsconfig.base.json
Lines 23 to 29 in bcda7ae
tsconfig.json
, but if they have any*.ts
configs in the package's root, and want to type-check them (we previously were not very consistent about this...) they'd end up withdist/src
awkwardnessinternal-packages/content
uses a custom setup because it has to account for content-collections behavior3. Cleanups
packages/cli
(squiggle-cli-experimental) is removed, no one uses.squiggleU
anymoreexamples
dir moved from root topackages/squiggle-lang/
(do we even need it? we have Squiggle Hub now...)