diff --git a/docs/runtime/bunfig.mdx b/docs/runtime/bunfig.mdx index dc0dc093ba5..6cf2ddd4222 100644 --- a/docs/runtime/bunfig.mdx +++ b/docs/runtime/bunfig.mdx @@ -512,11 +512,11 @@ prefer = "online" Valid values are: -| Value | Description | -| ----------- | ------------------------------------------------------------------------------------------------- | -| `"online"` | Default. Check the registry for stale packages as needed. | +| Value | Description | +| ----------- | -------------------------------------------------------------------------------------------------- | +| `"online"` | Default. Check the registry for stale packages as needed. | | `"offline"` | Skip staleness checks and resolve packages from the local cache. Equivalent to `--prefer-offline`. | -| `"latest"` | Always check npm for the latest matching versions. Equivalent to `--prefer-latest`. | +| `"latest"` | Always check npm for the latest matching versions. Equivalent to `--prefer-latest`. | ### `install.frozenLockfile` diff --git a/src/bun.js/bindings/BunPlugin.cpp b/src/bun.js/bindings/BunPlugin.cpp index f5abe87b3cc..e6f847749ef 100644 --- a/src/bun.js/bindings/BunPlugin.cpp +++ b/src/bun.js/bindings/BunPlugin.cpp @@ -526,6 +526,12 @@ extern "C" JSC_DEFINE_HOST_FUNCTION(JSMock__jsModuleMock, (JSC::JSGlobalObject * return {}; } + JSC::JSValue callbackValue = callframe->argument(1); + if (!callbackValue.isCell() || !callbackValue.isCallable()) { + scope.throwException(lexicalGlobalObject, JSC::createTypeError(lexicalGlobalObject, "mock(module, fn) requires a function"_s)); + return {}; + } + auto resolveSpecifier = [&]() -> void { JSC::SourceOrigin sourceOrigin = callframe->callerSourceOrigin(vm); if (sourceOrigin.isNull()) @@ -581,12 +587,6 @@ extern "C" JSC_DEFINE_HOST_FUNCTION(JSMock__jsModuleMock, (JSC::JSGlobalObject * resolveSpecifier(); RETURN_IF_EXCEPTION(scope, {}); - JSC::JSValue callbackValue = callframe->argument(1); - if (!callbackValue.isCell() || !callbackValue.isCallable()) { - scope.throwException(lexicalGlobalObject, JSC::createTypeError(lexicalGlobalObject, "mock(module, fn) requires a function"_s)); - return {}; - } - JSC::JSObject* callback = callbackValue.getObject(); JSModuleMock* mock = JSModuleMock::create(vm, globalObject->mockModule.mockModuleStructure.getInitializedOnMainThread(globalObject), callback); diff --git a/test/js/bun/test/mock/mock-module-non-string.test.ts b/test/js/bun/test/mock/mock-module-non-string.test.ts index 13551aa8e5d..b77a294702b 100644 --- a/test/js/bun/test/mock/mock-module-non-string.test.ts +++ b/test/js/bun/test/mock/mock-module-non-string.test.ts @@ -1,4 +1,5 @@ import { expect, mock, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; test("mock.module throws TypeError for non-string first argument", () => { // @ts-expect-error @@ -36,3 +37,89 @@ test("mock.module does not crash on specifiers that are not valid npm package na } Bun.gc(true); }); + +test("mock.module throws TypeError without resolving when callback is missing", () => { + // The resolver can reach the package-manager auto-install path and reentrantly + // tick the JS event loop. When the caller forgets the callback, we must throw + // before running the resolver at all. + const specifiers = [ + // valid npm package name — bypasses the isNPMPackageName gate in the resolver + "PbQ", + "some-package-that-does-not-exist", + "@scope/pkg", + "function f3() {}", + "() => 1", + ]; + for (const specifier of specifiers) { + // @ts-expect-error missing callback on purpose + expect(() => mock.module(specifier)).toThrow("mock(module, fn) requires a function"); + } + for (const specifier of specifiers) { + // @ts-expect-error non-callable second argument on purpose + expect(() => mock.module(specifier, 123)).toThrow("mock(module, fn) requires a function"); + } + Bun.gc(true); +}); + +test("mock.module does not run the resolver when callback is missing", async () => { + // When auto-install is enabled and the specifier is a valid npm package name, + // the resolver reaches the package-manager path, reentrantly ticks the JS + // event loop, and blocks waiting on the registry. If the callback is missing + // we must throw before the resolver runs so none of that happens. + let requests = 0; + const { promise: gotRequest, resolve: onRequest } = Promise.withResolvers(); + await using server = Bun.serve({ + port: 0, + fetch() { + requests++; + onRequest(); + return new Response("{}", { status: 404, headers: { "content-type": "application/json" } }); + }, + }); + + using dir = tempDir("mock-module-no-callback-no-resolve", { + "index.js": ` + const { vi } = Bun.jest(); + try { vi.mock("PbQ"); } catch (e) { console.error("ERR:", e.message); } + try { vi.mock("some-valid-pkg-name-abc123"); } catch (e) { console.error("ERR:", e.message); } + Bun.gc(true); + console.log("done"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "--install=force", "index.js"], + env: { + ...bunEnv, + BUN_CONFIG_REGISTRY: server.url.href, + NPM_CONFIG_REGISTRY: server.url.href, + BUN_INSTALL_CACHE_DIR: String(dir) + "/.cache", + }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + // Before the fix, the resolver blocks on the registry and the process never + // exits on its own. Race the exit against the first registry request so the + // test fails fast (instead of timing out) on a regression. + const result = await Promise.race([ + Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]).then(([stdout, stderr, exitCode]) => ({ + kind: "exited" as const, + stdout, + stderr, + exitCode, + })), + gotRequest.then(() => ({ kind: "request" as const })), + ]); + + if (result.kind === "request") { + proc.kill(); + throw new Error("mock.module ran the resolver and hit the registry when callback was missing"); + } + + expect(result.stderr).toContain("mock(module, fn) requires a function"); + expect(result.stdout).toContain("done"); + expect(requests).toBe(0); + expect(result.exitCode).toBe(0); +});