test(console-starter): record unresolvable relative imports in the alias closure

.changeset/alias-closure-relative-miss-5386.md

@@ -0,0 +1,39 @@
+---
+---
+
+Tests only — this publishes nothing, declared explicitly with an empty frontmatter
+rather than left undeclared. No package `src/` is touched; the only file changed is
+`examples/console-starter/test/vite-alias-closure.test.ts`, and that example is private.
+
+Makes the alias-closure walker record an unresolvable **relative** import instead of
+dropping it.
+
+`computeClosure()` walks two kinds of specifier. The bare-specifier branch pushed a miss
+onto `unresolvable`; the relative branch dropped one with no record at all. So
+`expect(closure.unresolvable).toEqual([])` was not a weak assertion for relative imports
+— it was a structurally empty one. It could not fail no matter how many relative
+specifiers the walk failed to follow, and the `filesWalked` floor was the only signal
+that anything had gone wrong.
+
+That is how two earlier conversions to explicit extensions (objectui#4538, objectui#5214)
+each truncated this walk while landing green: the floor had enough slack to absorb both,
+and only went red once app-shell — the largest package — converted and took the count
+under 500. The direct symptom was being swallowed one branch away the whole time.
+
+Measured on `main` at the time of this change, with the resolver ablated to its
+pre-objectui#5357 behaviour to reproduce that regression class: 275 relative specifiers
+dropped, `filesWalked` 1245 to 402, packages reached 29 to 22 — and `unresolvable` still
+reporting `[]`. With this change the same ablation fails the suite with all 275 named,
+each alongside the file that imports it.
+
+The miss is recorded only for specifiers that are *meant* to be modules — no extension,
+or one of the JS/TS emitted extensions. `ts.preProcessFile` also reports `./styles.css`,
+`./data.json` and `./logo.svg`, which `resolveModule` cannot resolve by design, so
+recording those identically would fail the suite for a reason that is not a defect.
+Assets and Vite resource specifiers (`?raw`, `?url`, `?inline`) are skipped into a
+separate `nonModuleSkipped` list — explicitly, and observably, rather than by accident.
+
+Four fixtures pin the class against the real walker so a future edit cannot silently
+re-blind the branch: `./Foo.js` resolving through to the `Foo.tsx` on disk, planted
+unresolvable modules of both spellings being named, assets staying out of `unresolvable`
+while still being accounted for, and the classifier's boundary cases.

examples/console-starter/test/vite-alias-closure.test.ts

@@ -32,6 +32,7 @@
 
 import { describe, it, expect } from 'vitest';
 import fs from 'node:fs';
+import os from 'node:os';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import ts from 'typescript';
@@ -81,6 +82,45 @@ function resolveModule(target: string): string | null {
   return null;
 }
 
+/**
+ * The spellings a specifier that is *meant to be a module* can end with. Derived
+ * from `RESOLVABLE` so the two cannot drift, plus the two TypeScript module
+ * extensions that are legal to write but that `resolveModule` never has to
+ * probe for (a `.mts` source is imported as `.mjs`).
+ */
+const MODULE_EXTENSIONS = [...RESOLVABLE, '.mts', '.cts'];
+
+/**
+ * Does this relative specifier denote a module the walk is supposed to follow?
+ *
+ * `ts.preProcessFile` reports EVERY import specifier, including `./styles.css`,
+ * `./data.json` and `./logo.svg` — which `resolveModule` deliberately cannot
+ * resolve, because its candidate list is JS and TS only. Those are not walk
+ * failures, and recording them as such would fail this test for a reason that
+ * is not a defect. The closure contains two `.css` imports today, both of which
+ * happen to resolve only because `resolveModule` returns any path that exists
+ * on disk verbatim; a stylesheet that is virtual, generated, or shipped only in
+ * `dist` would land in the miss branch the moment it appeared.
+ *
+ * So the boundary is drawn on *intent*, not on whether resolution happened to
+ * succeed: no extension, or one of the JS/TS emitted extensions, is a module.
+ * Anything else is an asset and is skipped EXPLICITLY — recorded in
+ * `nonModuleSkipped` rather than dropped — so that the skip stays auditable and
+ * this branch can never again go quiet by accident.
+ *
+ * A specifier carrying a `?query` or `#hash` is a Vite resource specifier
+ * (`?raw`, `?url`, `?inline`, `?worker`) rather than a plain module path, and
+ * the walk cannot follow it meaningfully — it counts as a non-module skip. The
+ * closure contains none today.
+ */
+function isModuleSpecifier(specifier: string): boolean {
+  if (specifier.includes('?') || specifier.includes('#')) return false;
+  // `.` and `..` are extensionless directory imports, not an `.`-extension.
+  const ext = /\.[a-zA-Z0-9]+$/.exec(specifier)?.[0];
+  if (!ext) return true;
+  return MODULE_EXTENSIONS.includes(ext.toLowerCase());
+}
+
 const packageSrc = (pkg: string) =>
   path.join(repoRoot, 'packages', pkg.slice('@object-ui/'.length), 'src');
 
@@ -99,15 +139,23 @@ interface Closure {
   direct: Set<string>;
   filesWalked: number;
   unresolvable: string[];
+  /**
+   * Relative specifiers the walk skipped on purpose because they are assets,
+   * not modules. Never asserted empty — this exists so the skip is observable
+   * instead of implicit, and so a fixture can prove the boundary is drawn where
+   * it is documented to be.
+   */
+  nonModuleSkipped: string[];
 }
 
-function computeClosure(): Closure {
+function computeClosure(entryDir: string = path.join(exampleDir, 'src')): Closure {
   const packages = new Map<string, Set<string>>();
   const direct = new Set<string>();
   const unresolvable: string[] = [];
+  const nonModuleSkipped: string[] = [];
   const seen = new Set<string>();
 
-  const exampleSrc = path.join(exampleDir, 'src');
+  const exampleSrc = entryDir;
 
   function walk(file: string): void {
     if (seen.has(file)) return;
@@ -122,7 +170,24 @@ function computeClosure(): Closure {
     for (const specifier of specifiers) {
       if (specifier.startsWith('.')) {
         const resolved = resolveModule(path.resolve(path.dirname(file), specifier));
-        if (resolved) walk(resolved);
+        if (resolved) {
+          walk(resolved);
+          continue;
+        }
+        // A miss used to vanish here with no record, which made
+        // `unresolvable` structurally empty for this entire specifier class:
+        // `expect(closure.unresolvable).toEqual([])` could not fail for a
+        // relative import no matter how many the walk failed to follow. That is
+        // how objectui#4538's and objectui#5214's conversions each truncated
+        // this walk while landing green — the `filesWalked` floor was the only
+        // signal there was, and it had enough slack to hide two pull requests.
+        if (isModuleSpecifier(specifier)) {
+          unresolvable.push(
+            `${specifier} (unresolvable relative import, imported by ${path.relative(repoRoot, file)})`,
+          );
+        } else {
+          nonModuleSkipped.push(`${specifier} (imported by ${path.relative(repoRoot, file)})`);
+        }
         continue;
       }
       // Third-party and @objectstack/* are real registry dependencies; only
@@ -149,7 +214,7 @@ function computeClosure(): Closure {
     if (/\.(ts|tsx|js|jsx)$/.test(entry)) walk(path.join(exampleSrc, entry));
   }
 
-  return { packages, direct, filesWalked: seen.size, unresolvable };
+  return { packages, direct, filesWalked: seen.size, unresolvable, nonModuleSkipped };
 }
 
 describe('console-starter vite alias table', () => {
@@ -168,6 +233,10 @@ describe('console-starter vite alias table', () => {
   it('reaches the workspace graph it is meant to cover', () => {
     // Same guard for the walker: a resolution regression that walked nothing
     // would make the closure assertion trivially true.
+    //
+    // `unresolvable` now covers BOTH walk branches — bare `@object-ui/*` and
+    // relative — so a resolution regression is reported as itself, by name and
+    // importer, instead of only as a file count that drifted toward a floor.
     expect(closure.filesWalked).toBeGreaterThan(500);
     expect(closure.unresolvable).toEqual([]);
   });
@@ -197,3 +266,106 @@ describe('console-starter vite alias table', () => {
     expect(undeclared).toEqual([]);
   });
 });
+
+/**
+ * `closure.unresolvable` above is asserted empty, and on a healthy tree it IS
+ * empty — which is exactly the reading that cannot be trusted on its own. An
+ * assertion that receives nothing looks identical to one that cannot receive
+ * anything, and for the relative branch it *was* the second of those for three
+ * pull requests.
+ *
+ * So the empty reading is only worth what these fixtures are worth: the same
+ * walker, over a tree built on purpose, must report the misses that genuinely
+ * are misses and stay silent about the assets that are not. Pinning the class
+ * here means a future edit cannot re-blind the branch without turning this red.
+ */
+describe('the closure walker records relative misses instead of dropping them', () => {
+  /** Builds a throwaway source tree and walks it with the real `computeClosure`. */
+  function withFixture<T>(files: Record<string, string>, run: (dir: string) => T): T {
+    const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'alias-closure-')));
+    try {
+      for (const [name, content] of Object.entries(files)) {
+        const target = path.join(dir, name);
+        fs.mkdirSync(path.dirname(target), { recursive: true });
+        fs.writeFileSync(target, content);
+      }
+      return run(dir);
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  }
+
+  it('walks `./Foo.js` through to the `Foo.tsx` on disk', () => {
+    // The case that produced every row of objectui#5386's table: a package whose
+    // build preserves specifiers must spell `Foo.tsx` as `./Foo.js`, and the
+    // walk has to follow it rather than lose the subtree behind it.
+    const closure = withFixture(
+      {
+        'entry.ts': `import './Foo.js';\n`,
+        'Foo.tsx': `import './Bar';\nexport const Foo = 1;\n`,
+        'Bar.ts': `export const Bar = 2;\n`,
+      },
+      (dir) => computeClosure(dir),
+    );
+
+    // entry.ts + Foo.tsx + Bar.ts — the subtree behind the `.js` spelling is
+    // reached, not dropped at the first re-export.
+    expect(closure.filesWalked).toBe(3);
+    expect(closure.unresolvable).toEqual([]);
+  });
+
+  it('reports a relative module specifier that does not resolve', () => {
+    // The counter-probe for the empty reading above: plant misses of both
+    // module spellings and require this walker to name them.
+    const closure = withFixture(
+      {
+        'entry.ts': `import './missing-module.js';\nimport './gone';\n`,
+      },
+      (dir) => computeClosure(dir),
+    );
+
+    expect(closure.unresolvable).toHaveLength(2);
+    expect(closure.unresolvable.join('\n')).toContain('./missing-module.js');
+    expect(closure.unresolvable.join('\n')).toContain('./gone');
+  });
+
+  it('skips an unresolvable asset specifier explicitly rather than reporting it', () => {
+    // The boundary condition: a stylesheet, image or font import is not a
+    // missing module. `ts.preProcessFile` reports them all, and `resolveModule`
+    // cannot resolve any of them, so recording them alongside real misses would
+    // manufacture failures that are not defects.
+    const closure = withFixture(
+      {
+        'entry.ts':
+          `import './theme.css';\n` +
+          `import './logo.svg';\n` +
+          `import './font.woff2';\n` +
+          `import './data.json';\n` +
+          `import './raw.css?inline';\n` +
+          `import './real.js';\n`,
+        'real.ts': `export const real = 1;\n`,
+      },
+      (dir) => computeClosure(dir),
+    );
+
+    // None of the five assets is a defect...
+    expect(closure.unresolvable).toEqual([]);
+    // ...but each is accounted for, so the skip is a decision and not a drop.
+    expect(closure.nonModuleSkipped).toHaveLength(5);
+    expect(closure.nonModuleSkipped.join('\n')).toContain('./theme.css');
+    expect(closure.nonModuleSkipped.join('\n')).toContain('./raw.css?inline');
+    // The module alongside them still resolves and is walked.
+    expect(closure.filesWalked).toBe(2);
+  });
+
+  it('classifies the specifier spellings that decide the boundary', () => {
+    // Directly pins `isModuleSpecifier`, so the line stays where it is
+    // documented even if the walk around it is rewritten.
+    for (const module of ['./Foo', '.', '..', '../pkg/src', './Foo.js', './Foo.ts', './Foo.mts']) {
+      expect(isModuleSpecifier(module)).toBe(true);
+    }
+    for (const asset of ['./a.css', './a.svg', './a.json', './a.woff2', './a.js?worker']) {
+      expect(isModuleSpecifier(asset)).toBe(false);
+    }
+  });
+});