Address PR review comments

- Add test for virtual module as entrypoint (requested by alii)
- Include module name in error messages for better debugging (requested by alii)
- Check outputs in bun-build-api test to verify virtual module content (requested by alii)
- Use intrinsic argument validation (, ) for consistency
- Guard against undefined onResolve map to prevent crashes

All tests continue to pass with these improvements.
This commit is contained in:
Claude Bot
2025-09-10 01:03:07 +00:00
parent c605abc61c
commit 52c138ef15
3 changed files with 44 additions and 8 deletions

View File

@@ -356,11 +356,11 @@ export function runSetupFunction(
onStart,
resolve: notImplementedIssueFn(2771, "build.resolve()"),
module: (specifier: string, callback: () => { contents: string; loader?: string }) => {
if (typeof specifier !== "string") {
throw new TypeError("module() specifier must be a string");
if (!(typeof specifier === "string")) {
throw $ERR_INVALID_ARG_TYPE("specifier", "string", specifier);
}
if (typeof callback !== "function") {
throw new TypeError("module() callback must be a function");
if (!$isCallable(callback)) {
throw $ERR_INVALID_ARG_TYPE("callback", "function", callback);
}
// Store the virtual module
@@ -428,6 +428,11 @@ export function runOnResolvePlugins(this: BundlerPlugin, specifier, inputNamespa
return null;
}
if (!onResolve) {
this.onResolveAsync(internalID, null, null, null);
return null;
}
var results = onResolve.$get(inputNamespace);
if (!results) {
this.onResolveAsync(internalID, null, null, null);
@@ -558,22 +563,22 @@ export function runOnLoadPlugins(
try {
if (!result || !$isObject(result)) {
throw new TypeError('Virtual module must return an object with "contents" property');
throw new TypeError(`Virtual module "${path}" must return an object with "contents" property`);
}
var { contents, loader = "js" } = result;
if (!(typeof contents === "string")) {
throw new TypeError('Virtual module must return an object with "contents" as a string');
throw new TypeError(`Virtual module "${path}" must return an object with "contents" as a string`);
}
if (!(typeof loader === "string")) {
throw new TypeError('Virtual module "loader" must be a string if provided');
throw new TypeError(`Virtual module "${path}" "loader" must be a string if provided`);
}
const chosenLoader = LOADERS_MAP[loader];
if (chosenLoader === undefined) {
throw new TypeError(`Loader ${loader} is not supported.`);
throw new TypeError(`Virtual module "${path}": Loader ${loader} is not supported.`);
}
this.onLoadAsync(internalID, contents, chosenLoader);

View File

@@ -424,6 +424,11 @@ describe("Bun.build", () => {
});
expect(result.success).toBe(true);
expect(result.outputs).toHaveLength(1);
// Check that the virtual module content is in the output
const output = await result.outputs[0].text();
expect(output).toContain("Hello from virtual module");
});
test("non-object plugins throw invalid argument errors", () => {

View File

@@ -406,6 +406,32 @@ test("Bun.build plugin virtual modules - onLoad plugins still work", async () =>
expect(output).toContain("by onLoad plugin");
});
test("Bun.build plugin virtual modules - virtual module as entrypoint", async () => {
using dir = tempDir("virtual-entrypoint", {});
const result = await Bun.build({
entrypoints: ["virtual-entry"],
outdir: String(dir),
plugins: [
{
name: "in-memory-entrypoint",
setup(build) {
build.module("virtual-entry", () => ({
contents: `console.log("Hello from virtual entrypoint");`,
loader: "js",
}));
},
},
],
});
expect(result.success).toBe(true);
expect(result.outputs).toHaveLength(1);
const output = await Bun.file(result.outputs[0].path).text();
expect(output).toContain("Hello from virtual entrypoint");
});
test("Bun.build plugin virtual modules - no memory leak on repeated builds", async () => {
using dir = tempDir("virtual-memory", {
"entry.ts": `