Repository navigation
Conversation
|
Updated 3:05 AM PT - Dec 1st, 2025
❌ @RiskyMH, your commit a6ecf14 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 21459That installs a local version of the PR into your bun-21459 --bun |
|
Vite has Also |
Properly integrated import.meta.glob support into new parser structure: - Added handleImportMetaGlobCall to P.zig - Added import_meta_glob case handling in visitExpr.zig - Added glob detection for import.meta.glob in maybe.zig - Updated SideEffects.zig to handle import_meta_glob
| /** | ||
| * Import attributes to pass to the import statement. | ||
| * This is the standard way to specify import options. | ||
| * | ||
| * @example | ||
| * const modules = import.meta.glob('./assets/*.txt', { with: { type: 'text' } }) | ||
| * | ||
| * // code produced by bun | ||
| * const modules = { | ||
| * './assets/file.txt': () => import('./assets/file.txt', { with: { type: 'text' } }), | ||
| * } | ||
| */ | ||
| with?: ImportAttributes; |
There was a problem hiding this comment.
vite doesn't have this one, i just thought it was nice because often you want the text content or path (like when using the dynamic import normally)
| // todo: | ||
| // /** | ||
| // * If true, imports all modules eagerly (synchronously). | ||
| // * If false (default), returns functions that import modules lazily. | ||
| // */ | ||
| // eager?: Eager; | ||
| eager?: false; |
There was a problem hiding this comment.
I do want to do this, but its way more change so for a different pr. Personally it makes more sense to use a different way for eger instead like in the top level import itself:
import files from "./src/*.ts";
alii
left a comment
alii
left a comment
There was a problem hiding this comment.
Just a small change for the type definitions
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/bun-types/globals.d.ts (1)
1307-1316: Remove genericEageroverload from import.meta.glob (type unsoundness)The generic Eager lets callers force eager typing (e.g.
import.meta.glob<true, T>) while runtime still returns lazy loader functions. Remove the generic overload so eager can only be selected via runtime options.File: packages/bun-types/globals.d.ts (around lines 1307–1316)
- glob<Eager extends boolean = false, TModule = unknown>( - pattern: string | string[], - options?: ImportMetaGlobOptions<Eager>, - ): Eager extends true ? Record<string, TModule> : Record<string, () => Promise<TModule>>; glob<TModule = unknown>( pattern: string | string[], options?: ImportMetaGlobOptions<false>, ): Record<string, () => Promise<TModule>>; - glob<TModule = unknown>(pattern: string | string[], options?: ImportMetaGlobOptions<true>): Record<string, TModule>; + // TODO: add an overload for eager: true once implementedRepo-wide search for
import.meta.glob<truereturned 0 matches.
🧹 Nitpick comments (1)
packages/bun-types/globals.d.ts (1)
1324-1349: Minor: remove unused generic fromImportMetaGlobOptions
ImportMetaGlobOptions<Eager>no longer needs theEagerparameter if you drop the generic function overload. Simplify:-interface ImportMetaGlobOptions<Eager extends boolean = false> { +interface ImportMetaGlobOptions { // eager not implemented yet eager?: false; … }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/api/import-meta.md(2 hunks)packages/bun-types/globals.d.ts(1 hunks)test/integration/bun-types/fixture/import-meta.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/api/import-meta.md
🧰 Additional context used
📓 Path-based instructions (4)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/integration/bun-types/fixture/import-meta.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/integration/bun-types/fixture/import-meta.ts
test/integration/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place integration tests under
test/integration/
Files:
test/integration/bun-types/fixture/import-meta.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/integration/bun-types/fixture/import-meta.tspackages/bun-types/globals.d.ts
🧠 Learnings (15)
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/html.test.ts : html.test.ts should contain tests relating to HTML files themselves
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*-fixture.ts : Name test fixture files that are spawned by tests with the suffix `-fixture.ts`
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode
Applied to files:
test/integration/bun-types/fixture/import-meta.tspackages/bun-types/globals.d.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Use shared utilities from test/harness.ts where applicable
Applied to files:
test/integration/bun-types/fixture/import-meta.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Applied to files:
test/integration/bun-types/fixture/import-meta.tspackages/bun-types/globals.d.ts
📚 Learning: 2025-09-07T05:41:52.563Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{js,ts} : Author modules as CommonJS-style with `require(...)` and export via `export default {}` (no ESM `import`/named exports)
Applied to files:
packages/bun-types/globals.d.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/ecosystem.test.ts : ecosystem.test.ts should focus on concrete library integration bugs rather than whole-package coverage
Applied to files:
packages/bun-types/globals.d.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (3)
test/integration/bun-types/fixture/import-meta.ts (2)
1-35: LGTM overall: good coverage of patterns and typing ergonomicsCovers arrays of patterns,
with.type,base, explicitTModule, and negative tests for eager. Nice.
3-7: Make runtime loop conditional to keep this file a pure typing fixtureIf this file is only for type-checking, avoid top-level await that performs real imports; wrap the loop in a never-executed guard:
-const fixtures = import.meta.glob("./**/*.ts"); -for (const [file, importFn] of Object.entries(fixtures)) { - console.log(file, await importFn()); -} +const fixtures = import.meta.glob("./**/*.ts"); +if (false) { + for (const [file, importFn] of Object.entries(fixtures)) { + console.log(file, await importFn()); + } +}Confirm whether this fixture is executed by any test runner.
packages/bun-types/globals.d.ts (1)
1387-1387: Avoid shadowing TS lib'sImportAttributes— prefer TS lib type or a Bun‑scoped aliasGlobal
interface ImportAttributescan merge with TypeScript's lib and cause conflicts. I couldn't verifyImportCallOptionsin the environment — confirm whether the TS lib provides it and, if so, preferImportCallOptions["with"]; otherwise apply the permissive Bun‑scoped variant below.- with?: ImportAttributes; + // Use TS lib type if available; otherwise a permissive record with a typed `type` field. + with?: ({ type?: "css" | "file" | "json" | "jsonc" | "toml" | "yaml" | "txt" | "text" | "html" | (string & {}) } & Record<string, string>);And delete the global interface:
-interface ImportAttributes { - type: "css" | "file" | "json" | "jsonc" | "toml" | "yaml" | "txt" | "text" | "html" | (string & {}); -}Also applies to: 1403-1405.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/bun-types/globals.d.ts(1 hunks)src/ast/E.zig(1 hunks)src/ast/P.zig(2 hunks)src/ast/SideEffects.zig(1 hunks)src/ast/visitExpr.zig(1 hunks)src/bun.js/RuntimeTranspilerCache.zig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bun.js/RuntimeTranspilerCache.zigsrc/ast/SideEffects.zigsrc/ast/visitExpr.zigsrc/ast/E.zigsrc/ast/P.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/RuntimeTranspilerCache.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bunWhen adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
Files:
src/bun.js/RuntimeTranspilerCache.zigsrc/ast/SideEffects.zigsrc/ast/visitExpr.zigsrc/ast/E.zigsrc/ast/P.zig
🧠 Learnings (1)
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
PR: oven-sh/bun#22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/ast/P.zig
| var path_buf: bun.PathBuffer = undefined; | ||
| const slash_normalized = if (comptime bun.Environment.isWindows) | ||
| strings.normalizeSlashesOnly(&path_buf, path, '/') | ||
| else | ||
| path; | ||
|
|
||
| const duped = bun.handleOom(p.allocator.dupe(u8, slash_normalized)); | ||
| bun.handleOom(matched_files.append(duped)); | ||
| } | ||
| } | ||
|
|
||
| std.sort.block([]const u8, matched_files.items, {}, struct { | ||
| fn lessThan(_: void, a: []const u8, b_path: []const u8) bool { | ||
| return strings.order(a, b_path) == .lt; | ||
| } | ||
| }.lessThan); | ||
|
|
||
| // Create dynamic imports | ||
| var properties: []G.Property = bun.handleOom(p.allocator.alloc(G.Property, matched_files.items.len)); | ||
|
|
||
| for (matched_files.items, 0..) |file_path, i| { | ||
| // add the base path and/or query string to the import path | ||
| const import_path: []const u8 = if (base_path) |base| | ||
| if (query) |q| | ||
| bun.handleOom(std.fmt.allocPrint(p.allocator, "{s}/{s}{s}", .{ base, file_path, q })) | ||
| else | ||
| bun.handleOom(std.fmt.allocPrint(p.allocator, "{s}/{s}", .{ base, file_path })) | ||
| else if (query) |q| | ||
| bun.handleOom(std.fmt.allocPrint(p.allocator, "{s}{s}", .{ file_path, q })) | ||
| else | ||
| file_path; |
There was a problem hiding this comment.
Return relative module specifiers, not absolute paths
matched_files.append(duped) stores the walker’s absolute path. The keys we emit (and the string we pass to addImportRecord) therefore become absolute filesystem paths, while Vite’s import.meta.glob semantics require module specifiers relative to the importer (e.g. ./modules/foo.js). The same regression means base/query are concatenated with a full absolute path, producing nonsensical specifiers and broken bundler resolution.
Please compute the path relative to search_dir (trim the shared prefix with the proper separator guard) before appending it, then use that relative slice both for the object key and as the import string (prefix .//../ as needed). A safe pattern is:
- const duped = bun.handleOom(p.allocator.dupe(u8, slash_normalized));
- bun.handleOom(matched_files.append(duped));
+ const rel_path = blk: {
+ if (strings.hasPrefix(slash_normalized, search_dir) and slash_normalized.len >= search_dir.len) {
+ var start = search_dir.len;
+ if (start < slash_normalized.len and (slash_normalized[start] == '/' or slash_normalized[start] == '\\')) {
+ start += 1;
+ }
+ break :blk slash_normalized[start..];
+ }
+ break :blk slash_normalized;
+ };
+ const specifier = if (rel_path.len == 0 or rel_path[0] == '.' or rel_path[0] == '/')
+ rel_path
+ else
+ bun.handleOom(std.fmt.allocPrint(p.allocator, "./{s}", .{rel_path}));
+ const duped = bun.handleOom(p.allocator.dupe(u8, specifier));
+ bun.handleOom(matched_files.append(duped));Then use that specifier when building import_path (prepending base_path/query afterwards). This restores the expected module specifiers and keeps us aligned with Vite.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var path_buf: bun.PathBuffer = undefined; | |
| const slash_normalized = if (comptime bun.Environment.isWindows) | |
| strings.normalizeSlashesOnly(&path_buf, path, '/') | |
| else | |
| path; | |
| const duped = bun.handleOom(p.allocator.dupe(u8, slash_normalized)); | |
| bun.handleOom(matched_files.append(duped)); | |
| } | |
| } | |
| std.sort.block([]const u8, matched_files.items, {}, struct { | |
| fn lessThan(_: void, a: []const u8, b_path: []const u8) bool { | |
| return strings.order(a, b_path) == .lt; | |
| } | |
| }.lessThan); | |
| // Create dynamic imports | |
| var properties: []G.Property = bun.handleOom(p.allocator.alloc(G.Property, matched_files.items.len)); | |
| for (matched_files.items, 0..) |file_path, i| { | |
| // add the base path and/or query string to the import path | |
| const import_path: []const u8 = if (base_path) |base| | |
| if (query) |q| | |
| bun.handleOom(std.fmt.allocPrint(p.allocator, "{s}/{s}{s}", .{ base, file_path, q })) | |
| else | |
| bun.handleOom(std.fmt.allocPrint(p.allocator, "{s}/{s}", .{ base, file_path })) | |
| else if (query) |q| | |
| bun.handleOom(std.fmt.allocPrint(p.allocator, "{s}{s}", .{ file_path, q })) | |
| else | |
| file_path; | |
| var path_buf: bun.PathBuffer = undefined; | |
| const slash_normalized = if (comptime bun.Environment.isWindows) | |
| strings.normalizeSlashesOnly(&path_buf, path, '/') | |
| else | |
| path; | |
| // Compute a module‐specifier relative to search_dir | |
| const rel_path = blk: { | |
| if (strings.hasPrefix(slash_normalized, search_dir) and slash_normalized.len >= search_dir.len) { | |
| var start = search_dir.len; | |
| if (start < slash_normalized.len and | |
| (slash_normalized[start] == '/' or slash_normalized[start] == '\\')) | |
| { | |
| start += 1; | |
| } | |
| break :blk slash_normalized[start..]; | |
| } | |
| break :blk slash_normalized; | |
| }; | |
| const specifier = if (rel_path.len == 0 or rel_path[0] == '.' or rel_path[0] == '/') | |
| rel_path | |
| else | |
| bun.handleOom(std.fmt.allocPrint(p.allocator, "./{s}", .{rel_path})); | |
| const duped = bun.handleOom(p.allocator.dupe(u8, specifier)); | |
| bun.handleOom(matched_files.append(duped)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/ast/P.zig around lines 6129 to 6159, the code currently appends the
walker’s absolute path into matched_files causing emitted keys and import
strings to be absolute; compute a path specifier relative to search_dir instead:
trim the common prefix between the absolute file path and search_dir ensuring
you only cut at a path separator boundary (handle Windows vs POSIX separators),
produce a relative slice and if it does not start with "." or ".." prefix it
with "./", then append that relative specifier (not the absolute path) to
matched_files; when building import_path use this specifier and then prepend
base_path and append query as needed so the object keys and import strings are
proper module specifiers for Vite.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/bundler/bundle_v2.zig(1 hunks)test/no-validate-exceptions.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bundler/bundle_v2.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bunWhen adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
Files:
src/bundler/bundle_v2.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/no-validate-exceptions.txt
🧠 Learnings (11)
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
PR: oven-sh/bun#22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/bundler/bundle_v2.zig
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/node/test/** : Vendored Node.js tests under test/js/node/test/ are exceptions and do not follow Bun’s test style
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Do not set explicit test timeouts; Bun already has timeouts
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Import common utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, platform helpers, GC helpers)
Applied to files:
test/no-validate-exceptions.txt
📚 Learning: 2025-10-12T02:22:34.349Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.349Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/no-validate-exceptions.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
| if (this.graph.ast.len > resolve.import_record.importer_source_index) { | ||
| const record: *ImportRecord = &this.graph.ast.items(.import_records)[resolve.import_record.importer_source_index].slice()[resolve.import_record.import_record_index]; | ||
| if (record.loader) |override_loader| { | ||
| break :blk override_loader; | ||
| } | ||
| } | ||
| break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file; |
There was a problem hiding this comment.
Still missing the import_record_index bounds check
We now guard importer_source_index, but import_record_index can still point past the slice when it comes from plugins or stale resolve data. That will read beyond the list and crash. Please add the per‐slice length check and fall back to the default loader when it’s out of range.
- const loader = blk: {
- if (this.graph.ast.len > resolve.import_record.importer_source_index) {
- const record: *ImportRecord = &this.graph.ast.items(.import_records)[resolve.import_record.importer_source_index].slice()[resolve.import_record.import_record_index];
- if (record.loader) |override_loader| {
- break :blk override_loader;
- }
- }
- break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file;
- };
+ const loader = blk: {
+ const all_import_records = this.graph.ast.items(.import_records);
+ if (resolve.import_record.importer_source_index >= all_import_records.len)
+ break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file;
+ const list = all_import_records[resolve.import_record.importer_source_index].slice();
+ if (resolve.import_record.import_record_index >= list.len)
+ break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file;
+ const record: *ImportRecord = &list[resolve.import_record.import_record_index];
+ if (record.loader) |override_loader| break :blk override_loader;
+ break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file;
+ };🤖 Prompt for AI Agents
In src/bundler/bundle_v2.zig around lines 2468 to 2474, the code checks
importer_source_index but does not verify import_record_index against the length
of the slice, which can lead to out-of-bounds access; update the block to get
the slice for .import_records at importer_source_index, check that
resolve.import_record.import_record_index < slice.len before indexing, and only
use record.loader when that bounds check passes; if the index is out of range,
fall back to the default loader path.loader(&this.transpiler.options.loaders)
orelse options.Loader.file.
| // todo: | ||
| // /** | ||
| // * If true, imports all modules eagerly (synchronously). | ||
| // * If false (default), returns functions that import modules lazily. | ||
| // * | ||
| // * @example | ||
| // * const eager = import.meta.glob('./src/*.ts', { eager: true }) | ||
| // * const normal = import.meta.glob('./src/*.ts', { eager: false }) | ||
| // * | ||
| // * // code produced by bun | ||
| // * import * as __modules_foo from './src/foo.ts' | ||
| // * import * as __modules_bar from './src/bar.ts' | ||
| // * const eager = { | ||
| // * './src/foo.ts': __modules_foo, | ||
| // * './src/bar.ts': __modules_bar, | ||
| // * } | ||
| // * | ||
| // * const normal = { | ||
| // * './src/foo.ts': () => import('./src/foo.ts'), | ||
| // * './src/bar.ts': () => import('./src/bar.ts'), | ||
| // * } | ||
| // */ | ||
| // eager?: Eager; | ||
| eager?: false; |
There was a problem hiding this comment.
Docs could use a little bit of love, imo.
Also, why expose this if it is not currently supported?
There was a problem hiding this comment.
Realized I didn't leave a very helpful message when I said they could use a little bit more love. What I'm specifically referring to is:
- the docs could offer an example that is a "birds-eye view" of the function, and does not assume that the user has a mental model of "code transformation" if you will
- perhaps the docs could explain what "eager" loading means in terms of how it impacts the execution of the module
- perhaps you might want to remove the
(synchronously)since I think it just aliases the wordeagerlyand doesn't necessarily shed much light on the concept (imo)
My opinions -- feel free to disagree
There was a problem hiding this comment.
I mean, the docs are basically written the same way as vite. But yeah dynamic imports may be confusing. I dont really know of a better way to describe the limbo land of not proper import but a function that returns promise of imported file.
| ```ts | ||
| const modules = import.meta.glob("./modules/*.ts"); | ||
| // const modules = { | ||
| // './modules/a.ts': () => import('./modules/a.ts'), | ||
| // './modules/b.ts': () => import('./modules/b.st'), | ||
| // } | ||
| ``` |
There was a problem hiding this comment.
const modules = import.meta.glob("./modules/*.ts");resolves to:
const modules = {
'./modules/a.ts': () => import('./modules/a.ts'),
'./modules/b.ts': () => import('./modules/b.st'),
}Maybe? Idk, no strong opinions
There was a problem hiding this comment.
You might also want to talk a little bit about vite compatibility:
https://vite.dev/guide/features
Do we have support for multiple patterns, negative patterns, etc?
There was a problem hiding this comment.
Oh oops I forgot about negative patterns, but yeah it supports the array of patterns. The issue is i dont know at what point the docs get bloated (and if nextjs and others would be adding this then could get weird)
Yeah it has those, its just eager that isn't supported here.
Regarding documenting that, i'm not sure how to do it here but still keep things simple to understand and all (+consistent to rest of this page).
| * const module = await modules['./src/foo.ts']() | ||
| */ | ||
| glob<Eager extends boolean = false, TModule = unknown>( | ||
| pattern: string | string[], |
There was a problem hiding this comment.
It seems like we support multiple patterns, but it's not 100% clear how to use that, perhaps some docs would be neat?
Support listSupported
Future implementation
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundler/bundle_v2.zig (1)
661-667: Critical: Add defensive bounds checks before reading ImportRecordThe code at line 662 indexes into import records using
import_record.importer_source_indexandimport_record.import_record_indexwithout any bounds checking. These indices come from resolve input and can be stale or invalid, leading to out-of-bounds access and crashes.Based on learnings: prefer bounds checking over
isValid()checks when accessing import records.Apply this diff to add defensive bounds checks:
const loader: Loader = brk: { - const record: *ImportRecord = &this.graph.ast.items(.import_records)[import_record.importer_source_index].slice()[import_record.import_record_index]; - if (record.loader) |out_loader| { - break :brk out_loader; - } - break :brk path.loader(&transpiler.options.loaders) orelse options.Loader.file; + const all_import_records = this.graph.ast.items(.import_records); + if (import_record.importer_source_index >= all_import_records.len) + break :brk path.loader(&transpiler.options.loaders) orelse options.Loader.file; + const list = all_import_records[import_record.importer_source_index].slice(); + if (import_record.import_record_index >= list.len) + break :brk path.loader(&transpiler.options.loaders) orelse options.Loader.file; + const record: *ImportRecord = &list[import_record.import_record_index]; + if (record.loader) |out_loader| break :brk out_loader; + break :brk path.loader(&transpiler.options.loaders) orelse options.Loader.file; };
♻️ Duplicate comments (4)
src/ast/P.zig (1)
6139-6240: Return relative specifiers instead of absolute paths.
BunGlobWalkeryields absolute filesystem paths, but you append them directly intomatched_filesand then reuse those slices for the object keys and the dynamic import strings. That turns everyimport.meta.glob()entry into/abs/path/to/file.js, which breaks Vite semantics (keys must be module specifiers relative to the importer such as./modules/foo.js) and produces nonsensical strings oncebase/queryare concatenated. This regresses the exact bug called out in the earlier review at Lines 6139‑6169. Fix by trimming thesearch_dirprefix (guard the separator) to obtain a relative specifier, prefix with./when needed, and use that normalized slice both for the object key andaddImportRecordinput (applybase/queryafterwards).Apply:
- const duped = bun.handleOom(p.allocator.dupe(u8, slash_normalized)); - bun.handleOom(matched_files.append(duped)); + const rel_path = blk: { + if (strings.hasPrefix(slash_normalized, search_dir) and slash_normalized.len >= search_dir.len) { + var start = search_dir.len; + if (start < slash_normalized.len and (slash_normalized[start] == '/' or slash_normalized[start] == '\\')) { + start += 1; + } + break :blk slash_normalized[start..]; + } + break :blk slash_normalized; + }; + const specifier = if (rel_path.len == 0 or rel_path[0] == '.' or rel_path[0] == '/') + bun.handleOom(p.allocator.dupe(u8, rel_path)) + else + bun.handleOom(std.fmt.allocPrint(p.allocator, "./{s}", .{rel_path})); + bun.handleOom(matched_files.append(specifier));src/bundler/bundle_v2.zig (1)
2467-2473: Still missing the import_record_index bounds checkThe code checks
importer_source_indexat line 2467 but does not verify thatimport_record_indexis within the slice bounds before indexing at line 2468. Plugin-provided indices can be stale or invalid, leading to out-of-bounds access and crashes.Based on learnings: prefer bounds checking over
isValid()checks when accessing import records.Apply this diff to add the missing per-slice bounds check:
const loader = blk: { - if (this.graph.ast.len > resolve.import_record.importer_source_index) { - const record: *ImportRecord = &this.graph.ast.items(.import_records)[resolve.import_record.importer_source_index].slice()[resolve.import_record.import_record_index]; - if (record.loader) |override_loader| { - break :blk override_loader; - } - } - break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file; + const all_import_records = this.graph.ast.items(.import_records); + if (resolve.import_record.importer_source_index >= all_import_records.len) + break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file; + const list = all_import_records[resolve.import_record.importer_source_index].slice(); + if (resolve.import_record.import_record_index >= list.len) + break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file; + const record: *ImportRecord = &list[resolve.import_record.import_record_index]; + if (record.loader) |override_loader| break :blk override_loader; + break :blk path.loader(&this.transpiler.options.loaders) orelse options.Loader.file; };src/js_printer.zig (1)
2099-2102: Fallback still doesn’t throw — invoke the IIFE.The
.import_meta_globbranch is still printing a bare function expression, so the runtime gets a function value instead of throwing. Please append()(and a terminator) to actually execute the IIFE.- p.print("(function() { throw new Error('import.meta.glob was not transformed at build time'); })"); + p.print("(function(){throw new Error('import.meta.glob was not transformed at build time')})();");packages/bun-types/globals.d.ts (1)
1366-1374: Type soundness violation: Remove the eager overload.The third overload at line 1374 promises
Record<string, TModule>when eager mode is enabled, but this is unsound:
ImportMetaGlobOptions.eageris explicitly constrained tofalse(line 1408), making this overload unreachable through normal usage- The PR description confirms eager mode is not implemented in the runtime
- The runtime will always emit lazy loader functions
() => Promise<TModule>, never raw modulesAdditionally, the first overload's conditional return type
Eager extends true ? Record<string, TModule> : Record<string, () => Promise<TModule>>is misleading sinceImportMetaGlobOptions<Eager>can only acceptfalseas a valideagervalue.This was flagged in a previous review but appears to still be present. The types must match runtime behavior to prevent false expectations and potential runtime errors.
Apply this diff to remove the unsound overload and simplify the signature:
- glob<Eager extends boolean = false, TModule = unknown>( + glob<TModule = unknown>( pattern: string | string[], - options?: ImportMetaGlobOptions<Eager>, - ): Eager extends true ? Record<string, TModule> : Record<string, () => Promise<TModule>>; - glob<TModule = unknown>( - pattern: string | string[], - options?: ImportMetaGlobOptions<false>, + options?: ImportMetaGlobOptions, ): Record<string, () => Promise<TModule>>; - glob<TModule = unknown>(pattern: string | string[], options?: ImportMetaGlobOptions<true>): Record<string, TModule>;When eager mode is implemented in the future, add back an overload with a separate interface like
ImportMetaGlobEagerOptionsthat explicitly setseager: true.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
docs/api/import-meta.md(2 hunks)packages/bun-types/globals.d.ts(1 hunks)src/ast/P.zig(3 hunks)src/bundler/bundle_v2.zig(1 hunks)src/js_printer.zig(2 hunks)test/js/bun/glob/import-meta-glob.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/ast/P.zigsrc/bundler/bundle_v2.zigsrc/js_printer.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use private fields in Zig with the#prefix (e.g.,struct { #foo: u32 };)
Prefer decl literals in Zig (e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing@importstatements at the bottom of the Zig file (formatter may reorder automatically)
Prefer@import("bun")rather than@import("root").bunor@import("../bun.zig")
Files:
src/ast/P.zigsrc/bundler/bundle_v2.zigsrc/js_printer.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place JavaScript and TypeScript tests under test/js/
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/bun/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/bun/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Bun-specific API tests under test/js/bun/
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/{bun,node}/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests by module under
/test/js/bun/and/test/js/node/
Files:
test/js/bun/glob/import-meta-glob.test.ts
src/**/js_*.zig
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
src/**/js_*.zig: Implement JavaScript bindings in a Zig file named with a js_ prefix (e.g., js_smtp.zig, js_your_feature.zig)
Handle reference counting correctly with ref()/deref() in JS-facing Zig code
Always implement proper cleanup in deinit() and finalize() for JS-exposed types
Files:
src/js_printer.zig
src/{**/js_*.zig,bun.js/api/**/*.zig}
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Files:
src/js_printer.zig
🧠 Learnings (12)
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
PR: oven-sh/bun#22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/ast/P.zigsrc/bundler/bundle_v2.zig
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer running tests via bun bd test <file> and use provided harness utilities (bunEnv, bunExe, tempDir)
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js,test/**/*-fixture.ts} : Use `using`/`await using` for resource cleanup with Bun APIs (e.g., `Bun.spawn`, `Bun.listen`, `Bun.serve`)
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : Import common utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, platform helpers, GC helpers)
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-09-13T17:01:41.393Z
Learnt from: RiskyMH
PR: oven-sh/bun#21459
File: docs/api/import-meta.md:0-0
Timestamp: 2025-09-13T17:01:41.393Z
Learning: For docs/api/import-meta.md, keep documentation entries concise and consistent with the existing style - brief descriptions with small inline examples, not extensive bullet points or detailed explanations.
Applied to files:
docs/api/import-meta.md
🧬 Code graph analysis (1)
test/js/bun/glob/import-meta-glob.test.ts (1)
test/harness.ts (3)
tempDirWithFiles(259-266)bunExe(102-105)tempDir(277-284)
🪛 LanguageTool
docs/api/import-meta.md
[grammar] ~74-~74: Use correct spacing
Context: ...ng file paths to lazy-loading functions. ts const modules = import.meta.glob("./src/*.ts"); // const modules = { // './src/a.ts': () => import('./src/a.ts'), // './src/b.ts': () => import('./src/b.ts'), // } {% /table %}
(QB_NEW_EN_OTHER_ERROR_IDS_5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (1)
packages/bun-types/globals.d.ts (1)
1463-1465: LGTM!The
ImportAttributesinterface correctly defines supported import types with a flexible catch-all for custom loaders. The(string & {})pattern preserves autocomplete for common types while allowing any string value.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/ast/P.zig (1)
6138-6243: Return module specifiers relative to the importerWe’re still emitting absolute filesystem paths as both object keys and import targets (
slash_normalizedis appended directly), so the generated map looks like{ "/Users/me/project/src/foo.js": () => import("/Users/me/project/src/foo.js") }. That breaks bundle-time resolution and diverges from Vite’s contract, which requires specifiers relative to the current module (e.g../foo.js). Please slice offsearch_dir, guard the separator, add a./prefix when needed, and reuse that relative specifier when buildingimport_path(then applybase/query). Example fix:- const duped = bun.handleOom(p.allocator.dupe(u8, slash_normalized)); - bun.handleOom(matched_files.append(duped)); + const rel = blk: { + if (strings.hasPrefix(slash_normalized, search_dir) and slash_normalized.len >= search_dir.len) { + var start = search_dir.len; + if (start < slash_normalized.len and (slash_normalized[start] == '/' or slash_normalized[start] == '\\')) { + start += 1; + } + break :blk slash_normalized[start..]; + } + break :blk slash_normalized; + }; + const specifier = if (rel.len == 0 or rel[0] == '.' or rel[0] == '/') + bun.handleOom(p.allocator.dupe(u8, rel)) + else + bun.handleOom(std.fmt.allocPrint(p.allocator, "./{s}", .{rel})); + bun.handleOom(matched_files.append(specifier));Then build
import_pathfromspecifier(applybase/queryon top). This keeps us compatible with Vite and makes bundler resolution work again.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/ast/P.zig(3 hunks)test/js/bun/glob/import-meta-glob.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/ast/P.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use private fields in Zig with the#prefix (e.g.,struct { #foo: u32 };)
Prefer decl literals in Zig (e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing@importstatements at the bottom of the Zig file (formatter may reorder automatically)
Prefer@import("bun")rather than@import("root").bunor@import("../bun.zig")
Files:
src/ast/P.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place JavaScript and TypeScript tests under test/js/
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/bun/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/bun/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Bun-specific API tests under test/js/bun/
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/js/bun/glob/import-meta-glob.test.ts
test/js/{bun,node}/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests by module under
/test/js/bun/and/test/js/node/
Files:
test/js/bun/glob/import-meta-glob.test.ts
🧠 Learnings (11)
📚 Learning: 2025-09-06T03:37:41.154Z
Learnt from: taylordotfish
PR: oven-sh/bun#22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.
Applied to files:
src/ast/P.zig
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js,test/**/*-fixture.ts} : Use `using`/`await using` for resource cleanup with Bun APIs (e.g., `Bun.spawn`, `Bun.listen`, `Bun.serve`)
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/js/bun/glob/import-meta-glob.test.ts
🧬 Code graph analysis (1)
test/js/bun/glob/import-meta-glob.test.ts (1)
test/harness.ts (3)
tempDirWithFiles(259-266)bunExe(102-105)tempDir(277-284)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (2)
test/js/bun/glob/import-meta-glob.test.ts (2)
856-856: Bundler transformation check is appropriate.The assertion that the bundled output doesn't contain "glob" is a reasonable sanity check to verify that
import.meta.globcalls were transformed away during bundling. While string searches can be fragile, this check serves as a quick validation that the transformation occurred.
5-880: Excellent comprehensive test coverage.The test suite thoroughly covers:
- Runtime behavior with various glob patterns (single, multiple, recursive, negative)
- Named export extraction via the
importoption- Query and
withoption propagation- Base path semantics with relative and parent directories
- Bundler integration with various scenarios (basic, splitting, negative patterns)
- Error cases (eager mode not yet supported, empty results)
The tests properly use
bunExe()andbunEnvfrom harness, spawn processes with correct stdio handling, and await process completion. The organization into runtime and bundler behavior sections is clear and maintainable.Based on learnings: Tests follow the guidelines for using
bun:test, spawning Bun withbunExe()andbunEnv, and organizing tests undertest/js/bun/glob/.
| test("returns lazy-loading functions for matched files", async () => { | ||
| const dir = tempDirWithFiles("import-glob-basic", { | ||
| "index.js": ` | ||
| const modules = import.meta.glob('./modules/*.js'); | ||
| console.log(JSON.stringify(Object.keys(modules))); | ||
| console.log(typeof modules['./modules/a.js']); | ||
| `, | ||
| "modules/a.js": `export const name = "a";`, | ||
| "modules/b.js": `export const name = "b";`, | ||
| "modules/c.js": `export const name = "c";`, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "index.js"], | ||
| env: bunEnv, | ||
| cwd: dir, | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([ | ||
| new Response(proc.stdout).text(), | ||
| new Response(proc.stderr).text(), | ||
| proc.exited, | ||
| ]); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| expect(stderr).toBe(""); | ||
| const lines = stdout.trim().split("\n"); | ||
| expect(JSON.parse(lines[0])).toEqual(["./modules/a.js", "./modules/b.js", "./modules/c.js"]); | ||
| expect(lines[1]).toBe("function"); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Apply consistent resource cleanup pattern.
This test and several others (lines 38, 90, 122, 149, 195, 236, 267, 294) use tempDirWithFiles without automatic cleanup. Per coding guidelines, prefer using dir = tempDir(...) to ensure temp directories are cleaned up even if tests fail.
Apply this pattern consistently across all tests:
- const dir = tempDirWithFiles("import-glob-basic", {
+ using dir = tempDir("import-glob-basic", {
"index.js": `Then update the cwd to use String(dir) if needed:
cwd: dir,
+ cwd: String(dir),This refactor should be applied to tests at lines 7, 38, 90, 122, 149, 195, 236, 267, and 294 for consistency with tests at lines 342, 386, 424, 736, and 783 that already follow this pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("returns lazy-loading functions for matched files", async () => { | |
| const dir = tempDirWithFiles("import-glob-basic", { | |
| "index.js": ` | |
| const modules = import.meta.glob('./modules/*.js'); | |
| console.log(JSON.stringify(Object.keys(modules))); | |
| console.log(typeof modules['./modules/a.js']); | |
| `, | |
| "modules/a.js": `export const name = "a";`, | |
| "modules/b.js": `export const name = "b";`, | |
| "modules/c.js": `export const name = "c";`, | |
| }); | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "index.js"], | |
| env: bunEnv, | |
| cwd: dir, | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]); | |
| expect(exitCode).toBe(0); | |
| expect(stderr).toBe(""); | |
| const lines = stdout.trim().split("\n"); | |
| expect(JSON.parse(lines[0])).toEqual(["./modules/a.js", "./modules/b.js", "./modules/c.js"]); | |
| expect(lines[1]).toBe("function"); | |
| }); | |
| test("returns lazy-loading functions for matched files", async () => { | |
| using dir = tempDir("import-glob-basic", { | |
| "index.js": ` | |
| const modules = import.meta.glob('./modules/*.js'); | |
| console.log(JSON.stringify(Object.keys(modules))); | |
| console.log(typeof modules['./modules/a.js']); | |
| `, | |
| "modules/a.js": `export const name = "a";`, | |
| "modules/b.js": `export const name = "b";`, | |
| "modules/c.js": `export const name = "c";`, | |
| }); | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "index.js"], | |
| env: bunEnv, | |
| cwd: String(dir), | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]); | |
| expect(exitCode).toBe(0); | |
| expect(stderr).toBe(""); | |
| const lines = stdout.trim().split("\n"); | |
| expect(JSON.parse(lines[0])).toEqual(["./modules/a.js", "./modules/b.js", "./modules/c.js"]); | |
| expect(lines[1]).toBe("function"); | |
| }); |
🤖 Prompt for AI Agents
In test/js/bun/glob/import-meta-glob.test.ts around lines 7 to 36, replace the
raw tempDirWithFiles(...) call with the RAII pattern so the temp dir is always
cleaned up: declare it as "using dir = tempDirWithFiles(...)" (or "using dir =
tempDir(...)" if you prefer the plain tempDir helper) and update the Bun.spawn
cwd to use String(dir). Apply the same change to the other test locations listed
(lines 38, 90, 122, 149, 195, 236, 267, 294) to ensure consistent automatic
cleanup.
| test("handles empty results gracefully", () => { | ||
| const modules = import.meta.glob("./non-existent/*.js"); | ||
| expect(typeof modules).toBe("object"); | ||
| expect(Object.keys(modules)).toHaveLength(0); | ||
| expect(JSON.stringify(modules)).toBe("{}"); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Isolate test in spawned process for consistency.
Unlike other tests in this suite, this test runs import.meta.glob directly in the test process rather than spawning a separate Bun process. While this works, it creates inconsistency and could cause issues if the test file's directory structure changes.
Consider spawning a process like other tests for consistency:
- test("handles empty results gracefully", () => {
- const modules = import.meta.glob("./non-existent/*.js");
- expect(typeof modules).toBe("object");
- expect(Object.keys(modules)).toHaveLength(0);
- expect(JSON.stringify(modules)).toBe("{}");
- });
+ test("handles empty results gracefully", async () => {
+ using dir = tempDir("import-glob-empty", {
+ "index.js": `
+ const modules = import.meta.glob("./non-existent/*.js");
+ console.log('TYPE:', typeof modules);
+ console.log('LENGTH:', Object.keys(modules).length);
+ console.log('JSON:', JSON.stringify(modules));
+ `,
+ });
+
+ await using proc = Bun.spawn({
+ cmd: [bunExe(), "index.js"],
+ env: bunEnv,
+ cwd: String(dir),
+ });
+
+ const [stdout, stderr, exitCode] = await Promise.all([
+ new Response(proc.stdout).text(),
+ new Response(proc.stderr).text(),
+ proc.exited,
+ ]);
+
+ expect(exitCode).toBe(0);
+ expect(stderr).toBe("");
+ expect(stdout).toContain("TYPE: object");
+ expect(stdout).toContain("LENGTH: 0");
+ expect(stdout).toContain('JSON: {}');
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("handles empty results gracefully", () => { | |
| const modules = import.meta.glob("./non-existent/*.js"); | |
| expect(typeof modules).toBe("object"); | |
| expect(Object.keys(modules)).toHaveLength(0); | |
| expect(JSON.stringify(modules)).toBe("{}"); | |
| }); | |
| test("handles empty results gracefully", async () => { | |
| using dir = tempDir("import-glob-empty", { | |
| "index.js": ` | |
| const modules = import.meta.glob("./non-existent/*.js"); | |
| console.log('TYPE:', typeof modules); | |
| console.log('LENGTH:', Object.keys(modules).length); | |
| console.log('JSON:', JSON.stringify(modules)); | |
| `, | |
| }); | |
| await using proc = Bun.spawn({ | |
| cmd: [bunExe(), "index.js"], | |
| env: bunEnv, | |
| cwd: String(dir), | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(proc.stdout).text(), | |
| new Response(proc.stderr).text(), | |
| proc.exited, | |
| ]); | |
| expect(exitCode).toBe(0); | |
| expect(stderr).toBe(""); | |
| expect(stdout).toContain("TYPE: object"); | |
| expect(stdout).toContain("LENGTH: 0"); | |
| expect(stdout).toContain('JSON: {}'); | |
| }); |
🤖 Prompt for AI Agents
In test/js/bun/glob/import-meta-glob.test.ts around lines 229 to 234, this test
calls import.meta.glob directly in the test process which is inconsistent with
other tests; change it to spawn a separate Bun process that runs a small script
performing the import.meta.glob call and prints/asserts the result (or returns
JSON) so the parent test can read and assert that the result is an empty object;
ensure the spawned process is created with the same environment/working
directory as other tests, capture stdout/stderr, parse the output and replace
the direct assertions with assertions against the spawned process output and
exit code to maintain consistency with the suite.
| test("preserves import.meta.glob functionality after bundling", async () => { | ||
| const dir = tempDirWithFiles("import-glob-bundle", { | ||
| "index.js": ` | ||
| const modules = import.meta.glob('./src/*.js'); | ||
| console.log('COUNT:', Object.keys(modules).length); | ||
| console.log('FIRST_TYPE:', typeof Object.values(modules)[0]); | ||
| `, | ||
| "src/a.js": `export default "a";`, | ||
| "src/b.js": `export default "b";`, | ||
| }); | ||
|
|
||
| await using buildProc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "index.js", "--outfile", "dist/bundle.js"], | ||
| env: bunEnv, | ||
| cwd: dir, | ||
| }); | ||
| await buildProc.exited; | ||
|
|
||
| await using runProc = Bun.spawn({ | ||
| cmd: [bunExe(), "dist/bundle.js"], | ||
| env: bunEnv, | ||
| cwd: dir, | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([ | ||
| new Response(runProc.stdout).text(), | ||
| new Response(runProc.stderr).text(), | ||
| runProc.exited, | ||
| ]); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| expect(stderr).toBe(""); | ||
| const lines = stdout.trim().split("\n"); | ||
| expect(lines[0]).toBe("COUNT: 2"); | ||
| expect(lines[1]).toBe("FIRST_TYPE: function"); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Apply consistent resource cleanup in bundler tests.
Similar to runtime tests, bundler tests at lines 458, 496, 527, 567, 634, 681, and 830 use tempDirWithFiles without automatic cleanup, while tests at lines 736 and 783 correctly use using dir = tempDir(...).
Apply the same resource cleanup pattern:
- const dir = tempDirWithFiles("import-glob-bundle", {
+ using dir = tempDir("import-glob-bundle", {And update cwd references:
- cwd: dir,
+ cwd: String(dir),This ensures temp directories are properly cleaned up even if tests fail, following the coding guidelines for resource management with using/await using.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test("preserves import.meta.glob functionality after bundling", async () => { | |
| const dir = tempDirWithFiles("import-glob-bundle", { | |
| "index.js": ` | |
| const modules = import.meta.glob('./src/*.js'); | |
| console.log('COUNT:', Object.keys(modules).length); | |
| console.log('FIRST_TYPE:', typeof Object.values(modules)[0]); | |
| `, | |
| "src/a.js": `export default "a";`, | |
| "src/b.js": `export default "b";`, | |
| }); | |
| await using buildProc = Bun.spawn({ | |
| cmd: [bunExe(), "build", "index.js", "--outfile", "dist/bundle.js"], | |
| env: bunEnv, | |
| cwd: dir, | |
| }); | |
| await buildProc.exited; | |
| await using runProc = Bun.spawn({ | |
| cmd: [bunExe(), "dist/bundle.js"], | |
| env: bunEnv, | |
| cwd: dir, | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(runProc.stdout).text(), | |
| new Response(runProc.stderr).text(), | |
| runProc.exited, | |
| ]); | |
| expect(exitCode).toBe(0); | |
| expect(stderr).toBe(""); | |
| const lines = stdout.trim().split("\n"); | |
| expect(lines[0]).toBe("COUNT: 2"); | |
| expect(lines[1]).toBe("FIRST_TYPE: function"); | |
| }); | |
| test("preserves import.meta.glob functionality after bundling", async () => { | |
| using dir = tempDir("import-glob-bundle", { | |
| "index.js": ` | |
| const modules = import.meta.glob('./src/*.js'); | |
| console.log('COUNT:', Object.keys(modules).length); | |
| console.log('FIRST_TYPE:', typeof Object.values(modules)[0]); | |
| `, | |
| "src/a.js": `export default "a";`, | |
| "src/b.js": `export default "b";`, | |
| }); | |
| await using buildProc = Bun.spawn({ | |
| cmd: [bunExe(), "build", "index.js", "--outfile", "dist/bundle.js"], | |
| env: bunEnv, | |
| cwd: String(dir), | |
| }); | |
| await buildProc.exited; | |
| await using runProc = Bun.spawn({ | |
| cmd: [bunExe(), "dist/bundle.js"], | |
| env: bunEnv, | |
| cwd: String(dir), | |
| }); | |
| const [stdout, stderr, exitCode] = await Promise.all([ | |
| new Response(runProc.stdout).text(), | |
| new Response(runProc.stderr).text(), | |
| runProc.exited, | |
| ]); | |
| expect(exitCode).toBe(0); | |
| expect(stderr).toBe(""); | |
| const lines = stdout.trim().split("\n"); | |
| expect(lines[0]).toBe("COUNT: 2"); | |
| expect(lines[1]).toBe("FIRST_TYPE: function"); | |
| }); |
🤖 Prompt for AI Agents
In test/js/bun/glob/import-meta-glob.test.ts around lines 458 to 493, replace
the plain tempDirWithFiles(...) call with the resource-managed pattern used
elsewhere so the temp directory is always cleaned up: declare the directory with
the using/await using pattern (e.g., await using dir =
tempDirWithFiles("import-glob-bundle", { ... });) and update subsequent cwd
references to use that dir variable (cwd: dir or cwd: dir.path consistent with
other tests). Repeat this change for the other bundler test sites called out in
the review to ensure deterministic cleanup.
|
Awesome that this feature gets implemented! Seems not to support tsconfig path aliases at the moment? |
|
Also, --hot on the command line plus development: { hmr: true, console: true } in the server file still don't make the glob updated automatically when new files that match it are created - could that be implemented? (Even a full reload on the client side is currently not enough, I have to restart the server manually.) |
# Conflicts: # docs/api/import-meta.md # src/bun.js/RuntimeTranspilerCache.zig # test/no-validate-exceptions.txt
|
Closing as stale: this PR predates the Rust rewrite. Every If the underlying change is still wanted, it will need to be redone against the current Rust/C++ tree. Apologies for the churn, and thank you for the contribution. |
7 participants
What does this PR do?
Fixes #6060 by implementing most of vite's
import.meta.globapi. This allows for easy bundle time file including. It is missing the eager option because that's quite a bit extra of a change, but can work it out if needed. Also in future a direct import glob would be cool too.also while trying out edgecases, I made the bundler actually support
await import('./script.js', { with: { type: 'text' } });(right now it just does based on extension instead of specified one like runtime)How did you verify your code works?
made tests + manual