Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaScript API InlineConfig Loader/Callback/Provider #17543

Open
4 tasks done
Shakeskeyboarde opened this issue Jun 21, 2024 · 2 comments
Open
4 tasks done

JavaScript API InlineConfig Loader/Callback/Provider #17543

Shakeskeyboarde opened this issue Jun 21, 2024 · 2 comments

Comments

@Shakeskeyboarde
Copy link
Contributor

Shakeskeyboarde commented Jun 21, 2024

Description

Problem: Reusing resolved configuration (specifically plugins) in a "child command" will result in unexpected plugin behavior if the "parent command" was invoked using the JavaScript API with an inline config (InlineConfig) that includes plugins. A child command is used here to mean a Vite command, called from the JavaScript API, invoked by a plugin hook, which was invoked by the parent command.

I encountered this while writing the vite-live-preview plugin that runs the JavaScript API preview() command in a plugin closeBundle hook. It wants to inherit the configuration from the build command that uses the plugin.

In my plugin, the preview() command should use the same configuration as the build command. To achieve this, the plugin configResolved hook captures the resolved inlineConfig and configFile values. Those values are used as the seed for the preview() command config.

// simplified plugin example
export default (): Plugin => {
  let enabled = true;
  let inlineConfig: InlineConfig = {};
  let configFile: string | false = false;
  let previewServer: PreviewServer | undefined;

  return {
    name: 'example',
    async configResolved(resolvedConfig) {
      // Only operate on a build.
      if (resolvedConfig.command !== 'build') {
        // XXX: First problem. If the parent build command was called
        //      using the JavaScript API, and this plugin was passed
        //      inline, then this will retroactively disable the plugin
        //      in the parent command, causing client reloads (below)
        //      to stop working.
        enabled = false;
        return;
      }

      ({ inlineConfig, configFile = false } = resolvedConfig);
    },
    async closeBundle() {
      if (!enabled) return;

      // Hook called repeatedly if building with watch (rebuilds).
      // Signal clients to reload instead of starting another server.
      if (previewServer) {
        triggerClientReload();
        return;
      }

      previewServer = await preview({
        // Copy over all inline config props.
        ...inlineConfig,

        // Override the inline config file to the resolved (absolute) path,
        // or false if the build command did not load a config file.
        configFile,
      });
    },
  };
};

The above inlineConfig should be that which was passed to a JavaScript API command, for example build(inlineConfig). The Vite executable translates command line parameters into this inline config. They are treated as overrides for any file configuration (UserConfig) that is loaded.

The above configFile should be the absolute path to the config file loaded by the command. It will be undefined if the inlineConfig.configFile was false, or if it was undefined and no config file was auto-detected.

When a Vite command is invoked from the command line, there is no way (currently) to specify plugins via command line options. Therefore, the inlineConfig will never contain a non-empty plugins array when a command is invoked from the command line, and this problem will not exist for immediate child commands invoked via the JavaScript API according to the problem pattern.

When a Vite command is invoked from the JavaScript API, plugins can be provided in the inline config, and the problem may exist if a plugin uses a plugin factory function (command/recommended pattern) as a closure to share state between hooks. Because there is no way to get a new instance of a plugin in the inline config plugins array, the same plugin instance would be passed to the child command. Hooks that are only expected to be invoked once (eg. config, configResolved) will be invoked again, any shared state will not be set to initial values, and the state may be mutated by hooks that are being invoked by more than one command. This would be solved if we could instead call getInlineConfig() to get a newly generated inline config, containing new instances of plugins. Using an inline config factory actually mimics the config loading pattern that avoids this problem when commands are invoked via the Vite command line.

Suggested solution

Short Term (minor):

  • Allow JavaScript API commands which accept InlineConfig values, to also accept functions (factories). The factory type would be () => InlineConfig | Promise<InlineConfig>
  • Add a getInlineConfig(): Promise<InlineConfig> method to the ResolvedConfig type, which can be called to get the inline config.
    • If the original inline config was a function, the function would be re-invoked to get a new instance of the inline config.
    • If the original inline config was an object, it would be returned.
  • The current inlineConfig property of ResolvedConfig would continue to return the inline config instance used by the command.

Long Term (major):

  • Deprecate JavaScript API command prototypes which accept inline config as an object, rather than a function.
  • (maybe) Deprecate the ResolvedConfig.inlineConfig property. It might be useful to keep access to the instance used by a command, though it could lead to plugin developers following my path inheriting parent config incorrectly.

Alternative

In my plugin, I explicitly set the InlineConfig.plugins to undefined when I inherit it, to disable inheriting "inline" plugins. This is not ideal because it makes the plugin behave differently when used inline vs in a config file.

There is no other solution that allows plugin inheritance that I can find. I can't even find a way to design a stateful plugin that would work when shared.

One potential alternative would be to remove InlineConfig properties like plugins, that are not aligned with command line options, making the JavaScript API limited to equivalency with the command line.

Another alternative might be to provide a purpose built way of sharing states between hooks, that is not dependent on a plugin factory function closure.

Additional context

No response

Validations

@bluwy
Copy link
Member

bluwy commented Jun 24, 2024

I've sent a PR to support this before #10326, but rightfully, the plugins should be managing state that can work in this scenario. They can base their state on the ResolvedConfig so that it's not mixed? Many frameworks also load plugins once and share them between builds, so it should work today as long as the plugin is coded right.

@Shakeskeyboarde
Copy link
Contributor Author

Shakeskeyboarde commented Jun 24, 2024

I've sent a PR to support this before #10326, but rightfully, the plugins should be managing state that can work in this scenario. They can base their state on the ResolvedConfig so that it's not mixed? Many frameworks also load plugins once and share them between builds, so it should work today as long as the plugin is coded right.

I thought of that. But there's nothing available to all hooks that you can use to maintain multiple states. If you used something like a weak map with the resolved config as key, then subsequent hooks would need to be passed the resolved config to do the lookup, and not all of them are. Same goes for any other potential key/reference option.

Also, you were addressing restarts (rebuilds?). You mentioned using the build start hook. This isn't that. For that, mutating the current state is acceptable, because (re)builds/starts are serial/non-parallel within a single command invocation, and therefore part of a single lifecycle (though a bit of an edge case, and just squeaking by). I'm talking about running two commands, in parallel, one as a side effect of the other.

Even if there turns out to be a way to handle all cases of plugin reuse that I haven't thought of, the methods for handling them would definitely be outside the expectations for plugin design. It's not something a plugin writer would reasonably be expected to handle. The plugin factory closure is expected (assumed) to be a de facto "pre-command-start" hook.

To simplify the scenario: if someone were to instantiate (configure) a plugin once, capturing the returned plugin instance, and then use the JavaScript API to pass it into two parallel build commands, and that caused the plugin to behave incorrectly, I would expect the plugin author to tell the user that they used the plugin incorrectly. The solution would be to please reconfigure (recall the plugin factory function) for each build invocation. But, in the scenario I'm positing, where a plugin wants to run a JavaScript API command that inherits config (plugins) from the parent command, there's no way to currently reinstantiate the inherited plugins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants