Skip to content

Commit 46d34ab

Browse files
Elon Muskclaude
andauthored
fix(types): resolve the host importer's undeclared fallback from the caller, not from @objectstack/types (#11159)
* fix(types): resolve the host importer's undeclared fallback from the caller (#10943) `createHostImporter`'s docblock said the undeclared case "falls back to the importing package's own resolution". The fallback was a bare `import()` written inside `@objectstack/types`, and ESM resolves a bare specifier against the module containing the call — so it resolved from `@objectstack/types`, which under a pnpm-isolated layout sees only `@objectstack/spec`. A declared contract the implementation did not keep. Callers now hand in their own resolution base as `fallbackImport`. A string `parentURL` was measured and rejected in both spellings: `import.meta.resolve`'s parent argument is silently ignored unflagged (a phantom fix), and `createRequire(parentURL)` honours NODE_PATH, which is #4719's hole re-opened on the fallback path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r * chore(changeset): host importer resolves its undeclared fallback from the caller (#10943) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c74aefe commit 46d34ab

5 files changed

Lines changed: 383 additions & 20 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
'@objectstack/types': minor
3+
'@objectstack/verify': patch
4+
---
5+
6+
`createHostImporter`: resolve the undeclared fallback from the CALLER, not from `@objectstack/types`
7+
8+
The helper's documented contract said the undeclared case "falls back to the importing
9+
package's own resolution". It did not. The fallback was a bare `import()` written inside
10+
`@objectstack/types`, and Node ESM resolves a bare specifier against the module that
11+
CONTAINS the call — so it resolved from `@objectstack/types`, which under a pnpm-isolated
12+
layout can see only its own single dependency, `@objectstack/spec`. Measured from an app
13+
declaring nothing: `@objectstack/plugin-auth`, `@objectstack/plugin-audit` and `chalk` all
14+
resolve from `packages/cli` and all failed through the helper. Under a hoisted npm/yarn
15+
layout the same fallback usually does find the caller's dependencies, so the claim was
16+
green in some installs and absent in others.
17+
18+
`createHostImporter(hostRoot, options)` now takes the caller's resolution base as
19+
`options.fallbackImport` — the caller's own `import()`, written in the calling module:
20+
21+
```ts
22+
createHostImporter(hostRoot, { fallbackImport: (s) => import(s) })
23+
```
24+
25+
**minor, not patch, and not major.** New exported API (`HostImporterOptions`,
26+
`FallbackImport`, a second parameter) makes it additive rather than a fix-only patch. It
27+
is not a breaking change because the parameter is optional and omitting it keeps the
28+
previous resolution base exactly — an existing caller compiles and behaves as before. The
29+
`undeclared` failure text now names that retained default when a caller has not passed a
30+
base, so the gap reports itself instead of being rediscovered by measurement.
31+
32+
`@objectstack/verify` (patch) passes its own base from `bootStack`. Measured: this changes
33+
nothing for `@objectstack/organizations`, the only specifier it routes through the helper —
34+
that package is cloud-private and resolves from nowhere in the framework workspace. It is
35+
what stops the next app-supplied package added to that path from silently missing
36+
`packages/verify`'s own dependencies.
37+
38+
A string `parentURL` / `import.meta.url` base was measured on Node v22.22.2 and rejected in
39+
both spellings: `import.meta.resolve`'s parent argument is silently ignored without
40+
`--experimental-import-meta-resolve` (a change that would have compiled, run, and pinned
41+
green while ignoring the base), and `createRequire(parentURL)` is CJS resolution, which
42+
honours `NODE_PATH` — the hole the declaration gate exists to close, re-opened on the
43+
fallback path.

packages/qa/dogfood/test/enterprise-organizations.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,16 @@ export async function probeOrganizations(
9797
declared: boolean = process.env[MULTI_ORG_ENV] === '1',
9898
): Promise<OrganizationsProbe> {
9999
const root = hostRoot ?? process.cwd();
100-
const importFromHost = createHostImporter(root);
100+
// #10943: hand the helper THIS module's resolver. Its undeclared fallback is
101+
// documented as "the importing package's own resolution", and a bare
102+
// `import()` written inside `@objectstack/types` is that package's
103+
// resolution, not this one's — it can see only `@objectstack/spec`. Measured
104+
// to change nothing for `@objectstack/organizations` itself (cloud-private,
105+
// resolvable from nowhere in the framework workspace); it makes the
106+
// documented sentence true for this probe.
107+
const importFromHost = createHostImporter(root, {
108+
fallbackImport: (specifier) => import(/* webpackIgnore: true */ specifier),
109+
});
101110
try {
102111
await importFromHost(ORGANIZATIONS_PKG);
103112
return { available: true };

packages/types/src/node.test.ts

Lines changed: 183 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
3131
import * as NodeModule from 'node:module';
3232
import { tmpdir } from 'node:os';
3333
import { join } from 'node:path';
34+
import { pathToFileURL } from 'node:url';
3435
import {
3536
createHostImporter,
3637
createHostRequire,
@@ -176,12 +177,21 @@ describe('host-app package resolution (cloud#1013, #4700)', () => {
176177
});
177178

178179
it(
179-
"falls back to the importing package's own resolution when the host does not declare",
180+
'the DEFAULT fallback is this package\'s own resolution — @objectstack/spec and nothing else',
180181
async () => {
181-
// Whatever the host app does not declare must still load from the framework
182-
// package's own dependencies — that fallback is what keeps every
183-
// framework-owned load in `serve` (plugin-auth, plugin-security,
184-
// service-i18n, …) and in `bootStack` working exactly as before.
182+
// ⚠️ Read this case for what it measures, not for what it used to be
183+
// called (#10943). It was named "falls back to the importing package's
184+
// own resolution", which is the helper's DOCUMENTED contract — but it
185+
// passes because `@objectstack/spec` is the one dependency
186+
// `@objectstack/types` declares, so it is green whether the fallback
187+
// resolves from the caller or from here. It could never have failed on
188+
// the defect it appeared to guard, and it is why that defect survived to
189+
// be found by measurement instead.
190+
//
191+
// What it legitimately pins is the DEFAULT (no `fallbackImport`) base:
192+
// unchanged by #10943, so an out-of-tree caller keeps working. The
193+
// documented contract is pinned by the caller-anchored matrix below,
194+
// where every row can actually fail.
185195
const mod = await createHostImporter(undeclaringRoot)('@objectstack/spec');
186196
expect(mod).toBeTypeOf('object');
187197
},
@@ -397,3 +407,171 @@ describe('what counts as a declaration (#4719)', () => {
397407
}
398408
});
399409
});
410+
411+
/**
412+
* #10943 — the undeclared fallback resolves from the CALLER, not from
413+
* `@objectstack/types`.
414+
*
415+
* The card's own 4-row matrix, measured on `main` from an app declaring
416+
* nothing, is what these cases pin:
417+
*
418+
* via host importer bare import() from packages/cli
419+
* @objectstack/plugin-auth MODULE_NOT_FOUND OK
420+
* @objectstack/plugin-audit MODULE_NOT_FOUND OK
421+
* chalk MODULE_NOT_FOUND OK
422+
* @objectstack/spec OK —
423+
*
424+
* Rows 1-3 are packages the CALLER resolves and this package does not — the
425+
* helper's docblock promised them and delivered a `MODULE_NOT_FOUND`. Row 4 is
426+
* the single dependency `@objectstack/types` declares, and it is the row that
427+
* made the defect look absent: it resolves under EITHER base, so a pin written
428+
* on it alone is green in both worlds (the `@objectstack/spec` case above says
429+
* so in place, which is what it now legitimately guards).
430+
*
431+
* ── Why fixture packages and not the real four ──────────────────────────────
432+
*
433+
* The real four would make this test read `packages/cli`'s `node_modules`,
434+
* which is a cross-package input (`pnpm check:cross-package-test-inputs`) and
435+
* would hold the matrix hostage to whether those packages are BUILT — measured
436+
* here: with only this package's own closure built, `@objectstack/plugin-auth`
437+
* reads MODULE_NOT_FOUND from `packages/cli` too, for the unrelated reason that
438+
* its `dist/` does not exist yet. The fixtures carry the same four facts with
439+
* none of that: three packages only the caller can see (two scoped, one
440+
* unscoped — `chalk`'s shape), and `@objectstack/spec` itself for row 4.
441+
*
442+
* ── Row 4 is the row that catches a WIDENED implementation ──────────────────
443+
*
444+
* Rows 1-3 fail if the base does not move. Row 4 fails if the base moves but
445+
* the old one is kept ALONGSIDE it — a fallback that tried the caller and then
446+
* this package would satisfy rows 1-3 while quietly leaving every caller able
447+
* to reach `@objectstack/spec` without declaring it. That is a second de-facto
448+
* contract (Prime Directive #12), not the documented one.
449+
*
450+
* ⚠️ ORDER IS LOAD-BEARING WITHIN EACH ROW, and here is the measurement that
451+
* makes it so. Under this runner a dynamic `import()` inside the module under
452+
* test is cached by SPECIFIER, not by resolved URL: probed with one fixture
453+
* package and one name, the default base threw MODULE_NOT_FOUND before the
454+
* caller base had loaded it and RESOLVED the same name afterwards. So a
455+
* "the default base cannot see this" assertion is only true before anything has
456+
* loaded that name — which is why each row asserts its LEFT column first and
457+
* its RIGHT column second, in one case, on a name no other case touches. Split
458+
* a row into two cases, or reorder them, and the left column starts passing for
459+
* a reason that has nothing to do with this package.
460+
*/
461+
describe('the undeclared fallback resolves from the CALLER (#10943)', () => {
462+
/** Stand-ins for `plugin-auth` / `plugin-audit` / `chalk` — rows 1-3. */
463+
const CALLER_SCOPED_A = '@fixture/caller-scoped-a';
464+
const CALLER_SCOPED_B = '@fixture/caller-scoped-b';
465+
const CALLER_UNSCOPED = 'caller-unscoped';
466+
/**
467+
* Present in the caller fixture but NEVER loaded through any base — reserved
468+
* for the failure-text case, which needs a name the specifier cache has not
469+
* seen (see the ⚠️ above).
470+
*/
471+
const CALLER_NEVER_LOADED = '@fixture/caller-never-loaded';
472+
473+
/** A framework package that calls the helper, with its own `node_modules`. */
474+
let callerRoot: string;
475+
/** `(s) => import(s)` evaluated INSIDE `callerRoot` — a real caller base. */
476+
let fallbackImport: (specifier: string) => Promise<any>; // eslint-disable-line @typescript-eslint/no-explicit-any
477+
478+
beforeAll(async () => {
479+
callerRoot = mkdtempSync(join(tmpdir(), 'os-import-caller-pkg-'));
480+
writeFileSync(
481+
join(callerRoot, 'package.json'),
482+
JSON.stringify({ name: '@fixture/caller-pkg', version: '0.0.0-fixture', type: 'module' }),
483+
'utf8',
484+
);
485+
for (const name of [CALLER_SCOPED_A, CALLER_SCOPED_B, CALLER_UNSCOPED, CALLER_NEVER_LOADED]) {
486+
writeFixturePackage(callerRoot, name, `export const from = ${JSON.stringify(name)};\n`);
487+
}
488+
// The caller's own resolver, as a real module on disk at `callerRoot`. This
489+
// IS the fixture: `import()` written here resolves against THIS file,
490+
// exactly as `(s) => import(s)` written in `harness.ts` resolves against
491+
// `packages/verify`.
492+
writeFileSync(
493+
join(callerRoot, 'importer.mjs'),
494+
'export const fallbackImport = (s) => import(s);\nexport const here = import.meta.url;\n',
495+
'utf8',
496+
);
497+
const mod = await import(pathToFileURL(join(callerRoot, 'importer.mjs')).href);
498+
// PRECONDITION for every row below: the resolver really is anchored in the
499+
// fixture. Had the runner loaded that module through its own pipeline
500+
// instead of Node's, `import()` inside it would resolve from the runner's
501+
// root and this whole matrix would be measuring the wrong base.
502+
expect(mod.here).toBe(pathToFileURL(join(callerRoot, 'importer.mjs')).href);
503+
fallbackImport = mod.fallbackImport;
504+
});
505+
506+
afterAll(() => {
507+
if (callerRoot) rmSync(callerRoot, { recursive: true, force: true });
508+
});
509+
510+
// Rows 1-3. One case per row so each is independently sensitive: delete the
511+
// fix and all three go red on their own, naming their own package.
512+
for (const [row, pkg] of [
513+
[1, CALLER_SCOPED_A],
514+
[2, CALLER_SCOPED_B],
515+
[3, CALLER_UNSCOPED],
516+
] as const) {
517+
it(`row ${row}: ${pkg} — unreachable from here, loads through the caller's base`, async () => {
518+
// LEFT column, first and in this case (see the ⚠️ on ordering above):
519+
// the default base is this package's own resolution, and this package
520+
// declares only `@objectstack/spec`.
521+
const withoutBase = await createHostImporter(undeclaringRoot)(pkg).catch(
522+
(e: unknown) => e,
523+
);
524+
expect(hostImportFailureKind(withoutBase)).toBe('undeclared');
525+
526+
// RIGHT column: the same name, the same host app, the caller's base.
527+
const mod = await createHostImporter(undeclaringRoot, { fallbackImport })(pkg);
528+
expect(mod.from).toBe(pkg);
529+
});
530+
}
531+
532+
it("row 4: @objectstack/spec is not the caller's to resolve, and no longer leaks in", async () => {
533+
// The row that made the defect invisible, pointed the other way. The caller
534+
// fixture does not declare `@objectstack/spec`, so a fallback that is
535+
// genuinely the caller's cannot produce it — and one that ORs in this
536+
// package's own resolution still can. (Safe to assert after the default-base
537+
// case above loaded `@objectstack/spec`: that cache belongs to the module
538+
// under test, while this path runs entirely inside the fixture's own
539+
// resolver, which cannot see the package at all.)
540+
const err = await createHostImporter(undeclaringRoot, { fallbackImport })(
541+
'@objectstack/spec',
542+
).catch((e: unknown) => e);
543+
expect(hostImportFailureKind(err)).toBe('undeclared');
544+
expect((err as { code?: string }).code).toBe('MODULE_NOT_FOUND');
545+
});
546+
547+
it('the host DECLARATION still wins over the caller base (#4719 untouched)', async () => {
548+
// The fix moves one branch. The declared path must still resolve from the
549+
// host app, and an undeclared-but-NODE_PATH-reachable package must still be
550+
// refused: a caller base is not a way back into the hoisted store.
551+
const mod = await createHostImporter(hostRoot, { fallbackImport })(ORGANIZATIONS);
552+
expect(new mod.OrganizationsPlugin().name).toBe('com.objectstack.organizations');
553+
const err = await createHostImporter(undeclaringRoot, { fallbackImport })(
554+
HOISTED_ONLY,
555+
).catch((e: unknown) => e);
556+
expect(hostImportFailureKind(err)).toBe('undeclared');
557+
});
558+
559+
it('the undeclared failure NAMES a missing caller base instead of hiding it', async () => {
560+
// The pre-#10943 default is retained so an out-of-tree caller (cloud's
561+
// loader) cannot break under this parameter's arrival — so the one thing it
562+
// must not be is silent. A `MODULE_NOT_FOUND` naming no base is exactly what
563+
// let this defect survive being read.
564+
const withoutBase = await createHostImporter(undeclaringRoot)(CALLER_NEVER_LOADED).catch(
565+
(e: Error) => e,
566+
);
567+
expect(withoutBase.message).toMatch(/did not pass `fallbackImport`/);
568+
expect(withoutBase.message).toMatch(/@objectstack\/types/);
569+
570+
// And it must NOT appear when the caller did state its base — a false note
571+
// sends the next reader to a parameter that is already correct.
572+
const withBase = await createHostImporter(undeclaringRoot, { fallbackImport })(
573+
'@fixture/nowhere-at-all',
574+
).catch((e: Error) => e);
575+
expect(withBase.message).not.toMatch(/did not pass `fallbackImport`/);
576+
});
577+
});

0 commit comments

Comments
 (0)