Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 77 additions & 9 deletions packages/cli/test/vitest-resolution-base-collapse.e2e.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,6 +5,37 @@
* specifier resolves from, and the anti-vacuity control written beside such a
* test is vacuous too. This file is the executable record of that platform fact.
*
* ── #11775 UPDATE: M1 is now switched OFF for `@objectstack/types` here ─────
*
* `packages/cli/vitest.config.ts` now carries
* `test.server.deps.external = [/packages[\/]types[\/]dist/]`, which takes this
* helper off Vite's inline path and hands its `import()` back to Node. The
* consequence for this file is that the two M1 cases below assert the OPPOSITE
* of what they asserted when it was written, and that inversion IS the fix
* landing — the in-process reading and the real-Node reading are now the same
* reading. The cases were not deleted, because "the anchor is restored" is only
* meaningful next to the real-Node baseline this file already measures.
*
* Two things that did NOT change, and both are load-bearing:
*
* - **M1 remains true as a platform fact.** The entry names ONE dist path in
* ONE package's config. Every other workspace dependency reached from these
* tests is still inlined and still has its base flattened, so a new
* in-process pin over a base claim still needs the treatment this file
* documents. The M1 block says so where it can be read at the point of use.
* - **M2 is untouched.** `NODE_PATH` reaching a spawned child, and CJS
* honouring it, is not a Vite mechanism and no vitest config can reach it.
*
* ⚠️ Why the entry cannot collide with the source aliases in the same config
* (the question #11412 left open as an assumed tension): `resolve.alias` acts in
* the RESOLVE phase, so an aliased specifier is already an absolute `src/…`
* path by the time the externalise predicate is asked — and a `/dist/` pattern
* cannot match a `src/` path. `check:test-source-alias` is what keeps that
* true rather than merely likely: it FAILS any alias whose winning entry does
* not land under `src/`. Measured on #11775 rather than argued: with a source
* alias and a `/service-cache[\/]dist/` external both present for the same
* package, a marker existing only in `src` was the one that loaded.
*
* It is a PRECONDITION pin in the sense `packages/types/src/node.test.ts` uses
* the word: it asserts what the runner does, not what this repo's code does, so
* that the day vitest/Vite changes the behaviour the repo is told — instead of
Expand DownExpand Up@@ -83,7 +114,7 @@ import { join } from 'node:path';
import { promisify } from 'node:util';
import { pathToFileURL } from 'node:url';
import { beforeAll, afterAll, describe, expect, it } from 'vitest';
import { createHostImporter } from '@objectstack/types/node';
import { createHostImporter, hostImportFailureKind } from '@objectstack/types/node';
import { childEnv } from './helpers/serve-process.js';

const execFileAsync = promisify(execFile);
Expand DownExpand Up@@ -191,27 +222,64 @@ describe('#11412 CONTROLS — the probe can return every answer it is asked to d
});
});

describe('#11412 M1 — vitest flattens the resolution base an in-process test would measure', () => {
describe('#11775 M1 — the `server.deps.external` entry restores the anchor for THIS package', () => {
it('REAL NODE: the no-base importer cannot reach a CLI-declared package, and names its base', async () => {
expect(clean.noBaseImporter).not.toBe('RESOLVED');
expect(clean.noBaseImporter).toContain(`Cannot find package '${CLI_DECLARED}'`);
// Named, not merely failed — this is the sentence the whole card is about.
expect(clean.noBaseImporter).toMatch(/imported from .*[/\\]packages[/\\]types[/\\]/);
});

it('UNDER VITEST: the identical call RESOLVES — so no in-process pin over it can fail', async () => {
await expect(createHostImporter(appRoot)(CLI_DECLARED)).resolves.toBeDefined();
it('UNDER VITEST: the identical call now REFUSES, with real Node’s base and envelope', async () => {
// THE PIN (#11775). Before the config entry this call RESOLVED, because the
// inlined package’s `import()` was rewritten to `__vite_ssr_dynamic_import__`
// and re-anchored at the vitest root. Externalising `packages/types/dist`
// hands the call back to Node, so the in-process reading and the real-Node
// reading above are now THE SAME READING.
//
// Asserted as an envelope, never as a bare `toThrow()`: an unfixed path that
// throws some other `Error` would satisfy `toThrow()` while failing the only
// property this file is about.
const err = await createHostImporter(appRoot)(CLI_DECLARED).then(
() => undefined,
(e: unknown) => e,
);
expect(err).toBeInstanceOf(Error);
expect((err as { code?: string }).code).toBe('MODULE_NOT_FOUND');
expect(hostImportFailureKind(err)).toBe('undeclared');
// The load-bearing half: WHICH base failed. `packages/types` is the module
// that physically contains the `import()`, i.e. the anchor Node uses and
// vitest used to replace.
expect((err as Error).message).toContain(`Cannot find package '${CLI_DECLARED}'`);
expect((err as Error).message).toMatch(/imported from .*[/\\]packages[/\\]types[/\\]/);
// Same sentence real Node produced in the child, now produced in-process.
expect(clean.noBaseImporter).toMatch(/imported from .*[/\\]packages[/\\]types[/\\]/);

// …and the same importer still refuses a specifier nothing satisfies, so the
// line above is the base collapsing, not "everything resolves in here".
// lines above are the anchor working, not "everything fails in here".
await expect(createHostImporter(appRoot)(NOWHERE)).rejects.toThrow(
`Cannot find package '${NOWHERE}'`,
);
});

it('the mechanism is Vite rewriting the inlined package’s dynamic import', () => {
// If this ever reads false, M1 is gone and the two cases above should be
// re-measured before anything is written on top of them.
expect(String(createHostImporter)).toContain('__vite_ssr_dynamic_import__');
it('the mechanism is the config entry — the helper is EXTERNAL, while this file is still inlined', () => {
// Absence-of-a-marker is a vacuous probe unless something proves the marker
// would have shown up. This file is itself an inlined module, so Vite
// rewrote the `import()` written right here — that is the positive control,
// and it is what makes the assertion below a measurement rather than a dead
// string search (a vitest upgrade that renamed the marker would fail HERE,
// loudly, instead of silently "confirming" the line under it).
const inlinedHere = async (s: string) => import(/* @vite-ignore */ s);
expect(String(inlinedHere)).toContain('__vite_ssr_dynamic_import__');

// The helper crossed the inline/external boundary; this file did not.
expect(String(createHostImporter)).not.toContain('__vite_ssr_dynamic_import__');

// ⚠️ M1 IS NOT GONE AS A PLATFORM FACT. It is switched off for exactly the
// paths matching `packages/types/dist` in THIS package’s vitest config
// (#11775) — every other workspace dependency here is still inlined and
// still has its resolution base flattened, and any new in-process pin over
// a base claim must still prove itself the way this file does.
});
});

Expand Down
58 changes: 57 additions & 1 deletion packages/cli/vitest.config.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -124,7 +124,56 @@
// The per-spawn cost is module-graph EXECUTION, not compilation.
//
// So the work is real, the price is fair, and nothing contained in this package
// removes it without changing what the e2e tests assert. Swapping the spawns to
// removes it without changing what the e2e tests assert.
//
// ## THE `test` BLOCK THAT NOW EXISTS, AND WHY IT IS NOT THE ONE REFUSED ABOVE
//
// #11775 added `test.server.deps.external`. Everything above still stands: the
// block deliberately sets NOTHING that has a default a test file can observe —
// no `globals`, no `environment`, no `pool`, no `isolate`, no `maxWorkers`. It
// is not a performance lever (the section above measured those and rejected
// them), and adding it changed no test's configuration except the resolution of
// one dependency.
//
// WHAT IT DOES. Vitest's default `server.deps.external` is `[/\/node_modules\//]`,
// evaluated against a module's REALPATH. A pnpm-linked workspace package's
// realpath is the package directory itself — `packages/types/dist/node.mjs` —
// which contains no `/node_modules/` segment, so every workspace dependency is
// INLINED, even one reached through `exports` to `dist/`. Vite then rewrites the
// `import()` written inside it to `__vite_ssr_dynamic_import__`, which resolves
// from the vitest root instead of from the module physically containing the
// call. Node ESM does the opposite: it anchors a bare specifier at the
// containing module. `@objectstack/types/node`'s `createHostImporter` EXISTS to
// resolve against a specific base, so under vitest it was measuring a base that
// had been flattened out from under it (#11412), and the entry below hands that
// call back to Node.
//
// ⚠️ THE PATTERN MUST MATCH THE REALPATH, AND A NAME-SHAPED ONE MATCHES NOTHING
// — SILENTLY. `/@objectstack[\/]types/` looks like the obvious spelling and is
// the trap: the realpath carries neither the package name nor `/node_modules/`,
// so that pattern matches zero modules and the experiment reads as
// "externalising does not help" rather than as "the pattern was wrong". #11775
// was first measured wrong for exactly that reason. Anything edited here needs a
// POSITIVE CONTROL that the pattern matches something — a pattern that matches
// nothing and a mechanism that does not work are indistinguishable from the
// outcome alone.
//
// ⚠️ IT DOES NOT FIGHT THE `resolve.alias` ENTRIES ABOVE, by construction.
// `resolve.alias` acts in the RESOLVE phase, so an aliased specifier is already
// an absolute `…/src/…` path before this predicate is consulted, and a `/dist/`
// pattern cannot match a `src/` path. That is not a coincidence to be preserved
// by care: `check:test-source-alias` FAILS any alias whose winning entry does
// not land under `src/`, so the gate that was assumed to be in tension with this
// entry is the same gate that keeps the two disjoint. Measured, not argued — see
// `test/vitest-resolution-base-collapse.e2e.test.ts`.
//
// COSTS, so the next person extending this list knows what they buy: an
// externalised package cannot be `vi.mock`ed and is not instrumented for
// coverage. Both were checked against this package when the entry landed —
// `packages/cli` has no `vi.mock` of `@objectstack/types` (its only mock targets
// are `../utils/optional-package.js`, `node:fs/promises` and
// `@objectstack/cloud-connection`) — but neither is free, and a package added
// here later must be re-checked for both. Swapping the spawns to
// the built entry would halve per-spawn boot and is exactly the source-vs-dist
// trade `scripts/check-test-source-alias.mjs` exists to refuse — see the note
// above on why a test that passes GREEN against a stale artifact is the
Expand DownExpand Up@@ -168,4 +217,11 @@ export default defineConfig({
},
],
},
test: {
server: {
deps: {
external: [/packages[\/]types[\/]dist/],
},
},
},
});
Loading