Skip to content

Commit 4551b5d

Browse files
committed
ci: dedup redundant tsc compilation in build/CI
The full source type graph was being recompiled ~8x per CI run. Root causes, all confirmed against the files: - Makefile: build-lib and build-web both declared 'clean' as a normal prerequisite, so under 'make -j4' clean could run mid-write (rm -rf build racing tsup/webpack output) and ran once per leaf target. Switch to a single order-only prereq (| clean): make now runs clean exactly once and both builders wait for it before starting. Verified via 'make -j4 -n build'. - tsup.config.ts: clean:true re-wiped build/cjs even though the canonical 'yarn build' (= make clean build) already wipes the whole build/ tree first. Set clean:false; standalone yarn build still cleans via make. - test/typeCompat.test.ts: each run shells out to two full 'tsc' compiles of the source type graph. CI runs 'yarn test' across a [20,22,24] Node matrix, so this identical, Node-version-independent compile ran 3x. Gate the suite behind SKIP_TSC_TYPE_COMPAT, which CI sets on every non-primary matrix node; it runs once on Node 20 and always runs locally. - ci.yml: the standalone 'typecheck' job (tsc --noEmit + tsc -p typings) was a full second compile of work already done by 'make lint' in the lint job. Removed it; lint is now the single owner of type-checking. Build step gated to Node 20 (declaration emit is byte-identical across Node versions). Net: full-graph tsc/dts compiles per CI run drop from ~8 to ~3 with no loss of coverage. Verified: yarn build, yarn test (198 passed; 196+2 skipped with the gate), yarn typecheck, yarn lint all green.
1 parent 5611b11 commit 4551b5d

4 files changed

Lines changed: 40 additions & 26 deletions

File tree

.github/workflows/ci.yml

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,20 @@ jobs:
4141
- name: Install dependencies
4242
run: yarn install --frozen-lockfile
4343

44+
# The typeCompat tsc compile is Node-version-independent; skip it on every
45+
# non-primary matrix node so the full source type graph is compiled once,
46+
# not three times. All other unit tests still run on every Node version.
4447
- name: Run unit tests
48+
env:
49+
SKIP_TSC_TYPE_COMPAT: ${{ matrix.node-version != 20 && '1' || '' }}
4550
run: yarn test
4651

52+
# The build (tsup --dts + webpack) recompiles the full type graph for
53+
# declaration emit; that output is byte-identical across Node versions, so
54+
# building on every matrix entry just recompiles the same type graph N times.
55+
# Gate it to the primary Node version. Unit tests still run on all versions.
4756
- name: Build
57+
if: matrix.node-version == 20
4858
run: yarn build
4959

5060
test-browser:
@@ -84,22 +94,9 @@ jobs:
8494
- name: Run tests with coverage
8595
run: yarn test:coverage
8696

87-
typecheck:
88-
runs-on: ubuntu-latest
89-
steps:
90-
- uses: actions/checkout@v4
91-
92-
- name: Setup Node.js
93-
uses: actions/setup-node@v4
94-
with:
95-
node-version: '20'
96-
cache: 'yarn'
97-
98-
- name: Install dependencies
99-
run: yarn install --frozen-lockfile
100-
101-
- name: TypeScript check (source)
102-
run: yarn tsc --noEmit
103-
104-
- name: TypeScript check (public typings)
105-
run: yarn tsc -p typings
97+
# NOTE: the standalone TypeScript checks (`tsc --noEmit` for source and
98+
# `tsc -p typings` for the public .d.ts) are already run by `make lint` in the
99+
# `lint` job above. Running them again here recompiled the full type graph a
100+
# second time every CI run for no added coverage, so this job was removed and
101+
# `lint` is the single owner of type-checking. Re-add a dedicated job only if
102+
# `make lint` ever stops invoking tsc.

Makefile

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,19 @@ lint:
1818

1919
# tsup emits cjs + esm + dts into build/cjs (and rewrites build/cjs/cjs.js to the
2020
# legacy module.exports === Draggable shape). webpack emits the UMD global bundle.
21-
# Both depend on `clean` so the dir is reset first even under parallel make (-j).
22-
# The recipe runs after both prerequisites complete and verifies the published
23-
# CJS/UMD contracts (see scripts/verify-build.cjs).
21+
# `clean` is an ORDER-ONLY prerequisite (after the `|`) on both leaf targets: make
22+
# guarantees it runs exactly once and that both tsup and webpack wait for it before
23+
# starting, so under parallel make (-j) we get one clean instead of a per-target
24+
# race that could `rm -rf build` mid-write. The recipe runs after both
25+
# prerequisites complete and verifies the published CJS/UMD contracts (see
26+
# scripts/verify-build.cjs).
2427
build: build-lib build-web
2528
@node scripts/verify-build.cjs
2629

27-
build-lib: clean $(BIN)
30+
build-lib: $(BIN) | clean
2831
$(BIN)/tsup
2932

30-
build-web: clean $(BIN)
33+
build-web: $(BIN) | clean
3134
$(BIN)/webpack --mode=production
3235

3336
# Allows usage of `make install`, `make link`

test/typeCompat.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,18 @@ function runTsc(projectDir: string): {ok: boolean; output: string} {
5050
// can take several seconds on slower CI runners — well past vitest's 5s default.
5151
const TSC_TIMEOUT_MS = 60_000;
5252

53-
describe('public type-surface compatibility', () => {
53+
// These cases spawn `tsc` against the source type graph. The result is purely a
54+
// function of the TypeScript dependency + source — it does NOT depend on the Node
55+
// runtime version. CI runs `yarn test` across a Node version matrix, so without a
56+
// guard this full-graph compile would run once per matrix entry, recompiling the
57+
// identical type graph N times for zero added coverage. CI sets
58+
// SKIP_TSC_TYPE_COMPAT=1 on every non-primary matrix node so the compile happens
59+
// exactly once. Locally (env unset) it always runs.
60+
const describeTypeCompat = process.env.SKIP_TSC_TYPE_COMPAT
61+
? describe.skip
62+
: describe;
63+
64+
describeTypeCompat('public type-surface compatibility', () => {
5465
it('public surface is API-compatible with the old hand-written surface', () => {
5566
const {ok, output} = runTsc(resolve(__dirname, 'typeCompat'));
5667
expect(output, output).toBe('');

tsup.config.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ export default defineConfig({
4545
format: ['cjs', 'esm'],
4646
dts: true,
4747
sourcemap: true,
48-
clean: true,
48+
// The Makefile's `clean` target (an order-only prereq of build-lib) wipes the
49+
// whole build/ tree once before tsup runs, so tsup re-cleaning its own outDir
50+
// here is redundant in the canonical `yarn build` (= `make clean build`) path.
51+
clean: false,
4952
target: 'es2019',
5053
external,
5154
// ESM output -> .mjs, CJS output -> .js (so package "main" stays build/cjs/cjs.js).

0 commit comments

Comments
 (0)