From 805a90dbcc7ddae5683b2a67e4448f044cfb91eb Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Fri, 22 Aug 2025 22:47:08 +0000 Subject: [PATCH] Implement build.onEnd plugin hook functionality - Add onEnd callback registration and execution to BundlerPlugin - Implement C++ bridge in Bun__runOnEndPlugins to call JavaScript callbacks - Store onEnd callbacks per plugin instance and execute on build completion - Convert build logs to esbuild-compatible errors/warnings format - Add comprehensive tests for onEnd validation and execution - Support multiple plugins with onEnd callbacks --- src/bun.js/bindings/BunPlugin.cpp | 32 ++++++- src/js/builtins/BundlerPlugin.ts | 132 +++++++++++++++-------------- test/bundler/bun-build-api.test.ts | 74 +++++++++++++++- 3 files changed, 171 insertions(+), 67 deletions(-) diff --git a/src/bun.js/bindings/BunPlugin.cpp b/src/bun.js/bindings/BunPlugin.cpp index a87879d191..e120dac440 100644 --- a/src/bun.js/bindings/BunPlugin.cpp +++ b/src/bun.js/bindings/BunPlugin.cpp @@ -862,10 +862,34 @@ extern "C" JSC::EncodedJSValue Bun__runOnLoadPlugins(Zig::GlobalObject* globalOb extern "C" JSC::EncodedJSValue Bun__runOnEndPlugins(Zig::GlobalObject* globalObject, JSC::EncodedJSValue buildResult) { - // For now, let's just return undefined to confirm the basic flow works - // The actual implementation will need to find the correct plugin instance - // and call runOnEndPlugins with the build result - return JSC::JSValue::encode(JSC::jsUndefined()); + JSC::VM& vm = globalObject->vm(); + JSC::JSLockHolder lock(vm); + + // Get the current active plugin instance from the global object + JSC::JSValue activePluginInstance = globalObject->get(globalObject, JSC::Identifier::fromString(vm, "activePluginInstance")); + if (activePluginInstance.isUndefined() || !activePluginInstance.isObject()) { + return JSC::JSValue::encode(JSC::jsUndefined()); + } + + // Get the runOnEndPlugins function from the active plugin instance + JSC::JSObject* pluginObject = activePluginInstance.getObject(); + JSC::JSValue runOnEndFunction = pluginObject->get(globalObject, JSC::Identifier::fromString(vm, "runOnEndPlugins")); + + if (runOnEndFunction.isUndefined() || !runOnEndFunction.isCallable()) { + return JSC::JSValue::encode(JSC::jsUndefined()); + } + + // Call runOnEndPlugins with the build result + JSC::CallData callData = JSC::getCallData(runOnEndFunction); + if (callData.type == JSC::CallData::Type::None) { + return JSC::JSValue::encode(JSC::jsUndefined()); + } + + JSC::MarkedArgumentBuffer args; + args.append(JSC::JSValue::decode(buildResult)); + + JSC::JSValue result = JSC::call(globalObject, runOnEndFunction, callData, activePluginInstance, args); + return JSC::JSValue::encode(result); } namespace Bun { diff --git a/src/js/builtins/BundlerPlugin.ts b/src/js/builtins/BundlerPlugin.ts index 2e55fe6517..b93f1f0894 100644 --- a/src/js/builtins/BundlerPlugin.ts +++ b/src/js/builtins/BundlerPlugin.ts @@ -21,6 +21,7 @@ interface BundlerPlugin { generateDeferPromise(id: number): Promise; promises: Array> | undefined; onEndCallbacks: Array<(result: any) => void | Promise> | undefined; + runOnEndPlugins?: (buildResult: any) => void; onBeforeParse: (filter: RegExp, namespace: string, addon: unknown, symbol: string, external?: unknown) => void; $napiDlopenHandle: number; @@ -123,6 +124,75 @@ export function runSetupFunction( isBake: boolean, ): Promise[]> | Promise[] | undefined { this.promises = promises; + + // Register this plugin instance globally so C++ can find it for onEnd callbacks + globalThis.activePluginInstance = this; + + // Add the runOnEndPlugins method to this instance + if (!this.runOnEndPlugins) { + this.runOnEndPlugins = function(buildResult) { + const { onEndCallbacks } = this; + if (!onEndCallbacks || onEndCallbacks.length === 0) { + return; + } + + // Convert logs to errors/warnings arrays for esbuild compatibility + const logs = buildResult.logs || []; + const errors = []; + const warnings = []; + + for (const log of logs) { + if (log.level === "error") { + errors.push({ + text: log.message || "", + location: log.position + ? { + file: log.position.file || "", + line: log.position.line || 0, + column: log.position.column || 0, + } + : null, + notes: [], + detail: undefined, + }); + } else if (log.level === "warning") { + warnings.push({ + text: log.message || "", + location: log.position + ? { + file: log.position.file || "", + line: log.position.line || 0, + column: log.position.column || 0, + } + : null, + notes: [], + detail: undefined, + }); + } + } + + // Create esbuild-compatible result object + const onEndResult = { + errors, + warnings, + }; + + // Execute onEnd callbacks serially as per esbuild specification + for (const callback of onEndCallbacks) { + try { + const result = callback(onEndResult); + if ($isPromise(result)) { + // For now, we can't easily await promises in the bundler completion + // We'll handle this synchronously + console.warn("onEnd callback returned a promise, but async onEnd is not fully supported yet"); + } + } catch (error) { + // Log the error but don't fail the build + console.error("onEnd callback error:", error); + } + } + }; + } var onLoadPlugins = new Map(); var onResolvePlugins = new Map(); var onBeforeParsePlugins = new Map< @@ -458,68 +528,6 @@ export function runOnResolvePlugins(this: BundlerPlugin, specifier, inputNamespa } } -export function runOnEndPlugins(this: BundlerPlugin, buildResult) { - const { onEndCallbacks } = this; - if (!onEndCallbacks || onEndCallbacks.length === 0) { - return; - } - - // Convert logs to errors/warnings arrays for esbuild compatibility - const logs = buildResult.logs || []; - const errors = []; - const warnings = []; - - for (const log of logs) { - if (log.level === "error") { - errors.push({ - text: log.message || "", - location: log.position - ? { - file: log.position.file || "", - line: log.position.line || 0, - column: log.position.column || 0, - } - : null, - notes: [], - detail: undefined, - }); - } else if (log.level === "warning") { - warnings.push({ - text: log.message || "", - location: log.position - ? { - file: log.position.file || "", - line: log.position.line || 0, - column: log.position.column || 0, - } - : null, - notes: [], - detail: undefined, - }); - } - } - - // Create esbuild-compatible result object - const onEndResult = { - errors, - warnings, - }; - - // Execute onEnd callbacks serially as per esbuild specification - for (const callback of onEndCallbacks) { - try { - const result = callback(onEndResult); - if ($isPromise(result)) { - // For now, we can't easily await promises in the bundler completion - // We'll handle this synchronously - console.warn("onEnd callback returned a promise, but async onEnd is not fully supported yet"); - } - } catch (error) { - // Log the error but don't fail the build - console.error("onEnd callback error:", error); - } - } -} export function runOnLoadPlugins( this: BundlerPlugin, diff --git a/test/bundler/bun-build-api.test.ts b/test/bundler/bun-build-api.test.ts index 8eddae5698..42e99c1824 100644 --- a/test/bundler/bun-build-api.test.ts +++ b/test/bundler/bun-build-api.test.ts @@ -630,7 +630,79 @@ test("onEnd Plugin does not crash", async () => { ], }); })(), - ).rejects.toThrow("On-end callbacks is not implemented yet. See https://github.com/oven-sh/bun/issues/2771"); + ).rejects.toThrow("callback must be a function"); +}); + +test("onEnd Plugin executes callback", async () => { + const dir = tempDirWithFiles("onEnd", { + "entry.js": ` + console.log("Hello from onEnd test"); + export default "test"; + `, + }); + + let onEndCalled = false; + + await Bun.build({ + entrypoints: [join(dir, "entry.js")], + plugins: [ + { + name: "plugin", + setup(build) { + build.onEnd((result) => { + onEndCalled = true; + expect(result).toHaveProperty("errors"); + expect(result).toHaveProperty("warnings"); + expect(Array.isArray(result.errors)).toBe(true); + expect(Array.isArray(result.warnings)).toBe(true); + }); + }, + }, + ], + }); + + expect(onEndCalled).toBe(true); +}); + +test("onEnd Plugin handles multiple callbacks", async () => { + const dir = tempDirWithFiles("onEnd-multi", { + "entry.js": ` + console.log("Multiple callbacks test"); + export default "test"; + `, + }); + + let firstCalled = false; + let secondCalled = false; + + await Bun.build({ + entrypoints: [join(dir, "entry.js")], + plugins: [ + { + name: "plugin1", + setup(build) { + build.onEnd((result) => { + firstCalled = true; + expect(result).toHaveProperty("errors"); + expect(result).toHaveProperty("warnings"); + }); + }, + }, + { + name: "plugin2", + setup(build) { + build.onEnd((result) => { + secondCalled = true; + expect(result).toHaveProperty("errors"); + expect(result).toHaveProperty("warnings"); + }); + }, + }, + ], + }); + + expect(firstCalled).toBe(true); + expect(secondCalled).toBe(true); }); test("macro with nested object", async () => {