test(scripts,types): judge the spawned-build env by REMOVAL, and correct three comments that outran their measurement (objectui#8712)

.changeset/8712-ratchet-vacuous-green.md

@@ -0,0 +1,12 @@
+---
+---
+
+Test- and comment-only; nothing this package publishes changes.
+
+The objectui#8598 ratchet over builds a test spawns now judges the child
+environment by what it does to `VITEST` rather than by whether the token occurs
+in the expression: an environment that SETS the variable is refused, and the
+rest-pattern spelling of the scrub is accepted alongside `delete`. Two comments
+about `emptyOutDir` on the `./zod` bundle config are corrected to the measured
+effect of a flip, and one built-artifact case is renamed to the property it
+actually measures.

packages/types/src/__tests__/zod-subpath-bundle-config-8598.test.ts

@@ -22,9 +22,16 @@
  *
  *  1. ⛔ `emptyOutDir: false`. `outDir` is inside the project root, so Vite's
  *     DEFAULT is to empty it — and by the time this config runs, `dist/` holds
- *     the 124 files `tsc` just emitted for the whole package. Flipping this
- *     deletes the published package and leaves one bundle behind; the build
- *     still exits 0. This is the assertion this file exists for.
+ *     everything `tsc` just emitted for the whole package. ⚠️ What a flip costs
+ *     is stated as measured, because this header used to overstate it
+ *     (objectui#8712). On `2596b1b85`, `vite build --emptyOutDir` took `dist/`
+ *     from 128 files to 89 — the whole loss inside `dist/zod/` (40 → 1),
+ *     carrying the `./zod` typings and every per-category zod module. `outDir`
+ *     is a SUBDIRECTORY, so what a flip empties is that directory, not the
+ *     published package as a whole. `vite build` reports it as a success; the
+ *     build script reds one step later, in `check:dist-completeness`. This
+ *     assertion is what fails FIRST, in `unit`, before a build is ever run, and
+ *     it is the assertion this file exists for.
  *  2. The entry is `src/zod/index.zod.ts` and the output lands on
  *     `dist/zod/index.zod.js`. Those two together are what makes this an
  *     IN-PLACE overwrite of one `tsc` output rather than a new published file:
@@ -141,9 +148,10 @@ describe('objectui#8598 — the `./zod` subpath build config', () => {
   it('⛔ never empties the out dir — `dist/` holds the whole published package by then', () => {
     expect(
       build.emptyOutDir,
-      '`outDir` is inside the project root, so Vite empties it by DEFAULT. `tsc` has ' +
-        'already written all of `dist/` when this build runs, so the default deletes the ' +
-        'published package and exits 0.',
+      '`outDir` is inside the project root, so Vite empties it by DEFAULT — and `tsc` has ' +
+        'already written all of `dist/` when this build runs. A flip deletes everything in ' +
+        '`dist/zod/`, the `./zod` typings included, and `vite build` reports that as a ' +
+        'success; the build only reds one step later, in `check:dist-completeness`.',
     ).toBe(false);
   });
 

packages/types/src/__tests__/zod-subpath-single-module-8598.dist.spec.tsx

@@ -112,7 +112,19 @@ describe('objectui#8598 — the shipped `./zod` face is one bundled module', ()
     ).toEqual([]);
   });
 
-  it('REFUSES a nested off-spec node through a single-schema entry', () => {
+  /**
+   * ⚠️ An ACCEPT-SET control over the WHOLE module — ⛔ not a measurement of the
+   * single-schema entry, which is what this case used to claim in its name
+   * (objectui#8712). A vitest import evaluates the entire barrel, as this
+   * file's header already says, so the #8344 fill is reachable here whatever
+   * the module boundary looks like. Measured against a `tsc` barrel of 30
+   * separate modules — the ablated shape this file exists to refuse — the case
+   * below PASSED, and the file went red through the structural case above,
+   * which is the one that can see the defect. What this case holds is the other
+   * half: the schema still refuses what it must and accepts what it must, so a
+   * bundle that had started refusing everything cannot pass for a fix.
+   */
+  it('REFUSES a nested off-spec node, and accepts its well-formed twin', () => {
     expect(BUILT!.CardSchema.safeParse(NESTED_OFF_SPEC).success).toBe(false);
     // The control: the same slot, on-spec, still parses. A schema that refused
     // everything would satisfy the case above and be a much worse regression.

packages/types/vite.config.ts

@@ -49,9 +49,17 @@ if (process.env.VITEST) {
  * expected set from tsconfig and only counts PRESENCE, the `./zod` typings stay
  * `tsc`'s own, and no `exports` / `files` / `sideEffects` field moves.
  *
- * ⛔ `emptyOutDir` MUST stay false. `dist/` holds `tsc`'s 124 files by the time
- * this runs, and the default for an `outDir` inside the project root is to empty
- * it — which would delete the whole published package and leave one bundle.
+ * ⛔ `emptyOutDir` MUST stay false — and what a flip actually costs is stated
+ * here exactly, because this comment used to overstate it (objectui#8712).
+ * `outDir` is INSIDE the project root, so Vite's default is to empty it, and
+ * `tsc` has already written all of `dist/` by the time this runs. Measured on
+ * `2596b1b85` with `vite build --emptyOutDir`: `dist/` went 128 files → 89, all
+ * of the loss inside `dist/zod/` (40 → 1) — the `./zod` typings and every
+ * per-category zod module. So the flip empties that ONE directory rather than
+ * the published package as a whole, and it is not silent: `vite build` itself
+ * exits 0, and the build script's third step then reds with
+ * `missing 39 of 128 files tsc emits`. A gate one step later is still a
+ * published package built wrong, so the setting stays false.
  */
 export default defineConfig({
   build: {

scripts/__tests__/spawned-build-vitest-env-8598.test.ts

@@ -52,6 +52,14 @@ const ROOT = path.resolve(fileURLToPath(import.meta.url), '../../..');
  *  3. Spawns are found by AST, not by substring: `toContain('VITEST')` is
  *     satisfied by the word appearing in a comment, which is precisely the shape
  *     of a call site somebody explained instead of fixing.
+ *  4. ⭐ The environment is judged by what it DOES to `VITEST`, never by whether
+ *     the token occurs in it. Measured for objectui#8712, while the judgement
+ *     was `env.includes('VITEST')`: `env: { ...process.env, VITEST: 'true' }` —
+ *     a tree that SETS the variable this gate exists to remove — passed at
+ *     `2 passed`, and the idiomatic rest-pattern scrub
+ *     `const { VITEST: _v, ...BUILD_ENV } = process.env` was REFUSED at both
+ *     call sites. Presence of the token was the wrong question in both
+ *     directions at once. Removal is the property, so removal is what is read.
  */
 
 /** Node child-process entry points that start a program. */
@@ -79,41 +87,144 @@ function testFiles(): string[] {
   return found.sort();
 }
 
+/**
+ * What the resolved `env:` expression does to `VITEST`.
+ *
+ * `scrubbed` — it is REMOVED, in either idiomatic spelling: `delete env.VITEST`
+ *   on a copy, or a rest pattern that binds the key away
+ *   (`const { VITEST: _v, ...env } = process.env`). Both are correct fixes and
+ *   both are accepted.
+ * `sets`     — an object literal ASSIGNS `VITEST`. ⛔ The one shape that must
+ *   never pass: it is the exact leak this gate exists to stop, spelled out.
+ * `inherits` — neither of those. No `env:` at all, or one that hands the child
+ *   the parent's environment unchanged.
+ */
+type EnvVerdict = 'scrubbed' | 'sets' | 'inherits';
+
 interface BuildSpawn {
   readonly file: string;
   readonly line: number;
-  /**
-   * Text of the `env:` value passed in the options object, if any — with an
-   * IDENTIFIER resolved to its declaration in the same file.
-   *
-   * ⚠️ Resolving is the difference between this gate working and this gate
-   * looking like it works. The fix it exists to enforce is naturally written as
-   * a named constant (`env: BUILD_ENV`), and the text of that property contains
-   * no `VITEST` at all — so a gate reading the property alone reports the FIXED
-   * tree as leaking. Measured: it did, on both call sites, before this resolved.
-   * ⛔ The resolution is also deliberately narrow — the declaration of that one
-   * name, never the whole file — because a file-wide substring search passes on
-   * any mention of `VITEST` anywhere, including a comment about it.
-   */
-  readonly env: string | null;
+  readonly verdict: EnvVerdict;
+}
+
+/** The name a property or binding element is keyed by, when it is a plain one. */
+function keyName(key: ts.PropertyName | ts.BindingName | undefined): string | null {
+  if (key === undefined) return null;
+  if (ts.isIdentifier(key)) return key.text;
+  if (ts.isStringLiteralLike(key)) return key.text;
+  return null;
+}
+
+/** `delete <anything>.VITEST`, in either accessor spelling. */
+function deletesVitest(node: ts.Node): boolean {
+  if (!ts.isDeleteExpression(node)) return false;
+  const target = node.expression;
+  if (ts.isPropertyAccessExpression(target)) return target.name.text === 'VITEST';
+  return (
+    ts.isElementAccessExpression(target) &&
+    ts.isStringLiteralLike(target.argumentExpression) &&
+    target.argumentExpression.text === 'VITEST'
+  );
 }
 
-/** The initializer text of a `const`/`let` named `name` in this file, if there is one. */
-function declarationText(source: ts.SourceFile, name: string): string | null {
-  let text: string | null = null;
+/** `{ VITEST: _v, ...rest }` — binds the key away and collects everything else. */
+function omitsVitest(pattern: ts.ObjectBindingPattern): boolean {
+  const collectsRest = pattern.elements.some((element) => element.dotDotDotToken !== undefined);
+  const bindsVitest = pattern.elements.some(
+    (element) =>
+      element.dotDotDotToken === undefined &&
+      keyName(element.propertyName ?? element.name) === 'VITEST',
+  );
+  return collectsRest && bindsVitest;
+}
+
+/** An object literal that ASSIGNS `VITEST` — the leak, written on purpose. */
+function assignsVitest(node: ts.Node): boolean {
+  if (!ts.isObjectLiteralExpression(node)) return false;
+  return node.properties.some(
+    (property) =>
+      (ts.isPropertyAssignment(property) || ts.isShorthandPropertyAssignment(property)) &&
+      keyName(property.name) === 'VITEST',
+  );
+}
+
+/** Where a `const`/`let` named `name` in this file gets its value. */
+interface Declared {
+  /** Its initializer, when the name is bound directly. */
+  readonly initializer: ts.Expression | null;
+  /** The pattern it is the rest element of, when it is destructured out of one. */
+  readonly restOf: ts.ObjectBindingPattern | null;
+}
+
+function declarationOf(source: ts.SourceFile, name: string): Declared | null {
+  let found: Declared | null = null;
   const visit = (node: ts.Node): void => {
-    if (
-      ts.isVariableDeclaration(node) &&
-      ts.isIdentifier(node.name) &&
-      node.name.text === name &&
-      node.initializer !== undefined
-    ) {
-      text = node.initializer.getText(source);
+    if (ts.isVariableDeclaration(node)) {
+      if (ts.isIdentifier(node.name) && node.name.text === name && node.initializer !== undefined) {
+        found = { initializer: node.initializer, restOf: null };
+      } else if (ts.isObjectBindingPattern(node.name)) {
+        for (const element of node.name.elements) {
+          if (
+            element.dotDotDotToken !== undefined &&
+            ts.isIdentifier(element.name) &&
+            element.name.text === name
+          ) {
+            found = { initializer: null, restOf: node.name };
+          }
+        }
+      }
     }
     node.forEachChild(visit);
   };
   visit(source);
-  return text;
+  return found;
+}
+
+/**
+ * What an `env:` expression does to `VITEST`.
+ *
+ * ⚠️ Resolving names is the difference between this gate working and this gate
+ * looking like it works. The fix it enforces is naturally written as a named
+ * constant (`env: BUILD_ENV`), and the property alone says nothing about
+ * `VITEST` — so a gate reading only the property reports the FIXED tree as
+ * leaking. Measured: it did, on both call sites, before this resolved.
+ *
+ * ⛔ The resolution stays deliberately narrow — the declaration of the name the
+ * spawn passes, and the names spread into it, never the whole file. A wider
+ * search answers "does this file mention `VITEST`", and a comment about the
+ * variable answers that just as well as a line removing it.
+ */
+function verdictFor(value: ts.Expression, source: ts.SourceFile, seen: Set<string>): EnvVerdict {
+  if (ts.isIdentifier(value)) {
+    if (seen.has(value.text)) return 'inherits';
+    seen.add(value.text);
+    const declared = declarationOf(source, value.text);
+    if (declared === null) return 'inherits';
+    if (declared.restOf !== null) return omitsVitest(declared.restOf) ? 'scrubbed' : 'inherits';
+    return declared.initializer === null
+      ? 'inherits'
+      : verdictFor(declared.initializer, source, seen);
+  }
+
+  let removes = false;
+  let assigns = false;
+  const visit = (node: ts.Node): void => {
+    if (deletesVitest(node)) removes = true;
+    if (ts.isObjectBindingPattern(node) && omitsVitest(node)) removes = true;
+    if (assignsVitest(node)) assigns = true;
+    if (ts.isSpreadAssignment(node) && ts.isIdentifier(node.expression)) {
+      const inner = verdictFor(node.expression, source, seen);
+      if (inner === 'scrubbed') removes = true;
+      if (inner === 'sets') assigns = true;
+    }
+    node.forEachChild(visit);
+  };
+  visit(value);
+
+  // An assignment wins over a removal in the same expression: whatever else it
+  // did, the child ends up carrying the variable.
+  if (assigns) return 'sets';
+  return removes ? 'scrubbed' : 'inherits';
 }
 
 /** Every call in `file` that starts a child process running a `build` script. */
@@ -144,22 +255,23 @@ function buildSpawns(file: string): BuildSpawn[] {
           : '';
       if (SPAWNERS.has(callee) && namesABuild(node)) {
         const options = node.arguments.find(ts.isObjectLiteralExpression.bind(ts));
-        let env: string | null = null;
+        let verdict: EnvVerdict = 'inherits';
         if (options !== undefined) {
           for (const property of options.properties) {
             const key =
               property.name !== undefined && ts.isIdentifier(property.name) ? property.name.text : '';
-            if (key !== 'env' || !ts.isPropertyAssignment(property)) continue;
-            const value = property.initializer;
-            env = ts.isIdentifier(value)
-              ? (declarationText(source, value.text) ?? value.getText(source))
-              : value.getText(source);
+            if (key !== 'env') continue;
+            if (ts.isPropertyAssignment(property)) {
+              verdict = verdictFor(property.initializer, source, new Set<string>());
+            } else if (ts.isShorthandPropertyAssignment(property)) {
+              verdict = verdictFor(property.name, source, new Set<string>());
+            }
           }
         }
         found.push({
           file,
           line: source.getLineAndCharacterOfPosition(node.getStart(source)).line + 1,
-          env,
+          verdict,
         });
       }
     }
@@ -189,8 +301,11 @@ describe(`objectui#8598 — ${SPAWNS.length} build spawn(s) in the test tree`, (
   });
 
   it('every one of them scrubs VITEST from the child environment', () => {
-    const leaking = SPAWNS.filter((s) => s.env === null || !s.env.includes('VITEST')).map(
-      (s) => `${s.file}:${s.line}`,
+    const leaking = SPAWNS.filter((s) => s.verdict !== 'scrubbed').map(
+      (s) =>
+        `${s.file}:${s.line} — this environment ${
+          s.verdict === 'sets' ? 'SETS `VITEST`' : 'hands the child `VITEST` unchanged'
+        }`,
     );
 
     expect(
@@ -199,7 +314,8 @@ describe(`objectui#8598 — ${SPAWNS.length} build spawn(s) in the test tree`, (
         "child's environment. Every packages/* vite config refuses to load when it sees that " +
         'variable with a cwd that is not the repo root, so the build exits 1 before the bundler ' +
         'runs and the test reports a build failure it did not cause (objectui#8598, ' +
-        '`Test (shard 2/4)`). Pass an env with `VITEST` deleted — see `BUILD_ENV` in ' +
+        '`Test (shard 2/4)`). Remove the key — `delete env.VITEST` on a copy, or a ' +
+        '`{ VITEST: _v, ...env }` rest pattern; both are accepted. See `BUILD_ENV` in ' +
         `${NAMED_MEMBER}:\n  ` + leaking.join('\n  '),
     ).toEqual([]);
   });