diff --git a/src/js/builtins/BundlerPlugin.ts b/src/js/builtins/BundlerPlugin.ts index 9f05e17b5988..d88fa04fde5d 100644 --- a/src/js/builtins/BundlerPlugin.ts +++ b/src/js/builtins/BundlerPlugin.ts @@ -403,79 +403,110 @@ export function runOnResolvePlugins(this: BundlerPlugin, specifier, inputNamespa var promiseResult: any = (async (inputPath, inputNamespace, importer, kind) => { var { onResolve, onLoad } = this; - var results = onResolve.$get(inputNamespace); - if (!results) { - this.onResolveAsync(internalID, null, null, null); - return null; - } - for (let [filter, callback] of results) { - if (filter.test(inputPath)) { - var result = callback({ - path: inputPath, - importer, - namespace: inputNamespace, - resolveDir: inputNamespace === "file" ? require("node:path").dirname(importer) : undefined, - kind, - // pluginData - }); + const tryNamespace = async (matchNamespace: string, matchPath: string) => { + var results = onResolve.$get(matchNamespace); + if (!results) { + return false; + } - while (result && $isPromise(result) && $peekPromiseStatus(result) === 1) { - result = $peekPromiseSettledValue(result); - } + for (let [filter, callback] of results) { + if (filter.test(matchPath)) { + var result = callback({ + path: matchPath, + importer, + namespace: matchNamespace, + resolveDir: inputNamespace === "file" ? require("node:path").dirname(importer) : undefined, + kind, + // pluginData + }); + + while (result && $isPromise(result) && $peekPromiseStatus(result) === 1) { + result = $peekPromiseSettledValue(result); + } - if (result && $isPromise(result)) { - result = await result; - } + if (result && $isPromise(result)) { + result = await result; + } - if (!result || !$isObject(result)) { - continue; - } + if (!result || !$isObject(result)) { + continue; + } - var { path, namespace: userNamespace = inputNamespace, external } = result; - if (path !== undefined && typeof path !== "string") { - throw new TypeError("onResolve plugins 'path' field must be a string if provided"); - } + var { path, namespace: userNamespace = inputNamespace, external } = result; + if (path !== undefined && typeof path !== "string") { + throw new TypeError("onResolve plugins 'path' field must be a string if provided"); + } - if (result.namespace !== undefined && typeof result.namespace !== "string") { - throw new TypeError("onResolve plugins 'namespace' field must be a string if provided"); - } + if (result.namespace !== undefined && typeof result.namespace !== "string") { + throw new TypeError("onResolve plugins 'namespace' field must be a string if provided"); + } - if (!path) { - continue; - } + if (!path) { + continue; + } - if (!userNamespace) { - userNamespace = inputNamespace; - } - if (typeof external !== "boolean" && !$isUndefinedOrNull(external)) { - throw new TypeError('onResolve plugins "external" field must be boolean or unspecified'); - } + if (!userNamespace) { + userNamespace = inputNamespace; + } + if (typeof external !== "boolean" && !$isUndefinedOrNull(external)) { + throw new TypeError('onResolve plugins "external" field must be boolean or unspecified'); + } - if (!external) { - if (userNamespace === "file") { - if (process.platform !== "win32") { - if (path[0] !== "/" || path.includes("..")) { - throw new TypeError('onResolve plugin "path" must be absolute when the namespace is "file"'); + if (!external) { + if (userNamespace === "file") { + if (process.platform !== "win32") { + if (path[0] !== "/" || path.includes("..")) { + throw new TypeError('onResolve plugin "path" must be absolute when the namespace is "file"'); + } + } else { + if (require("node:path").isAbsolute(path) === false || path.includes("..")) { + throw new TypeError('onResolve plugin "path" must be absolute when the namespace is "file"'); + } } - } else { - if (require("node:path").isAbsolute(path) === false || path.includes("..")) { - throw new TypeError('onResolve plugin "path" must be absolute when the namespace is "file"'); + } + if (userNamespace === "dataurl") { + if (!path.startsWith("data:")) { + throw new TypeError('onResolve plugin "path" must start with "data:" when the namespace is "dataurl"'); } } - } - if (userNamespace === "dataurl") { - if (!path.startsWith("data:")) { - throw new TypeError('onResolve plugin "path" must start with "data:" when the namespace is "dataurl"'); + + if (userNamespace && userNamespace !== "file" && (!onLoad || !onLoad.$has(userNamespace))) { + throw new TypeError(`Expected onLoad plugin for namespace ${userNamespace} to exist`); } } + this.onResolveAsync(internalID, path, userNamespace, external); + return true; + } + } - if (userNamespace && userNamespace !== "file" && (!onLoad || !onLoad.$has(userNamespace))) { - throw new TypeError(`Expected onLoad plugin for namespace ${userNamespace} to exist`); + return false; + }; + + // Peek before awaiting so a synchronous callback stays on the synchronous path. + var matched = tryNamespace(inputNamespace, inputPath); + if ($peekPromiseStatus(matched) === 1 ? $peekPromiseSettledValue(matched) : await matched) { + return null; + } + + // Also offer "ns:rest" to onResolve({ namespace: "ns" }) with the stripped path. + if (inputNamespace === "file") { + var colon = inputPath.indexOf(":"); + if (colon > 0) { + var prefix = inputPath.slice(0, colon); + var isDriveLetter = + process.platform === "win32" && + colon === 1 && + inputPath.length > 2 && + (inputPath.charCodeAt(0) | 0x20) >= 97 && + (inputPath.charCodeAt(0) | 0x20) <= 122 && + (inputPath.charCodeAt(2) === 47 || inputPath.charCodeAt(2) === 92); + if (!isDriveLetter && prefix !== "file") { + matched = tryNamespace(prefix, inputPath.slice(colon + 1)); + if ($peekPromiseStatus(matched) === 1 ? $peekPromiseSettledValue(matched) : await matched) { + return null; } } - this.onResolveAsync(internalID, path, userNamespace, external); - return null; } } diff --git a/src/jsc/bindings/BunPlugin.cpp b/src/jsc/bindings/BunPlugin.cpp index db413a9f34b7..783ff9afd913 100644 --- a/src/jsc/bindings/BunPlugin.cpp +++ b/src/jsc/bindings/BunPlugin.cpp @@ -797,13 +797,8 @@ std::optional BunPlugin::OnLoad::resolveVirtualModule(const String& path return virtualModules->contains(path) ? std::optional { path } : std::nullopt; } -EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, const BunString* namespaceString, const BunString* path, const BunString* importer) +static EncodedJSValue runOnResolveGroup(JSC::JSGlobalObject* globalObject, BunPlugin::Group& group, const WTF::String& pathString, const BunString* importer) { - Group* groupPtr = this->group(namespaceString ? namespaceString->toWTFString(BunString::ZeroCopy) : String()); - if (groupPtr == nullptr) { - return JSValue::encode(jsUndefined()); - } - Group& group = *groupPtr; auto& filters = group.filters; if (filters.size() == 0) { @@ -813,7 +808,6 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, cons auto& callbacks = group.callbacks; auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); - WTF::String pathString = path->toWTFString(BunString::ZeroCopy); JSC::MarkedArgumentBuffer matchedCallbacks; matchedCallbacks.ensureCapacity(filters.size()); @@ -845,11 +839,9 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, cons JSC::JSObject* paramsObject = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype(), 2); const auto& builtinNames = WebCore::builtinNames(vm); - auto* pathJS = Bun::toJS(globalObject, *path); - RETURN_IF_EXCEPTION(scope, {}); paramsObject->putDirect( vm, builtinNames.pathPublicName(), - pathJS); + jsString(vm, pathString)); auto* importerJS = Bun::toJS(globalObject, *importer); RETURN_IF_EXCEPTION(scope, {}); paramsObject->putDirect( @@ -898,6 +890,31 @@ EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, cons return JSValue::encode(JSC::jsUndefined()); } +EncodedJSValue BunPlugin::OnResolve::run(JSC::JSGlobalObject* globalObject, const BunString* namespaceString, const BunString* path, const BunString* importer) +{ + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); + + WTF::String nsString = namespaceString ? namespaceString->toWTFString(BunString::ZeroCopy) : String(); + WTF::String pathString = path->toWTFString(BunString::ZeroCopy); + + if (Group* groupPtr = this->group(nsString)) { + EncodedJSValue result = runOnResolveGroup(globalObject, *groupPtr, pathString, importer); + RETURN_IF_EXCEPTION(scope, {}); + if (!JSValue::decode(result).isUndefined()) { + RELEASE_AND_RETURN(scope, result); + } + } + + // Also offer the full "ns:path" to onResolve({ filter: /^ns:/ }) like Bun.build does. + if (!nsString.isEmpty() && !this->fileNamespace.filters.isEmpty()) { + WTF::String fullSpecifier = makeString(nsString, ":"_s, pathString); + RELEASE_AND_RETURN(scope, runOnResolveGroup(globalObject, this->fileNamespace, fullSpecifier, importer)); + } + + return JSValue::encode(JSC::jsUndefined()); +} + } // namespace Zig extern "C" JSC::EncodedJSValue Bun__runOnResolvePlugins(Zig::GlobalObject* globalObject, const BunString* namespaceString, const BunString* path, const BunString* from, BunPluginTarget target) diff --git a/src/jsc/bindings/JSBundlerPlugin.cpp b/src/jsc/bindings/JSBundlerPlugin.cpp index ca57cc2450dc..1be145bccb54 100644 --- a/src/jsc/bindings/JSBundlerPlugin.cpp +++ b/src/jsc/bindings/JSBundlerPlugin.cpp @@ -76,14 +76,8 @@ void BundlerPlugin::NamespaceList::append(JSC::VM& vm, JSC::RegExp* filter, Stri nsGroup->append(WTF::move(filter_regexp)); } -static bool anyMatchesForNamespace(JSC::VM& vm, BundlerPlugin::NamespaceList& list, BunString* namespaceStr, BunString* path) +static bool anyMatchesForNamespace(JSC::VM& vm, BundlerPlugin::NamespaceList& list, const String& namespaceString, const String& pathString) { - auto namespaceString = namespaceStr ? namespaceStr->transferToWTFString() : String(); - auto pathString = path->transferToWTFString(); - - if (list.fileNamespace.isEmpty() && list.namespaces.isEmpty()) - return false; - unsigned index = 0; auto* group = list.group(namespaceString, index); if (group == nullptr) { @@ -102,11 +96,31 @@ static bool anyMatchesForNamespace(JSC::VM& vm, BundlerPlugin::NamespaceList& li } bool BundlerPlugin::anyMatchesCrossThread(JSC::VM& vm, BunString* namespaceStr, BunString* path, bool isOnLoad) { - if (isOnLoad) { - return anyMatchesForNamespace(vm, this->onLoad, namespaceStr, path); - } else { - return anyMatchesForNamespace(vm, this->onResolve, namespaceStr, path); + auto namespaceString = namespaceStr ? namespaceStr->transferToWTFString() : String(); + auto pathString = path->transferToWTFString(); + + auto& list = isOnLoad ? this->onLoad : this->onResolve; + if (list.fileNamespace.isEmpty() && list.namespaces.isEmpty()) + return false; + + if (anyMatchesForNamespace(vm, list, namespaceString, pathString)) + return true; + + // onResolve: also offer "ns:rest" to the "ns" group with the stripped path. + if (!isOnLoad && (namespaceString.isEmpty() || namespaceString == "file"_s) && !list.namespaces.isEmpty()) { + if (auto colon = pathString.find(':'); colon != WTF::notFound && colon != 0) { +#if OS(WINDOWS) + if (colon == 1 && pathString.length() > 2 && isASCIIAlpha(pathString[0]) && (pathString[2] == '/' || pathString[2] == '\\')) + return false; +#endif + auto prefixNamespace = pathString.left(colon); + auto afterNamespace = pathString.substring(colon + 1); + if (anyMatchesForNamespace(vm, list, prefixNamespace, afterNamespace)) + return true; + } } + + return false; } static const HashTableValue JSBundlerPluginHashTable[] = { diff --git a/test/js/bun/plugin/plugin-onresolve-namespace-prefix.test.ts b/test/js/bun/plugin/plugin-onresolve-namespace-prefix.test.ts new file mode 100644 index 000000000000..b7f59efa7d87 --- /dev/null +++ b/test/js/bun/plugin/plugin-onresolve-namespace-prefix.test.ts @@ -0,0 +1,137 @@ +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// A plugin that registers onResolve either as +// (A) onResolve({ filter: /^virt:/ }) — default/file namespace, full specifier +// (B) onResolve({ filter: /.*/, namespace: "virt" }) — explicit namespace, stripped path +// must handle `import "virt:x"` in both the runtime module loader and Bun.build. +// Previously the runtime only consulted (B) and Bun.build only consulted (A), +// so a plugin written one way worked in one context and failed in the other. + +const fixture = (forms: "A" | "B" | "AB") => ` +const calls: string[] = []; +const setup = (b: any) => { +${ + forms.includes("A") + ? ` b.onResolve({ filter: /^virt:/ }, (args: any) => { + calls.push("A:" + args.path); + return { path: "a", namespace: "va" }; + }); + b.onLoad({ filter: /.*/, namespace: "va" }, () => ({ contents: 'export default "VIA-A";', loader: "js" })); +` + : "" +}${ + forms.includes("B") + ? ` b.onResolve({ filter: /.*/, namespace: "virt" }, (args: any) => { + calls.push("B:" + args.path); + return { path: "b", namespace: "vb" }; + }); + b.onLoad({ filter: /.*/, namespace: "vb" }, () => ({ contents: 'export default "VIA-B";', loader: "js" })); +` + : "" +}}; + +Bun.plugin({ name: "dual", setup }); +const runtime = (await import("virt:thing")).default; + +const fs = require("fs"); +const path = require("path"); +const entry = path.join(import.meta.dir, "entry.ts"); +fs.writeFileSync(entry, 'import v from "virt:thing"; export { v };'); +const r = await Bun.build({ entrypoints: [entry], plugins: [{ name: "dual", setup }] }); +if (!r.success) { + console.log(JSON.stringify({ ok: false, logs: r.logs.map((l: any) => l.message) })); + process.exit(1); +} +const bundled = (await r.outputs[0].text()).match(/VIA-[AB]/)![0]; +console.log(JSON.stringify({ ok: true, runtime, bundled, calls })); +`; + +async function run(forms: "A" | "B" | "AB") { + using dir = tempDir("plugin-onresolve-ns", { + "index.ts": fixture(forms), + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +describe.concurrent("onResolve namespace-prefix dispatch is consistent between runtime and Bun.build", () => { + test('onResolve({ filter: /^virt:/ }) resolves `import "virt:x"` at runtime and in Bun.build', async () => { + const { stdout, stderr, exitCode } = await run("A"); + expect(stderr).toBe(""); + const out = JSON.parse(stdout); + expect(out.ok).toBe(true); + expect(out.runtime).toBe("VIA-A"); + expect(out.bundled).toBe("VIA-A"); + // Callback receives the full specifier in this form. + expect(out.calls).toContain("A:virt:thing"); + expect(exitCode).toBe(0); + }); + + test('onResolve({ namespace: "virt" }) resolves `import "virt:x"` at runtime and in Bun.build', async () => { + const { stdout, stderr, exitCode } = await run("B"); + expect(stderr).toBe(""); + const out = JSON.parse(stdout); + expect(out.ok).toBe(true); + expect(out.runtime).toBe("VIA-B"); + expect(out.bundled).toBe("VIA-B"); + // Callback receives the path with the "virt:" prefix stripped in this form. + expect(out.calls).toContain("B:thing"); + expect(exitCode).toBe(0); + }); + + test("registering both forms keeps each context's existing primary lookup", async () => { + const { stdout, stderr, exitCode } = await run("AB"); + expect(stderr).toBe(""); + const out = JSON.parse(stdout); + expect(out.ok).toBe(true); + expect(out.runtime).toBe("VIA-B"); + expect(out.bundled).toBe("VIA-A"); + expect(out.calls).toContain("B:thing"); + expect(out.calls).toContain("A:virt:thing"); + expect(exitCode).toBe(0); + }); + + test('onResolve({ namespace: "virt" }) returning { path } without namespace resolves to a file on disk in both', async () => { + using dir = tempDir("plugin-onresolve-ns-file", { + "real.js": `export default "FROM-DISK";`, + "entry.ts": `import v from "virt:thing"; export { v };`, + "index.ts": ` + const path = require("path"); + const real = path.join(import.meta.dir, "real.js"); + const setup = (b: any) => { + b.onResolve({ filter: /.*/, namespace: "virt" }, () => ({ path: real })); + }; + Bun.plugin({ name: "to-file", setup }); + const runtime = (await import("virt:thing")).default; + const r = await Bun.build({ + entrypoints: [path.join(import.meta.dir, "entry.ts")], + plugins: [{ name: "to-file", setup }], + }); + if (!r.success) { + console.log(JSON.stringify({ ok: false, logs: r.logs.map((l: any) => l.message) })); + process.exit(1); + } + const bundled = (await r.outputs[0].text()).includes("FROM-DISK") ? "FROM-DISK" : "?"; + console.log(JSON.stringify({ ok: true, runtime, bundled })); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "index.ts"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const out = JSON.parse(stdout); + expect(out).toEqual({ ok: true, runtime: "FROM-DISK", bundled: "FROM-DISK" }); + expect(exitCode).toBe(0); + }); +});