Skip to content

fix(utils): resolve bundle-inlined modules in importResolve #5971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
killagu merged 2 commits into from
Jun 20, 2026
+48 −1
Merged

fix(utils): resolve bundle-inlined modules in importResolve #5971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffc6286
fix(utils): resolve bundle-inlined modules in importResolve
Jun 19, 2026
b6724b3
test(utils): make importResolve disk-hit assertion cross-platform
killagu Jun 20, 2026
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
11 changes: 11 additions & 0 deletions packages/utils/src/import.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,17 @@ export function importResolve(filepath: string, options?: ImportResolveOptions):
}
}

// In bundle mode a module's source may not exist on disk (it is inlined into the
// bundle). After on-disk resolution has failed, if the registered bundle module
// loader recognizes the path, treat it as already resolved and return it as the
// canonical key, mirroring importModule. This must run before import.meta.resolve
// (which is unavailable in the bundled runtime).
const bundleModuleLoader = globalThis.__EGG_BUNDLE_MODULE_LOADER__;
if (bundleModuleLoader && bundleModuleLoader(normalizeBundleModulePath(filepath)) !== undefined) {
debug('[importResolve:bundle] %o => %o', filepath, filepath);
return filepath;
}
Comment on lines +381 to +385

@gemini-code-assist gemini-code-assist Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When resolving a relative path (e.g., ./foo) to a bundle-only module, the current implementation only checks the unresolved filepath against the bundle loader. Since the bundle loader typically indexes modules by their absolute canonical paths, this check will fail. We should resolve the relative path against the provided paths option and check those resolved absolute paths (along with common extensions) against the bundle loader to ensure correct resolution.

  const bundleModuleLoader = globalThis.__EGG_BUNDLE_MODULE_LOADER__;
  if (bundleModuleLoader) {
    const normalizedFilepath = normalizeBundleModulePath(filepath);
    if (bundleModuleLoader(normalizedFilepath) !== undefined) {
      debug('[importResolve:bundle] %o => %o', filepath, filepath);
      return filepath;
    }
    if (isRelativePath(filepath)) {
      for (const p of paths) {
        const resolvedPath = path.resolve(p, filepath);
        const normalizedResolved = normalizeBundleModulePath(resolvedPath);
        if (bundleModuleLoader(normalizedResolved) !== undefined) {
          debug('[importResolve:bundle:relative] %o => %o', filepath, resolvedPath);
          return resolvedPath;
        }
        for (const ext of [".js", ".ts", ".mjs", ".cjs", "/index.js", "/index.ts"]) {
          if (bundleModuleLoader(normalizedResolved + ext) !== undefined) {
            debug('[importResolve:bundle:relative] %o => %o', filepath, resolvedPath + ext);
            return resolvedPath + ext;
          }
        }
      }
    }
  }

@killagu killagu Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but I am intentionally not resolving relative paths here. This fallback is deliberately symmetric with importModule, which keys the loader on normalizeBundleModulePath(filepath) using the raw (unresolved) filepath. importResolve returns that same filepath as the canonical key precisely so a subsequent importModule(filepath) looks the loader up with an identical key.

Resolving ./foo to an absolute path before the lookup would make importResolve return a different key than the one importModule keys on, breaking that contract. The bundle loader itself owns the path namespace and decides what it recognizes; both functions hand it the same path-as-passed. So this is by design rather than an oversight, and I am leaving the relative-path test out for the same reason.


const extname = path.extname(filepath);
if ((!isAbsolute && extname === '.json') || !isESM) {
moduleFilePath = getRequire().resolve(filepath, {
Expand Down
38 changes: 37 additions & 1 deletion packages/utils/test/bundle-import.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import path from 'node:path';
import coffee from 'coffee';
import { afterEach, describe, it } from 'vitest';

import { importModule, setBundleModuleLoader } from '../src/import.ts';
import { importModule, importResolve, setBundleModuleLoader } from '../src/import.ts';
import { getFilepath } from './helper.ts';

describe('test/bundle-import.test.ts', () => {
Expand Down Expand Up @@ -135,4 +135,40 @@ describe('test/bundle-import.test.ts', () => {
const result = await importModule(filepath);
assert.deepEqual(result, fakeModule);
});

it('importResolve returns the path as canonical key for bundle-only modules', () => {
const seen: string[] = [];
setBundleModuleLoader((p) => {
seen.push(p);
return p === 'virtual/not-on-disk' ? { virtual: true } : undefined;
});

// The module is inlined into the bundle and has no source on disk, but the
// loader recognizes it, so importResolve hands it back unchanged.
assert.equal(importResolve('virtual/not-on-disk'), 'virtual/not-on-disk');
assert.ok(seen.includes('virtual/not-on-disk'));
});

it('importResolve normalizes Windows-style paths before the bundle lookup', () => {
setBundleModuleLoader((p) => (p === 'virtual/win/mod' ? { windows: true } : undefined));

const filepath = 'virtual/win/mod'.split(path.posix.sep).join(path.win32.sep);
assert.equal(importResolve(filepath), filepath);
});

it('importResolve does not consult the loader once on-disk resolution succeeds', () => {
let called = false;
setBundleModuleLoader(() => {
called = true;
return { hit: true };
});

const resolved = importResolve(getFilepath('esm'));
assert.match(resolved, /[\\/]fixtures[\\/]esm[\\/]index\.js$/);
assert.equal(called, false);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

it('importResolve still throws for missing modules when no loader is registered', () => {
assert.throws(() => importResolve('virtual/not-on-disk'));
});
Comment on lines +139 to +173

@gemini-code-assist gemini-code-assist Bot Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a test case to verify that importResolve can correctly resolve relative paths for bundle-only modules against the paths option.

  it('importResolve returns the path as canonical key for bundle-only modules', () => {
    const seen: string[] = [];
    setBundleModuleLoader((p) => {
      seen.push(p);
      return p === 'virtual/not-on-disk' ? { virtual: true } : undefined;
    });

    // The module is inlined into the bundle and has no source on disk, but the
    // loader recognizes it, so importResolve hands it back unchanged.
    assert.equal(importResolve('virtual/not-on-disk'), 'virtual/not-on-disk');
    assert.ok(seen.includes('virtual/not-on-disk'));
  });

  it('importResolve resolves relative paths for bundle-only modules against paths option', () => {
    const absolutePath = path.resolve(process.cwd(), 'virtual/relative/mod.js');
    const normalizedAbsolute = absolutePath.split(path.win32.sep).join(path.posix.sep);
    setBundleModuleLoader((p) => (p === normalizedAbsolute ? { relative: true } : undefined));

    assert.equal(importResolve('./relative/mod', { paths: [path.resolve(process.cwd(), 'virtual')] }), absolutePath);
  });

  it('importResolve normalizes Windows-style paths before the bundle lookup', () => {
    setBundleModuleLoader((p) => (p === 'virtual/win/mod' ? { windows: true } : undefined));

    const filepath = 'virtual/win/mod'.split(path.posix.sep).join(path.win32.sep);
    assert.equal(importResolve(filepath), filepath);
  });

  it('importResolve does not consult the loader once on-disk resolution succeeds', () => {
    let called = false;
    setBundleModuleLoader(() => {
      called = true;
      return { hit: true };
    });

    const resolved = importResolve(getFilepath('esm'));
    assert.ok(resolved.endsWith('/fixtures/esm/index.js'));
    assert.equal(called, false);
  });

  it('importResolve still throws for missing modules when no loader is registered', () => {
    assert.throws(() => importResolve('virtual/not-on-disk'));
  });

@killagu killagu Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skipping the relative-path test for the reason explained on the import.ts thread: the fallback is intentionally symmetric with importModule and keys the loader on the raw path-as-passed, so resolving relatives here would break the canonical-key contract. The other suggested cases are already covered by the existing tests.

});
Loading