Plugin Enhancement: options() hook

[FredKSchott]

In our current plugin system, Snowpack is responsible for loading each plugin. We'd like to move to a format closer to Rollup, where you load a plugin yourself in your config file, and then pass the result directly to Snowpack (instead of Snowpack doing the loading for you). Letting you load and initialize your plugins directly gives you more control over them.

Proposal

-module.exports = function(snowpackConfig, pluginOptions) {
+module.exports = function(pluginOptions) {
+  let isBundled;
   return {
     name: 'my-commenter-plugin',
+    options: (snowpackConfig) {
+      isBundled = snowpackConfig.buildOptions.bundle;
+   }, 
    async transform({ fileExt, filePath, contents, isDev }) {
-     if (fileExt === '.js' && snowpackConfig.buildOptions.bundle) {
+     if (fileExt === '.js' && isBundled) {
        return `/* I'm in a bundled app! */ ${contents}`;
      }
    }
  };
}

Implementation Stages

1. Add support for the options hook.
2. (optional) Add a deprecation notice if you reference the config object in your plugin.
3. Snowpack v3: Remove the config object argument from the function signature.

Would love any help on 1 (adding the options hook, as an explicit place to make changes to the config object). That shouldn't be blocked by the discussion on how we read from options in the future.

[999946]

Why not this.config? also this.config is read-only.

[FredKSchott]

Great idea, we actually considered this but ended up pursuing the proposal above as more explicit (and therefore easier to document). But, I agree that this.config (or even this.getConfig()) would get rid of the need to maintain a variable reference outside of the object.

Both of these ideas can coexist as well: options() can be for modifying the config before it's locked, and this.config can be used to access the locked, read-only reference.

[FredKSchott]

Would love any help on 1 (adding the options hook, as an explicit place to make changes to the config object). That shouldn't be blocked by the discussion on how we read from options in the future.

[MoonBall]

I would like to do it. What is the purpose of options: (snowpackConfig) { in your proposal?

[FredKSchott]

The options() hook should serve two purposes:

1. A chance for plugins to read config, so that we can get rid of the snowpackConfig argument in the current function wrapper.
2. A chance for plugins to modify the config object (right now, the existing snowpackConfig argument isn't 100% finalized when we pass it to the function, so modifying it could theoritically cause issues).

As I write this out, we may want to name it config() instead of options(), since "options" is technically what we call the arguments that we pass to the plugin and not the Snowpack config object itself, and this hook is meant to deal with the Snowpack config object.

[FredKSchott]

Tackled in #864, thanks @MoonBall !