bundler: bundle template-literal require()/import() via a __glob lookup map - #35680
Open
robobun wants to merge 9 commits into
Open
bundler: bundle template-literal require()/import() via a __glob lookup map#35680robobun wants to merge 9 commits into
robobun wants to merge 9 commits into
Claude / Claude Code Review
completed
Jul 25, 2026 in 40m 16s
Code review found 2 important issues
Found 4 candidates, confirmed 6. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 4 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/js_parser/p.rs:828-853 |
Rope EStrings truncated to first segment in shape extraction |
| 🔴 Important | src/js_parser/p.rs:869-882 |
Infinite recursion on self-referential const in append_dynamic_specifier_shape |
| 🟡 Nit | src/bundler/options.rs:108-113 |
Unconditional backslash→slash normalization corrupts POSIX filenames |
Annotations
Check failure on line 853 in src/js_parser/p.rs
claude / Claude Code Review
Rope EStrings truncated to first segment in shape extraction
The `EString` arm (and the `ETemplate` head/tail arms) call `s.string(self.arena)`, which for a rope `EString` returns only the first segment — it doesn't walk `.next`. Since `fold_string_addition` runs on `e_.args` before `try_glob_dynamic_require` (whenever `minify_syntax`/`inlining` is on, i.e. `--minify` or `target: bun`), `require("./bin" + "/" + arch + ".node")` folds to a rope and the extracted shape drops the `"/"`, yielding pattern `./bin*.node` instead of `./bin/*.node` — wrong files b
Check failure on line 882 in src/js_parser/p.rs
claude / Claude Code Review
Infinite recursion on self-referential const in append_dynamic_specifier_shape
The `EIdentifier` arm recurses into the stored initializer with no cycle guard, so a self-referential const like `const a = \`./x/${a}.js\`; require(a);` (syntactically valid, runtime TDZ error) sends `bun build` into unbounded recursion and a stack-overflow crash. Mutual references (`const a = \`./${b}\`; const b = \`./${a}\`; require(a);`) hit the same loop once both are inserted. Simplest fix: temporarily remove the ref from `glob_specifier_values` before recursing (re-insert after), or threa
Check warning on line 113 in src/bundler/options.rs
claude / Claude Code Review
Unconditional backslash→slash normalization corrupts POSIX filenames
The `\\` → `/` rewrite here is unconditional, but on POSIX `\\` is a legal filename byte, not a separator — a matched file literally named e.g. `weird\\name.js` gets its map key and import-record path corrupted to `./mods/weird/name.js`, so the runtime template value `./mods/weird\\name.js` misses the map. `write_sanitized_parent_dirs` in this same file already encodes the correct pattern (`b == b'/' || (cfg!(windows) && b == b'\\\\')`) with a comment explaining exactly this; gate the rewrite th
Loading