mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
perf(test): collapse hot dependency barrels via deps.optimizer (#4416)
* perf(test): collapse hot dependency barrels via deps.optimizer Import is 81-84% of unit-suite worker time, and the cost is linear in module COUNT rather than module weight: `pool: 'forks'` + `isolate: true` gives every test file a fresh process AND a fresh module registry, so each externalised package is re-imported cold once per file that reaches it. Two changes, both scoped to the `unit` / `unit-native` projects: 1. Eleven more packages in `deps.optimizer.ssr.include`. The optimizer collapses a package's many-hundred-file load into one chunk, paid once per process. Every added package was verified to have ZERO `vi.mock` callers across `src`, `test`, `packages` and `apps` -- that is the documented breakage mode, since pre-bundling wraps a package as a CJS-interop chunk and a mock factory returning only named exports then stops satisfying its consumers. `pg` was on the candidate list and was dropped: it has 2 mocking files in `packages/civitai-db`. `@electric-sql/pglite` passed the mock check and was still reverted, which is a second hazard now written into the config: pre-bundling relocates a module into `deps_ssr/` while a non-JS sidecar stays behind, so PGlite-backed suites died on `ENOENT: .../deps_ssr/pglite.data`. 2. `experimental.fsModuleCache` -- a per-module transform cache on disk that survives between separate `vitest run` invocations, which is the boundary `isolate: true` creates. It is format-versioned and lockfile-invalidated (`ensureCacheIntegrity()` clears it when the lockfile hash moves), so staleness self-corrects. Scoped per-project deliberately: a root-level value is inherited by every project, and the browser `component` suite and the `packages/*` / `apps/*` suites are not covered by the run that verified this. Verification -- the whole point, since the failure mode here is a config change that silently runs FEWER tests: baseline 22611 passed | 25 skipped (22636) 1454 files final (warm) 22611 passed | 25 skipped (22636) 1454 files Identical, exit 0. The pglite regression is why the passed count is the figure that matters: with pglite included the suite TOTAL stayed at exactly 22636 and `numFailedTests` stayed at 0, while 87 tests across 7 files were silently downgraded to SKIPPED. A total-count check would have waved that through. Timing is indicative only -- the box had parallel load -- but `transform` is the clean signal for the cache: 136.66s -> 27.53s on a warm run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test): drop the inert 'react' optimizer entry and record the subpath dual-instance hazard Three documentation corrections from an adversarial pre-merge audit. No runtime behaviour changes: the removed entry was already a no-op. 1. `'react'` was INERT and read as a delivered win. Vitest's resolveOptimizerConfig hardcodes `exclude = ["vitest", "react", "vue", ...]` and then filters `include` against it, so the entry was silently dropped (node_modules/vitest/dist/chunks/cli-api.CnMVyzaz.js:10109-10117). Verified at source, and by the produced bundle: deps_ssr/ contains react-dom.js and no react.js. Removed, with the reason recorded in its place so the next reader does not re-add it. 2. The comment above the added block said to re-run the suite and confirm the "collected test count" is unchanged — i.e. numTotalTests, which is exactly what the PGlite note sixteen lines above says never to use. Same hunk, opposite instruction, and it is the line someone follows when adding package #11. Corrected to PASSED. 3. Recorded a THIRD hazard class the comment did not enumerate: subpath dual-instance. With `noDiscovery: true` + `entries: []` only the BARE specifier is pre-bundled, so a subpath import resolves through the module runner to the original files and the two forms become distinct copies. Live in this repo today: `zod/v4` (4 files), `zustand/*` (50), `react-dom/*` (26). No failure observed, so this is latent rather than a live bug — but `instanceof` across two copies is the shape that breaks, and there is an `error instanceof z.ZodError` in src/pages/api/admin/manage-sanity-checks.ts. Audit also confirmed, against the installed Vitest source rather than the docs, the two mechanism claims this change rests on: deps.optimizer resolves strictly per-project (all 21 projects measured; only unit/unit-native carry a non-empty include), and a root-level `experimental` block WOULD leak fsModuleCache into every project — Vitest carries an explicit "always inherit" special case at :10372 — so the per-project scoping is necessary and works. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+63
-3
@@ -217,9 +217,28 @@ const unitTestConfig = {
|
||||
// (exports going missing as the graph grows). Measured: adding `redis` and `@aws-sdk/client-s3`
|
||||
// here takes four mock-holding files from 92 tests passing to 7 collected.
|
||||
//
|
||||
// Before adding a package, check `vi.mock('<pkg>'` across `src` returns nothing. The three
|
||||
// excluded on those grounds — `redis`, `@aws-sdk/client-s3`, `@aws-sdk/lib-storage` — are worth
|
||||
// ~275s and need `default` added to six mock factories first; that is a separate change.
|
||||
// Before adding a package, check `vi.mock('<pkg>'` across `src`, `test`, `packages` and `apps`
|
||||
// returns nothing — a subpath form (`vi.mock('pg/lib/x')`) counts. The four excluded on those
|
||||
// grounds:
|
||||
// `redis`, `@aws-sdk/client-s3`, `@aws-sdk/lib-storage` (5 / 2 / 3 mocking files) — worth
|
||||
// ~275s, and need `default` added to six mock factories first.
|
||||
// `pg` (2 mocking files, both in `packages/civitai-db`) — dropped from this list for exactly
|
||||
// that reason, not because it is cheap.
|
||||
//
|
||||
// 🔴 A zero-mocker package can still be unsafe, for a SECOND reason: a package that loads a
|
||||
// non-JS SIDECAR at runtime breaks when it is relocated. `@electric-sql/pglite` passes the
|
||||
// mock check and was still reverted out of this list — pre-bundling moves the module into
|
||||
// `node_modules/.vite/vitest/<hash>/deps_ssr/` while its WASM payload stays behind, so every
|
||||
// PGlite-backed suite dies on
|
||||
// ENOENT: open '…/deps_ssr/pglite.data'
|
||||
// Measured: 7 `*.behavior.test.ts` files went from 87 passing to 0, reported as SKIPPED rather
|
||||
// than failed, so the suite total stayed at 22,636 while 87 fewer tests actually executed.
|
||||
// Compare the PASSED count, never the total, when changing this list.
|
||||
// The three biggest fan-in packages measured over the test-reachable graph are blocked the same
|
||||
// way and are the follow-up this list is building toward:
|
||||
// `@tabler/icons-react` 5,952 package files / 166 importers — 2 mocking files
|
||||
// `next` 3,459 / 114 — 10 mocking files
|
||||
// `@mantine/core` 1,180 / 249 — 4 mocking files
|
||||
optimizer: {
|
||||
ssr: {
|
||||
enabled: true,
|
||||
@@ -229,10 +248,51 @@ const unitTestConfig = {
|
||||
'@tiptap/html',
|
||||
'@axiomhq/axiom-node',
|
||||
'@aws-sdk/s3-request-presigner',
|
||||
// Added below: each verified to have ZERO `vi.mock` callers anywhere in the workspace,
|
||||
// and the whole suite re-run to confirm the PASSED count is unchanged. Not the total —
|
||||
// see the PGlite note above, where the total stayed put while 87 tests stopped running.
|
||||
//
|
||||
// 🔴 A THIRD HAZARD CLASS, beyond mock-callers and non-JS sidecars: SUBPATH DUAL-INSTANCE.
|
||||
// `noDiscovery: true` + `entries: []` means only the BARE specifier is pre-bundled, so a
|
||||
// subpath import still resolves through the module runner to the original files and the
|
||||
// two forms become distinct copies. Live here today: `zod/v4` (4 files), `zustand/*`
|
||||
// (50), `react-dom/*` (26). No failure observed — but `instanceof` across the two copies
|
||||
// is the shape that breaks, e.g. `error instanceof z.ZodError` in
|
||||
// src/pages/api/admin/manage-sanity-checks.ts. Check subpath usage before adding a
|
||||
// package whose identity is compared with `instanceof`.
|
||||
'zod',
|
||||
// 'react' is NOT here on purpose. Vitest hardcodes
|
||||
// `exclude = ['vitest', 'react', 'vue']` in resolveOptimizerConfig and filters `include`
|
||||
// against it, so an entry for it is silently dropped — measured: `deps_ssr/` contains
|
||||
// react-dom.js but no react.js. Listing it would read as a delivered win that never
|
||||
// happened.
|
||||
'react-dom',
|
||||
'zustand',
|
||||
'immer',
|
||||
'clsx',
|
||||
'uuid',
|
||||
'prom-client',
|
||||
'@tanstack/react-query',
|
||||
'@paddle/paddle-node-sdk',
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
// A transform cache on disk, keyed per module and shared between separate `vitest run`
|
||||
// invocations — where `node_modules/.vite` only collapses the optimizer's work, this survives the
|
||||
// process boundary that `pool: 'forks'` + `isolate: true` creates for every single test file.
|
||||
//
|
||||
// Scoped to `unit`/`unit-native` rather than set at the root `test` block on purpose. A root-level
|
||||
// value is inherited by EVERY project (vitest resolves it into each project's config even without
|
||||
// `extends: true`), which would silently opt in the browser `component` project and the
|
||||
// `packages/*` + `apps/*` suites — none of which are covered by the run that verified this.
|
||||
// Widening it is a follow-up with its own verification, not a freebie.
|
||||
//
|
||||
// Staleness is self-correcting rather than something to remember: the cache carries a format
|
||||
// version, and `ensureCacheIntegrity()` hashes the lockfile on startup and nukes the whole cache
|
||||
// when it moves, so a dependency bump can't leave a stale transform behind. It writes to
|
||||
// `node_modules/.experimental-vitest-cache`, so it is gitignored with the rest of `node_modules`.
|
||||
experimental: { fsModuleCache: true },
|
||||
};
|
||||
|
||||
// Three Vitest projects sharing one config/runner:
|
||||
|
||||
Reference in New Issue
Block a user