Skip to content

Commit 591ff87

Browse files
authored
fix(compile): resolve bare npm imports in --bundle worker sources (#34967)
deno compile --bundle follows new Worker(new URL(...)) references and pulls the referenced modules into the bundle graph. When the entrypoint lives in a subdirectory with its own package scope (for example a dist/ build output with its own package.json) and a worker is authored in a sibling source tree, a bare npm import from that worker source failed to resolve with "Import X not a dependency", even though the package is declared, installed, and resolves fine from the entrypoint's own tree in the same build. Plain deno bundle was unaffected because it does not follow those worker references into the source tree. The package is already present in the build's resolved npm snapshot, so this falls back to resolving such bare specifiers against that snapshot by package name, the way Node and Bun locate a package in a reachable node_modules. The fallback only runs after normal resolution has already failed, so successful resolutions and plain deno bundle behavior are unchanged. Closes #34937
1 parent fbd422c commit 591ff87

12 files changed

Lines changed: 184 additions & 0 deletions

File tree

cli/tools/bundle/mod.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,20 @@ fn browser_map_disabled_specifier(error: &BundleError) -> Option<String> {
13881388
Some(disabled.specifier.clone())
13891389
}
13901390

1391+
/// Whether a specifier looks like a bare (npm/node-style) specifier, i.e. not
1392+
/// a relative path, absolute path, package-internal `#` import, or a URL with a
1393+
/// scheme (`file:`, `data:`, `node:`, `npm:`, `jsr:`, `http(s):`, ...).
1394+
fn looks_like_bare_specifier(specifier: &str) -> bool {
1395+
!specifier.is_empty()
1396+
&& !specifier.starts_with('.')
1397+
&& !specifier.starts_with('/')
1398+
&& !specifier.starts_with('#')
1399+
// Load-bearing on Windows: `Url::parse("C:/foo")` succeeds (scheme `c`), so
1400+
// a drive-letter absolute path is correctly treated as not bare here. Don't
1401+
// "simplify" this away or Windows absolute paths regress into the fallback.
1402+
&& Url::parse(specifier).is_err()
1403+
}
1404+
13911405
fn maybe_ignorable_resolution_error(
13921406
error: &ResolveWithGraphError,
13931407
) -> Option<String> {
@@ -1517,6 +1531,26 @@ impl DenoPluginHandler {
15171531
Ok(specifier) => Ok(Some(file_path_or_url(specifier)?)),
15181532
Err(e) => {
15191533
log::debug!("{}: {:?}", deno_terminal::colors::red("error"), e);
1534+
// The graph may record an unmapped-bare-specifier error for a referrer
1535+
// pulled in from outside the entrypoint's package scope (e.g. a source
1536+
// file reached via a `new Worker(new URL(...))` reference next to a
1537+
// `dist/`-rooted entrypoint). The package is still in the build's npm
1538+
// snapshot, so resolve it by name the way Node/Bun do before giving up.
1539+
if looks_like_bare_specifier(path)
1540+
&& let Some(url) = resolver.resolve_bare_specifier_in_npm_snapshot(
1541+
path,
1542+
Some(&referrer),
1543+
import_kind_to_resolution_mode(kind),
1544+
NodeResolutionKind::Execution,
1545+
)
1546+
{
1547+
log::debug!(
1548+
"{}: resolved {} via npm snapshot fallback",
1549+
deno_terminal::colors::cyan("op_bundle_resolve"),
1550+
path
1551+
);
1552+
return Ok(Some(file_path_or_url(url)?));
1553+
}
15201554
if let Some(specifier) = maybe_ignorable_resolution_error(&e) {
15211555
log::debug!(
15221556
"{}: resolution failed, but maybe ignorable",

libs/resolver/graph.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,51 @@ impl<
401401
)
402402
}
403403

404+
/// Best-effort resolution of a bare specifier against the managed npm
405+
/// snapshot, regardless of whether the referrer's `package.json` declares
406+
/// it.
407+
///
408+
/// `deno compile --bundle` follows `new Worker(new URL(...))` and similar
409+
/// references into modules that can live outside the entrypoint's package
410+
/// scope (e.g. a worker authored in a sibling source tree, pulled in
411+
/// alongside a `dist/`-rooted entrypoint). A bare npm import from such a
412+
/// referrer can fail to map even though the package is installed and
413+
/// resolves fine elsewhere in the same build. This lets the bundler fall
414+
/// back to the snapshot by package name, matching Node/Bun's "find it in a
415+
/// reachable node_modules" behavior.
416+
///
417+
/// Known limitation: the specifier carries no version constraint, so this
418+
/// resolves `name@*`. If the build's snapshot holds more than one version
419+
/// of the package, the snapshot picks one by name alone, which may not be
420+
/// the version the referrer's own `node_modules` tree would select. Node
421+
/// and Bun resolve to the nearest reachable version; this does not. For the
422+
/// common single-version case the result is identical.
423+
///
424+
/// Returns `None` when npm resolution isn't managed or the package isn't
425+
/// present in the snapshot.
426+
pub fn resolve_bare_specifier_in_npm_snapshot(
427+
&self,
428+
raw_specifier: &str,
429+
maybe_referrer: Option<&Url>,
430+
resolution_mode: node_resolver::ResolutionMode,
431+
resolution_kind: node_resolver::NodeResolutionKind,
432+
) -> Option<Url> {
433+
let req_ref =
434+
NpmPackageReqReference::from_str(&format!("npm:{raw_specifier}")).ok()?;
435+
// Bail unless npm resolution is managed; `resolve_managed_npm_req_ref`
436+
// unwraps these and would otherwise panic.
437+
let node_and_npm_resolver = self.resolver.node_and_npm_resolver.as_ref()?;
438+
node_and_npm_resolver.npm_resolver.as_managed()?;
439+
self
440+
.resolve_managed_npm_req_ref(
441+
&req_ref,
442+
maybe_referrer,
443+
resolution_mode,
444+
resolution_kind,
445+
)
446+
.ok()
447+
}
448+
404449
pub fn resolve(
405450
&self,
406451
raw_specifier: &str,
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
{
2+
"tempDir": true,
3+
"steps": [{
4+
"cwd": "entry",
5+
"args": "install",
6+
"output": "[WILDCARD]"
7+
}, {
8+
"if": "unix",
9+
"cwd": "entry",
10+
"args": "compile --bundle --no-check --allow-all --output app main.ts",
11+
"output": "compile.out"
12+
}, {
13+
"if": "unix",
14+
"cwd": "entry",
15+
"commandName": "./app",
16+
"args": [],
17+
"output": "run.out",
18+
"exitCode": 0
19+
}, {
20+
"if": "windows",
21+
"cwd": "entry",
22+
"args": "compile --bundle --no-check --allow-all --output app.exe main.ts",
23+
"output": "compile.out"
24+
}, {
25+
"if": "windows",
26+
"cwd": "entry",
27+
"commandName": "./app.exe",
28+
"args": [],
29+
"output": "run.out",
30+
"exitCode": 0
31+
}, {
32+
"if": "unix",
33+
"cwd": "entry",
34+
"args": "compile --bundle --no-check --allow-all --output app_unknown main_unknown.ts",
35+
"output": "compile_unknown.out",
36+
"exitCode": 1
37+
}, {
38+
"if": "windows",
39+
"cwd": "entry",
40+
"args": "compile --bundle --no-check --allow-all --output app_unknown.exe main_unknown.ts",
41+
"output": "compile_unknown.out",
42+
"exitCode": 1
43+
}]
44+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[WILDCARD]Embedded Files[WILDCARD]
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[WILDCARD]error: Import "@denotest/this-package-does-not-exist" not a dependency
2+
at [WILDLINE]/outside/worker_unknown.ts:5:7
3+
error: bundling failed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"nodeModulesDir": "auto"
3+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// The entrypoint lives in `entry/` (its package scope), where the npm
2+
// dependency is declared and resolves normally. It spawns a worker authored in
3+
// a sibling `outside/` directory, which has no package scope of its own and
4+
// does not declare the dependency. `deno compile` follows the
5+
// `new Worker(new URL(...))` reference and pulls that source file into the
6+
// graph; its bare npm import must still resolve against the build's npm
7+
// snapshot. See https://github.com/denoland/deno/issues/34937.
8+
import { getValue, setValue } from "@denotest/esm-basic";
9+
10+
setValue(5);
11+
console.log("main", getValue());
12+
13+
const worker = new Worker(
14+
new URL("../outside/worker.ts", import.meta.url),
15+
{ type: "module" },
16+
);
17+
worker.onmessage = (e) => {
18+
console.log("main got:", e.data);
19+
worker.terminate();
20+
};
21+
worker.postMessage("ping");
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Entrypoint for the negative case. Spawns a worker whose bare npm import is
2+
// not present in the build's snapshot; compiling this must error.
3+
const worker = new Worker(
4+
new URL("../outside/worker_unknown.ts", import.meta.url),
5+
{ type: "module" },
6+
);
7+
worker.postMessage("ping");
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "entry-app",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"@denotest/esm-basic": "1.0.0"
6+
}
7+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { getValue, setValue } from "@denotest/esm-basic";
2+
3+
self.onmessage = (e) => {
4+
setValue(7);
5+
(self as unknown as Worker).postMessage(
6+
"pong-" + e.data + "-" + getValue(),
7+
);
8+
};

0 commit comments

Comments
 (0)