Merge branch 'main' of github.com:oven-sh/bun into don/feat/plugin-factories

packages/bun-types/bun.d.ts

@@ -2633,7 +2633,7 @@ declare module "bun" {
         }; // | string;
     root?: string; // project root
     splitting?: boolean; // default true, enable code splitting
-    plugins?: BunPlugin[];
+    plugins?: BunPluginFactory[];
     // manifest?: boolean; // whether to return manifest
     external?: string[];
     packages?: "bundle" | "external";
@@ -5371,6 +5371,8 @@ declare module "bun" {
   /** https://bun.sh/docs/bundler/loaders */
   type Loader = "js" | "jsx" | "ts" | "tsx" | "json" | "toml" | "file" | "napi" | "wasm" | "text" | "css" | "html";
 
+  type MaybePromise<T> = T | Promise<T>;
+
   interface PluginConstraints {
     /**
      * Only apply the plugin when the import specifier matches this regular expression
@@ -5466,8 +5468,8 @@ declare module "bun" {
   }
 
   type OnLoadResult = OnLoadResultSourceCode | OnLoadResultObject | undefined | void;
-  type OnLoadCallback = (args: OnLoadArgs) => OnLoadResult | Promise<OnLoadResult>;
-  type OnStartCallback = () => void | Promise<void>;
+  type OnLoadCallback = (args: OnLoadArgs) => MaybePromise<OnLoadResult>;
+  type OnStartCallback = () => MaybePromise<void>;
 
   interface OnResolveArgs {
     /**
@@ -5511,9 +5513,7 @@ declare module "bun" {
     external?: boolean;
   }
 
-  type OnResolveCallback = (
-    args: OnResolveArgs,
-  ) => OnResolveResult | Promise<OnResolveResult | undefined | null> | undefined | null;
+  type OnResolveCallback = (args: OnResolveArgs) => MaybePromise<OnResolveResult | undefined | null>;
 
   type FFIFunctionCallable = Function & {
     // Making a nominally typed function so that the user must get it from dlopen
@@ -5613,7 +5613,7 @@ declare module "bun" {
      *
      * @returns `this` for method chaining
      */
-    module(specifier: string, callback: () => OnLoadResult | Promise<OnLoadResult>): this;
+    module(specifier: string, callback: () => MaybePromise<OnLoadResult>): this;
   }
 
   interface BunPlugin {
@@ -5655,6 +5655,8 @@ declare module "bun" {
     ): void | Promise<void>;
   }
 
+  type BunPluginFactory<P extends BunPlugin = BunPlugin> = P | (() => P);
+
   /**
    * Extend Bun's module resolution and loading behavior
    *
@@ -5694,7 +5696,7 @@ declare module "bun" {
    * ```
    */
   interface BunRegisterPlugin {
-    <T extends BunPlugin>(options: T): ReturnType<T["setup"]>;
+    <T extends BunPlugin>(options: BunPluginFactory<T>): ReturnType<T["setup"]>;
 
     /**
      * Deactivate all plugins

src/bake/bake.zig

@@ -111,23 +111,9 @@ pub const SplitBundlerOptions = struct {
 
         var iter = plugin_array.arrayIterator(global);
         while (iter.next()) |plugin_config| {
-            if (!plugin_config.isObject()) {
-                return global.throwInvalidArguments("Expected plugin to be an object", .{});
-            }
-
-            if (try plugin_config.getOptional(global, "name", ZigString.Slice)) |slice| {
-                defer slice.deinit();
-                if (slice.len == 0) {
-                    return global.throwInvalidArguments("Expected plugin to have a non-empty name", .{});
-                }
-            } else {
-                return global.throwInvalidArguments("Expected plugin to have a name", .{});
-            }
-
-            const function = try plugin_config.getFunction(global, "setup") orelse {
-                return global.throwInvalidArguments("Expected plugin to have a setup() function", .{});
-            };
-            const plugin_result = try plugin.addPlugin(function, empty_object, .null, false, true);
+            const jsplugin = try Plugin.JS.fromJS(global, plugin_config);
+            const plugin_obj = try jsplugin.toObject(global);
+            const plugin_result = try plugin.addPlugin(plugin_obj.setup, empty_object, .null, false, true);
             if (plugin_result.asAnyPromise()) |promise| {
                 promise.setHandled(global.vm());
                 // TODO: remove this call, replace with a promise list that must

src/bun.js/api/JSBundler.zig

@@ -113,22 +113,9 @@ pub const JSBundler = struct {
                 var onstart_promise_array: JSValue = JSValue.undefined;
                 var i: usize = 0;
                 while (iter.next()) |plugin| : (i += 1) {
-                    if (!plugin.isObject()) {
-                        return globalThis.throwInvalidArguments("Expected plugin to be an object", .{});
-                    }
+                    const jsplugin = try (try Plugin.JS.fromJS(globalThis, plugin)).toObject(globalThis);
 
-                    if (try plugin.getOptional(globalThis, "name", ZigString.Slice)) |slice| {
-                        defer slice.deinit();
-                        if (slice.len == 0) {
-                            return globalThis.throwInvalidArguments("Expected plugin to have a non-empty name", .{});
-                        }
-                    } else {
-                        return globalThis.throwInvalidArguments("Expected plugin to have a name", .{});
-                    }
-
-                    const function = try plugin.getFunction(globalThis, "setup") orelse {
-                        return globalThis.throwInvalidArguments("Expected plugin to have a setup() function", .{});
-                    };
+                    const function = jsplugin.setup;
 
                     var bun_plugins: *Plugin = plugins.* orelse brk: {
                         plugins.* = Plugin.create(
@@ -875,6 +862,70 @@ pub const JSBundler = struct {
     };
 
     pub const Plugin = opaque {
+        pub const JS = union(enum) {
+            factory: JSC.JSValue,
+            object: Object,
+
+            pub const Object = struct {
+                name: JSC.JSValue,
+                setup: JSC.JSValue,
+
+                fn fromJS(global: *JSC.JSGlobalObject, value: JSC.JSValue) bun.JSError!Object {
+                    if (comptime bun.Environment.allow_assert) {
+                        bun.assertWithLocation(value.isObject(), @src());
+                    }
+
+                    // plugin.name is a non-empty string
+                    const name = try value.get(global, "name") orelse
+                        return global.throwInvalidArguments("Expected plugin to have a name", .{});
+                    if (!name.isString()) {
+                        const ty = name.jsTypeString(global);
+                        return global.throwInvalidArguments("Expected plugin name to be a string, got '{}'", .{ty});
+                    }
+                    if (name.getLength(global) == 0)
+                        return global.throwInvalidArguments("Expected plugin name to be a non-empty string", .{});
+
+                    // plugin.setup(builder)
+                    const setup = try value.getFunction(global, "setup") orelse {
+                        return global.throwInvalidArguments("Expected plugin to have a setup() function", .{});
+                    };
+
+                    return Plugin.JS.Object{
+                        .name = name,
+                        .setup = setup,
+                    };
+                }
+            };
+
+            pub fn fromJS(global: *JSC.JSGlobalObject, value: JSC.JSValue) bun.JSError!Plugin.JS {
+                return if (value.isObject())
+                    Plugin.JS{ .object = try Object.fromJS(global, value) }
+                else if (value.isFunction())
+                    Plugin.JS{ .factory = value }
+                else err: {
+                    const ty = value.jsTypeString(global);
+                    break :err global.throwInvalidArguments("Expected plugin to be an object or a function, got '{}'", .{ty});
+                };
+            }
+
+            pub fn toObject(this: *const JS, global: *JSC.JSGlobalObject) bun.JSError!Plugin.JS.Object {
+                switch (this.*) {
+                    .object => |obj| return obj,
+                    .factory => |factory| {
+                        const result = try factory.call(global, global.toJSValue(), &[_]JSValue{});
+                        if (!result.isObject()) {
+                            const ty = result.jsTypeString(global);
+                            return global.throwTypeError("Expected plugin factory to return an object, got '{}'", .{ty});
+                        }
+                        if (result.asPromise()) |_| {
+                            return global.throwTypeError("Plugin factories cannot be async yet. Please move async logic into the setup() function.", .{});
+                        }
+                        return Plugin.JS.Object.fromJS(global, result);
+                    },
+                }
+            }
+        };
+
         extern fn JSBundlerPlugin__create(*JSC.JSGlobalObject, JSC.JSGlobalObject.BunPluginTarget) *Plugin;
         pub fn create(global: *JSC.JSGlobalObject, target: JSC.JSGlobalObject.BunPluginTarget) *Plugin {
             JSC.markBinding(@src());

src/bun.js/bindings/BunPlugin.cpp

@@ -260,31 +260,20 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionAppendOnResolvePluginBrowser, (JSC::JSGlobalO
     return jsFunctionAppendOnResolvePluginGlobal(globalObject, callframe, BunPluginTargetBrowser);
 }
 
-/// `Bun.plugin()`
-static inline JSC::EncodedJSValue setupBunPlugin(JSC::JSGlobalObject* globalObject, JSC::CallFrame* callframe, BunPluginTarget target)
+/// `Bun.plugin(options)`
+static inline JSC::JSValue setupBunPlugin(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSObject* options, BunPluginTarget target)
 {
-    auto& vm = JSC::getVM(globalObject);
+    ASSERT(options);
     auto throwScope = DECLARE_THROW_SCOPE(vm);
-    if (callframe->argumentCount() < 1) {
-        JSC::throwTypeError(globalObject, throwScope, "plugin needs at least one argument (an object)"_s);
-        return {};
-    }
 
-    JSC::JSObject* obj = callframe->uncheckedArgument(0).getObject();
-    if (!obj) {
-        JSC::throwTypeError(globalObject, throwScope, "plugin needs an object as first argument"_s);
-        return {};
-    }
-    RETURN_IF_EXCEPTION(throwScope, {});
-
-    JSC::JSValue setupFunctionValue = obj->getIfPropertyExists(globalObject, Identifier::fromString(vm, "setup"_s));
+    JSC::JSValue setupFunctionValue = options->getIfPropertyExists(globalObject, Identifier::fromString(vm, "setup"_s));
     RETURN_IF_EXCEPTION(throwScope, {});
     if (!setupFunctionValue || setupFunctionValue.isUndefinedOrNull() || !setupFunctionValue.isCell() || !setupFunctionValue.isCallable()) {
         JSC::throwTypeError(globalObject, throwScope, "plugin needs a setup() function"_s);
         return {};
     }
 
-    if (JSValue targetValue = obj->getIfPropertyExists(globalObject, Identifier::fromString(vm, "target"_s))) {
+    if (JSValue targetValue = options->getIfPropertyExists(globalObject, Identifier::fromString(vm, "target"_s))) {
         if (auto* targetJSString = targetValue.toStringOrNull(globalObject)) {
             String targetString = targetJSString->value(globalObject);
             if (!(targetString == "node"_s || targetString == "bun"_s || targetString == "browser"_s)) {
@@ -336,10 +325,37 @@ static inline JSC::EncodedJSValue setupBunPlugin(JSC::JSGlobalObject* globalObje
     RETURN_IF_EXCEPTION(throwScope, {});
 
     if (auto* promise = JSC::jsDynamicCast<JSC::JSPromise*>(result)) {
-        RELEASE_AND_RETURN(throwScope, JSValue::encode(promise));
+        RELEASE_AND_RETURN(throwScope, promise);
     }
 
-    RELEASE_AND_RETURN(throwScope, JSValue::encode(jsUndefined()));
+    RELEASE_AND_RETURN(throwScope, jsUndefined());
+}
+
+/// `Bun.plugin(optionsFactory)`
+static inline JSC::JSValue setupBunPlugin(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSFunction* makeOptions, BunPluginTarget target)
+{
+    ASSERT(makeOptions);
+
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    JSC::MarkedArgumentBuffer argList;
+
+    auto callData = getCallData(makeOptions);
+    JSC::JSValue ret = JSC::call(globalObject, JSValue(makeOptions), callData, JSValue(globalObject), argList);
+    RETURN_IF_EXCEPTION(throwScope, {});
+
+    JSC::JSObject* options = ret.getObject();
+    if (!options) {
+        JSC::throwTypeError(globalObject, throwScope, "plugin factory must return an object."_s);
+        return {};
+    }
+
+    // TODO: support async plugin factories. Emit a better error message than
+    // just "setup() function is missing".
+    if (auto* promise = JSC::jsDynamicCast<JSC::JSPromise*>(options)) {
+        JSC::throwTypeError(globalObject, throwScope, "plugin() does not support async plugin factories yet."_s);
+        return {};
+    }
+    RELEASE_AND_RETURN(throwScope, Bun::setupBunPlugin(vm, globalObject, options, target));
 }
 
 void BunPlugin::Group::append(JSC::VM& vm, JSC::RegExp* filter, JSC::JSObject* func)
@@ -941,7 +957,35 @@ BUN_DEFINE_HOST_FUNCTION(jsFunctionBunPluginClear, (JSC::JSGlobalObject * global
     return JSC::JSValue::encode(JSC::jsUndefined());
 }
 
+/// `Bun.plugin(options: BunPlugin | () => BunPlugin)`
 BUN_DEFINE_HOST_FUNCTION(jsFunctionBunPlugin, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callframe))
 {
-    return Bun::setupBunPlugin(globalObject, callframe, BunPluginTargetBun);
+    auto& vm = JSC::getVM(globalObject);
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+    if (callframe->argumentCount() < 1) {
+        JSC::throwTypeError(globalObject, throwScope, "plugin needs at least one argument (an object)"_s);
+        return {};
+    }
+
+    JSC::JSObject* obj = callframe->uncheckedArgument(0).getObject();
+    if (!obj) {
+        JSC::throwTypeError(globalObject, throwScope, "plugin needs an object or function as first argument"_s);
+        return {};
+    }
+    RETURN_IF_EXCEPTION(throwScope, {});
+
+    JSC::JSValue result;
+
+    if (auto* function = JSC::jsDynamicCast<JSC::JSFunction*>(obj)) {
+        if (function->isConstructor() || !function->isCallable()) {
+            JSC::throwTypeError(globalObject, throwScope, "plugin factories cannot be classes. Please use an arrow or named function instead."_s);
+            return {};
+        }
+        result = Bun::setupBunPlugin(vm, globalObject, jsCast<JSC::JSFunction*>(obj), BunPluginTargetBun);
+    } else {
+        result = Bun::setupBunPlugin(vm, globalObject, obj, BunPluginTargetBun);
+    }
+    RETURN_IF_EXCEPTION(throwScope, {});
+
+    RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(result));
 }

src/bun.js/bindings/bindings.zig

@@ -3063,6 +3063,10 @@ pub const JSGlobalObject = opaque {
         }
     }
 
+    pub inline fn throwTypeError(this: *JSGlobalObject, comptime fmt: [:0]const u8, args: anytype) bun.JSError {
+        return this.throwValue(this.createTypeErrorInstance(fmt, args));
+    }
+
     pub fn createTypeErrorInstance(this: *JSGlobalObject, comptime fmt: [:0]const u8, args: anytype) JSValue {
         if (comptime std.meta.fieldNames(@TypeOf(args)).len > 0) {
             var stack_fallback = std.heap.stackFallback(1024 * 4, this.allocator());

src/js/builtins/BundlerPlugin.ts

@@ -76,10 +76,17 @@ export function loadAndResolvePluginsForServe(
       if (!pluginModuleRaw || !pluginModuleRaw.default) {
         throw new TypeError(`Expected "${plugins[i]}" to be a module which default exports a bundler plugin.`);
       }
-      let pluginModule = pluginModuleRaw.default;
+      let pluginModule: BunPlugin | (() => BunPlugin) = pluginModuleRaw.default;
+      $assert(pluginModule); // checked above
+
+      if (typeof pluginModule === "function") {
+        pluginModule = pluginModule.$call(globalThis);
+      }
+
       if (!pluginModule || pluginModule.name === undefined || pluginModule.setup === undefined) {
         throw new TypeError(`"${plugins[i]}" is not a valid bundler plugin.`);
       }
+
       onstart_promises_array = await runSetupFn.$apply(bundlerPlugin, [
         pluginModule.setup,
         config,

test/bake/dev/dev-plugins.test.ts

@@ -33,6 +33,7 @@ devTest("onResolve", {
     await dev.fetch("/").equals("value: 1");
   },
 });
+
 devTest("onLoad", {
   framework: minimalFramework,
   pluginFile: `
@@ -66,6 +67,7 @@ devTest("onLoad", {
     await dev.fetch("/").equals("value: 1");
   },
 });
+
 devTest("onResolve + onLoad virtual file", {
   framework: minimalFramework,
   pluginFile: `
@@ -110,6 +112,55 @@ devTest("onResolve + onLoad virtual file", {
     ]);
   },
 });
+
+describe("plugin options", () => {
+  const files = {
+    "trigger.ts": `
+      throw new Error('should not be loaded');
+    `,
+    "routes/index.ts": `
+      import { value } from '../trigger.ts';
+
+      export default function (req, meta) {
+        return new Response('value: ' + value);
+      }
+    `,
+  };
+
+  devTest("onLoad", {
+    framework: minimalFramework,
+    pluginFile: `
+    import * as path from 'path';
+    export default [
+      {
+        name: 'a',
+        setup(build) {
+          build.onLoad({ filter: /trigger/ }, (args) => {
+            return { contents: 'export const value = 1;', loader: 'ts' };
+          });
+        },
+      }
+    ];
+  `,
+    files: {
+      "trigger.ts": `
+      throw new Error('should not be loaded');
+    `,
+      "routes/index.ts": `
+      import { value } from '../trigger.ts';
+
+      export default function (req, meta) {
+        return new Response('value: ' + value);
+      }
+    `,
+    },
+    async test(dev) {
+      await dev.fetch("/").equals("value: 1");
+      await dev.fetch("/").equals("value: 1");
+      await dev.fetch("/").equals("value: 1");
+    },
+  });
+});
 // devTest("onLoad with watchFile", {
 //   framework: minimalFramework,
 //   pluginFile: `

test/bundler/bun-build-api.test.ts

@@ -1,5 +1,6 @@
-import { describe, expect, test } from "bun:test";
-import { readFileSync, writeFileSync } from "fs";
+import type { BunPlugin } from "bun";
+import { describe, beforeAll, afterAll, expect, test, it } from "bun:test";
+import { readFileSync, writeFileSync, rmSync } from "fs";
 import { bunEnv, bunExe, tempDirWithFiles } from "harness";
 import path, { join } from "path";
 import assert from "assert";
@@ -611,6 +612,70 @@ describe("Bun.build", () => {
     const html = build.outputs.find(o => o.type === "text/html;charset=utf-8");
     expect(await html?.text()).toContain("<meta name='injected-by-plugin' content='true'>");
   });
+
+  describe("plugin options", () => {
+    let fixture: string;
+    let index: string;
+
+    function coffeePlugin(): BunPlugin {
+      return {
+        name: "coffee",
+        setup(build) {
+          build.onLoad({ filter: /\.coffee$/ }, () => {
+            return {
+              contents: "module.exports = 'world'",
+              loader: "js",
+            };
+          });
+        },
+      };
+    }
+
+    beforeAll(() => {
+      fixture = tempDirWithFiles("build-plugins-factory", {
+        "index.ts": `
+        import foo from "./foo.coffee";
+        console.log(foo)
+      `,
+        "foo.coffee": `
+        module.exports = "hello"
+      `,
+      });
+
+      index = join(fixture, "index.ts");
+    });
+
+    afterAll(() => {
+      rmSync(fixture, { recursive: true, force: true });
+    });
+
+    it("can be a BunPlugin object", async () => {
+      const build = await Bun.build({
+        entrypoints: [index],
+        plugins: [coffeePlugin()],
+      });
+
+      expect(build.success).toBeTrue();
+    });
+
+    it("can be a function that returns a BunPlugin object", async () => {
+      const build = await Bun.build({
+        entrypoints: [index],
+        plugins: [coffeePlugin],
+      });
+      expect(build.success).toBeTrue();
+    });
+
+    it("cannot be async (for now)", async () => {
+      expect(async () => {
+        await Bun.build({
+          entrypoints: [index],
+          // @ts-expect-error
+          plugins: [async () => coffeePlugin()],
+        });
+      }).toThrow(/does not support async plugin/);
+    });
+  });
 });
 
 test("onEnd Plugin does not crash", async () => {

test/js/bun/plugin/plugins.test.ts

@@ -186,6 +186,21 @@ plugin({
   },
 });
 
+plugin(() => ({
+  name: "plugin created by function",
+  setup(builder) {
+    builder
+      .onResolve({ filter: /.*/, namespace: "factory-sync" }, ({ path }) => ({
+        namespace: "factory-sync",
+        path,
+      }))
+      .onLoad({ filter: /.*/, namespace: "factory-sync" }, ({ path }) => ({
+        contents: `// ${path}\n\nexport default 42;`,
+        loader: "js",
+      }));
+  },
+}));
+
 // This is to test that it works when imported from a separate file
 import "../../third_party/svelte";
 import "./module-plugins";
@@ -510,3 +525,37 @@ it("import(...) without __esModule", async () => {
   const { default: mod } = await import("my-virtual-module-with-default");
   expect(mod).toBe("world");
 });
+
+describe("plugin factories", () => {
+  it("can be synchronous", async () => {
+    const result = await import("factory-sync:my-file");
+    expect(result.default).toBe(42);
+  });
+
+  it("must be callable", () => {
+    class Foo {
+      static setup() {}
+    }
+    expect(() => plugin(Foo)).toThrow(/factories cannot be classes/);
+  });
+
+  it("cannot be asynchronous (yet)", async () => {
+    expect(() => {
+      // @ts-expect-error
+      plugin(async () => ({
+        name: "plugin created by async function",
+        setup(builder) {
+          builder
+            .onResolve({ filter: /.*/, namespace: "factory-async" }, ({ path }) => ({
+              namespace: "factory-async",
+              path,
+            }))
+            .onLoad({ filter: /.*/, namespace: "factory-async" }, ({ path }) => ({
+              contents: `// ${path}\n\nexport default 42;`,
+              loader: "js",
+            }));
+        },
+      }));
+    }).toThrow(/does not support async plugin factories/);
+  });
+});