loader: document the empty --loader extension, accept it in bunfig too - #36904
elibarzilay wants to merge 1 commit into
Conversation
`--loader` takes an empty extension to map files that have none, but `--help` and the docs imply an extension is required, and bunfig's `[loader]` drops such a key instead of honoring it. Document it in `--help` (rust and zig copies) and in the run/bunfig docs pages, and accept an empty key in bunfig's `[loader]`, matching the CLI. In the zig parser the `continue` that skipped the key also left an uninitialized entry in the loader map, so a bunfig containing one sent extensionless imports through the `file` loader with no error.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
WalkthroughBun now accepts empty loader keys for files without extensions. The ChangesExtensionless loader mapping
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/run/run-extensionless.test.ts`:
- Around line 43-50: Update all five subprocess tests to capture
proc.stdout.text() and proc.exited concurrently, then assert the expected stdout
and verify that the resolved exitCode equals 0. Apply this consistently to each
Bun.spawn invocation in the extensionless subprocess test suite.
- Line 3: Remove tmpdirSync from the import statement at line 3 and ensure
tempDir is imported from harness. Replace all manual calls to tmpdirSync()
throughout the test file (including the locations around lines 40-41 and 69-70)
with the tempDir helper to align with the test harness coding guidelines that
require using tempDir for temporary directory creation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d7984e8c-6b62-4a56-b0cf-058413359e8d
📒 Files selected for processing (7)
docs/runtime/bunfig.mdxdocs/snippets/cli/run.mdxsrc/bunfig/bunfig.rssrc/runtime/cli/Arguments.rssrc/runtime/cli/Arguments.zigsrc/runtime/cli/bunfig.zigtest/cli/run/run-extensionless.test.ts
| import { describe, expect, test } from "bun:test"; | ||
| import { mkdirSync, writeFileSync } from "fs"; | ||
| import { bunEnv, bunExe, isWindows, tmpdirSync } from "harness"; | ||
| import { bunEnv, bunExe, isWindows, tempDirWithFiles, tmpdirSync } from "harness"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file =="
wc -l test/cli/run/run-extensionless.test.ts 2>/dev/null || true
sed -n '1,110p' test/cli/run/run-extensionless.test.ts 2>/dev/null || true
echo
echo "== harness exports for tempDir/tmpdirSync =="
rg -n "function tempDir|const tempDir|export .*tempDir|tmpdirSync" packages test -g '*.{ts,tsx,js,jsx,mjs,cjs}' | head -80
echo
echo "== imports/usages in target file =="
rg -n "tmpdirSync|tempDir|mkdirSync|exec|proc|exitCode|exited" test/cli/run/run-extensionless.test.tsRepository: oven-sh/bun
Length of output: 11122
Use tempDir for the new temporary directories.
The new tests call tmpdirSync() and create directories manually. Use the tempDir helper from harness instead.
Proposed change
-import { bunEnv, bunExe, isWindows, tempDirWithFiles, tmpdirSync } from "harness";
+import { bunEnv, bunExe, isWindows, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
- const dir = tmpdirSync();
- mkdirSync(dir, { recursive: true });
+ using dir = tempDir("run-extensionless-cli", {});
- const dir = tmpdirSync();
- mkdirSync(dir, { recursive: true });
+ using dir = tempDir("run-extensionless-import", {});As per coding guidelines, test files must use tempDir from harness and must not use tmpdirSync.
Also applies to: 40-41, 69-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/cli/run/run-extensionless.test.ts` at line 3, Remove tmpdirSync from the
import statement at line 3 and ensure tempDir is imported from harness. Replace
all manual calls to tmpdirSync() throughout the test file (including the
locations around lines 40-41 and 69-70) with the tempDir helper to align with
the test harness coding guidelines that require using tempDir for temporary
directory creation.
Source: Coding guidelines
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "--loader=:ts", join(dir, "./cool")], | ||
| cwd: dir, | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| }); | ||
| const stdout = await proc.stdout.text(); | ||
| expect(stdout).toEqual("hello world\n"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '^test/cli/run/run-extensionless\.test\.ts$' || true
echo "== file excerpt =="
if [ -f test/cli/run/run-extensionless.test.ts ]; then
nl -ba test/cli/run/run-extensionless.test.ts | sed -n '1,120p'
fi
echo "== subprocess tests in file =="
rg -n 'Bun\.spawn|await proc\.exited|exitCode|stdout\.text|expect\(stdout\)' test/cli/run/run-extensionless.test.ts || true
echo "== git status/stat (context only) =="
git status --short || true
git diff --stat || trueRepository: oven-sh/bun
Length of output: 265
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== file excerpt =="
if [ -f test/cli/run/run-extensionless.test.ts ]; then
awk '{ printf "%5d %s\n", NR, $0 }' test/cli/run/run-extensionless.test.ts | sed -n '1,120p'
fi
echo "== subprocess patterns in file =="
grep -nE 'Bun\.spawn|await proc\.exited|exitCode|stdout\.text|expect\(stdout\)' test/cli/run/run-extensionless.test.ts || trueRepository: oven-sh/bun
Length of output: 4565
Assert the subprocess exit code in the new subprocess tests.
Each test reads only proc.stdout. Capture proc.stdout.text() and proc.exited concurrently, assert the output, then assert exitCode is 0. Apply this to all five subprocess tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/cli/run/run-extensionless.test.ts` around lines 43 - 50, Update all five
subprocess tests to capture proc.stdout.text() and proc.exited concurrently,
then assert the expected stdout and verify that the resolved exitCode equals 0.
Apply this consistently to each Bun.spawn invocation in the extensionless
subprocess test suite.
Source: Coding guidelines
…ntry point `--loader :ts` and bunfig `"" = "ts"` must reach a file with no extension that is run directly, not only one that is imported. `<T>(x: T) => x` parses under the ts loader and not under tsx, so the test tells the two apart. These are the entry-point tests from #36904. Co-authored-by: Eli Barzilay <eli@barzilay.org>
|
Thank you for this, and for the clear write-up. The note on how the silent bunfig drop hid the feature was useful. Since this was opened, the CLI and bunfig code finished moving from Zig to Rust.
You are credited as co-author on the commits that carry your changes (0f24262 and 5634b44). The uninitialized map entry you found in the Zig parser does not exist in the Rust one, which pushes to a Closing this as superseded by #41996. If anything from here is missing there, please comment and I will pick it up. |
2 participants
Mostly a documentation gap.
--loaderalready accepts an empty extension, mapping files that have none —which matters because Bun reads extensionless files with the
tsxloader, wherea generic arrow (
<T>(x: T) => x) is an unclosed tag.colon_list_typedeliberately allows the empty key, but
--helpand the docs both imply anextension is required, so the capability is undiscoverable and untested.
Briefly, how it threw me off: I concluded the mapping didn't exist at all — the
flag docs implied it, and the bunfig spelling failed silently rather than saying
anything.
Included here:
--help(both copies) and thebun run/ bunfig docs pages mention the emptyextension
[loader]accepts an empty key, matching the CLI. The zig parser'scontinuefor that key also left an uninitialized entry in the loader map, soa bunfig containing one sent extensionless imports through the
fileloaderRepro of the bunfig behavior on 1.3.14:
I haven't built Bun locally: the four
--loadertests pass against a released1.3.14, while the bunfig test needs the parser change compiled, so CI is its
first real run.
The zig → rust migration is clearly in flight — this touches both copies of the
parser and of the help text. Please take it as a sketch of one way it could land
rather than a finished patch; happy to reshape it to whatever fits the
transition.