build: improve consistency #5520
Conversation
| "build:stubs": "script/build-stubs fs:src/stubs/fs.ts glob:src/stubs/glob.ts os:src/stubs/os.ts jsdom:src/stubs/jsdom.ts", | ||
| "clean": "pnpm --filter $npm_package_name... clean:self", | ||
| "clean:self": "rm -rf build", |
There was a problem hiding this comment.
Why don't frontends get tsc --build --clean tsconfig.build.json?
There was a problem hiding this comment.
Because frontends don't use tsc to build. They use vite build, which internally uses rollup and doesn't do incremental builds across runs, so there's no build cache files to clear.
I'm interested in hearing more about this. Perhaps I'll put together a doc to collect feedback as this is an area I'd like to address in the coming months. |
| @@ -1,15 +1,10 @@ | |||
| APP := .. | |||
| export ADMIN_WORKSPACE=/tmp/admin-integration-testing | |||
|
|
|||
| build-frontend: | |||
| build: | |||
| # building the frontend builds the backend too | |||
There was a problem hiding this comment.
Is this comment now true because of the build change or was it already true?
There was a problem hiding this comment.
This was kind of true before insofar as the backends are TypeScript dependencies of the frontends, but with the same caveats that it might not actually build everything if there's any non-tsc work to be done.
|
Nice improvements. I hadn't dealt with the issue of Rust code not being rebuilt automatically, since I haven't been in the Rust code yet, but it's nice to know I won't have to think much of it. And the commands themselves feel easier to reason about with |
|
Discussed this with @eventualbuddha and I'm good with merging this for the v4.0 release! Adam and I will be building a few more rounds of images before cert submission, and I'll be conducting some final QA passes as well, so we should be able to comfortably catch any issues with this, if any. Overall not too worried since we aren't modifying source code. And I generally have a sense that builds will either work or they won't - I don't foresee this causing an issue with just a subset of features. |
This change is meant to fix the issue where dependencies may not be fully built when running `pnpm build` within a project. We used to be able to rely entirely on `tsc --build` to build transitive dependencies. Because `tsconfig.build.json` files all referenced the build configuration for the projects they needed, `tsc --build` would traverse the graph and ensure everything was built. Once we added our first Rust code, this was no longer true. Building `apps/scan/backend` would re-build the TypeScript code in `libs/ballot-interpreter`, but not the Rust code. We've lived with this mostly because that code doesn't change very often. However, it's never been a very good setup and has led to build caches being out of date and people getting fed up and running `pnpm -w clean-all` to start over, which is time-consuming. This change does the following: - ensure all `libs` and `apps` have both a `build` and `clean` script - make `build` and `clean` use `pnpm`'s recursive topologically-ordered script running to run `build:self` and `clean:self` for all dependencies, respectively - moves what used to be `build` and `clean` into `build:self` and `clean:self`, respectively - uses `tsc --build --clean` to remove `*.tsbuildinfo` files, but not to clear `build` (more on this below) - updates `cargo clean` calls to clean only files related to the current project Why remove `*.tsbuildinfo` files with `tsc --build --clean`? There are a couple reasons: - we use `noEmit: true` with `tsconfig.json` files because we only use those files for type checking (and so they include test files), so under normal circumstances we shouldn't have any `tsconfig.tsbuildinfo` files - the `tsconfig.build.tsbuildinfo` file associated with `tsconfig.build.json` is not necessarily adjacent to the file, for example if `rootDir` is set to `src/ts` and `outDir` is set to `build` then `tsconfig.build.tsbuildinfo` will actually be in the project's parent directory due to the way the path to the `*.tsbuildinfo` files are computed; using `tsc --build --clean` ensures the computed path matches that of `tsc --build` Related to the above, we don't rely on `tsc --build --clean` to clear `build` because it only removes files that would be output by `tsc --build`. If you rename a `.ts` file, you'll be stuck with its built `.js` file until you clear the `build` directory by some other means. There are further improvements I'd like to make, but this should at least ensure that running `pnpm build` in a project properly builds all of its dependencies, whether TypeScript or Rust.
The backend will be built automatically.
Missed a couple spots.
We no longer need these now that `pnpm build` knows how to build dependencies properly.
eventualbuddha
force-pushed
the
brian/build/improve-build-consistency
branch
from
18455f5 to
b29ae08
Compare
This change is meant to fix the issue where dependencies may not be fully built when running
pnpm buildwithin a project.We used to be able to rely entirely on
tsc --buildto build transitive dependencies. Becausetsconfig.build.jsonfiles all referenced the build configuration for the projects they needed,tsc --buildwould traverse the graph and ensure everything was built. Once we added our first Rust code, this was no longer true. Buildingapps/scan/backendwould re-build the TypeScript code inlibs/ballot-interpreter, but not the Rust code. We've lived with this mostly because that code doesn't change very often. However, it's never been a very good setup and has led to build caches being out of date and people getting fed up and runningpnpm -w clean-allto start over, which is time-consuming.This change does the following:
libsandappshave both abuildandcleanscriptbuildandcleanusepnpm's recursive topologically-ordered script running to runbuild:selfandclean:selffor all dependencies, respectivelybuildandcleanintobuild:selfandclean:self, respectivelytsc --build --cleanto remove*.tsbuildinfofiles, but not to clearbuild(more on this below)cargo cleancalls to clean only files related to the current projectWhy remove
*.tsbuildinfofiles withtsc --build --clean? There are a couple reasons:noEmit: truewithtsconfig.jsonfiles because we only use those files for type checking (and so they include test files), so under normal circumstances we shouldn't have anytsconfig.tsbuildinfofilestsconfig.build.tsbuildinfofile associated withtsconfig.build.jsonis not necessarily adjacent to the file, for example ifrootDiris set tosrc/tsandoutDiris set tobuildthentsconfig.build.tsbuildinfowill actually be in the project's parent directory due to the way the path to the*.tsbuildinfofiles are computed; usingtsc --build --cleanensures the computed path matches that oftsc --buildRelated to the above, we don't rely on
tsc --build --cleanto clearbuildbecause it only removes files that would be output bytsc --build. If you rename a.tsfile, you'll be stuck with its built.jsfile until you clear thebuilddirectory by some other means.There are further improvements I'd like to make, but this should at least ensure that running
pnpm buildin a project properly builds all of its dependencies, whether TypeScript or Rust.