Skip to content

Conversation

@tconley1428
Copy link

@tconley1428 tconley1428 commented Oct 6, 2025

What was changed

Add plugin types for Connection, NativeConnection, Worker, Client, and Bundler, as well as a SimplePlugin class for easy construction of new plugins.

Why?

Plugins make it easier to share common configurations and apply them consistently.

Checklist

  1. Closes [Feature Request] Plugin support #1764

  2. How was this tested:

  1. Any docs updates needed?

@tconley1428 tconley1428 marked this pull request as ready for review October 17, 2025 16:37
@tconley1428 tconley1428 requested a review from a team as a code owner October 17, 2025 16:37
Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this design makes sense to me.

// Process plugins first to allow them to modify connect configuration
for (const plugin of options?.plugins ?? []) {
if (plugin.configureClient !== undefined) {
options = plugin.configureClient(options);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we ever expect/support a plugin altering the plugin list?

I'm not sure I can think of a reason for doing this, but the current implementation would allow altering the list, but the iteration wouldn't change. e.g. a removed plugin can still alter the options/added plugin will not get a chance to alter the options

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't intend to support that. If you have an idea of how to enforce that, we could.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could leverage Omit to make it clear that plugins won't be respected e.g. configureClient?(options: ClientOptions): Omit<ClientOptions, 'plugins'>

Running the configurations becomes a little more awkward as you have to destructure options each iteration e.g.

    // Add client plugins from the connection
    options.plugins = (options.plugins ?? []).concat(options.connection?.plugins ?? []);
    let { plugins, ...optionsWithoutPlugins } = options;

    // Process plugins first to allow them to modify connect configuration
    for (const plugin of options.plugins) {
      if (plugin.configureClient !== undefined) {
        optionsWithoutPlugins = plugin.configureClient({...optionsWithoutPlugins, plugins });
      }
    }
    options = { ...optionsWithoutPlugins, plugins };

    super(options);

It doesn't warn/error if the user tries to alter the plugin list, but any changes they make won't be respected.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omit is a good idea. It seems like I can just change the type in the plugin to Omit<ClientOptions, "plugins"> without any other change. They can return something with changed plugins, but expresses the intent

@tconley1428 tconley1428 changed the title Plugins/initial Plugins Implementation Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Plugin support

2 participants