fix: only apply the rollup wrap of dev server plugins if needed

Author: daKmoR

.changeset/fair-bugs-destroy.md

@@ -0,0 +1,14 @@
+---
+'plugins-manager': minor
+---
+
+Changes to `metaConfigToWebDevServerConfig`:
+
+- Renamed `wrapperFunction` to `rollupWrapperFunction`
+- Adds `setupRollupPlugins` which means those plugins are treated as rollup plugins and they will be wrapped by `rollupWrapperFunction` (if provided)
+- Plugins added via `setupPlugins` will no longer be wrapped
+- If you provide `config.plugins` then it will return that directly ignoring `setupPlugins` and `setupRollupPlugins`
+
+Changes to `metaConfigToRollupConfig`:
+
+- If you provide `config.plugins` then it will return that directly ignoring `setupPlugins`

.changeset/fair-worms-type.md

@@ -0,0 +1,7 @@
+---
+'@rocket/cli': patch
+---
+
+Changes to config:
+- Do not auto rollupWrap plugins added via `setupDevPlugins`.
+- If you provide `devServer.plugins` then it will return that directly ignoring `setupDevAndBuildPlugins` and `setupDevPlugins`

docs/docs/configuration/overview.md

@@ -23,11 +23,22 @@ New plugins can be added and all default plugins can be adjusted or even removed
 
 ```js
 export default {
+  // add remark/unified plugin to the markdown processing (e.g. enable special code blocks)
   setupUnifiedPlugins: [],
+
+  // add a rollup plugins to the web dev server (will be wrapped with @web/dev-server-rollup) AND the rollup build (e.g. enable json importing)
   setupDevAndBuildPlugins: [],
+
+  // add a plugin to the web dev server (will not be wrapped) (e.g. esbuild for typescript)
   setupDevPlugins: [],
+
+  // add a plugin to the rollup build (e.g. optimization steps)
   setupBuildPlugins: [],
+
+  // add a plugin to eleventy (e.g. a filter packs)
   setupEleventyPlugins: [],
+
+  // add a plugin to the cli (e.g. a new command like "rocket my-command")
   setupCliPlugins: [],
 };
 ```

docs/docs/tools/overview.md docs/docs/tools/plugins-manager.mddocs/docs/tools/overview.md renamed to docs/docs/tools/plugins-manager.md

@@ -216,22 +216,31 @@ const plugins = finalMetaPlugins.map(pluginObj => {
 
 **Examples**
 
-Rollup has a more specific helper
+Rollup has a more specific helper that handles
+
+- `config.setupPlugins`
+
+Note: if you provide `config.plugins` then it will return that directly ignoring `setupPlugins`
 
 ```js
 import { metaConfigToRollupConfig } from 'plugins-manager';
 
 const finalConfig = metaConfigToRollupConfig(currentConfig, defaultMetaPlugins);
 ```
 
-Web Dev Server has a more specific helper
+Web Dev Server has a more specific helper that handles
+
+- `config.setupPlugins`
+- `config.setupRollupPlugins`
+
+Note: if you provide `config.plugins` then it will return that directly ignoring `setupPlugins` and `setupRollupPlugins`
 
 ```js
 import { metaConfigToWebDevServerConfig } from 'plugins-manager';
 import { fromRollup } from '@web/dev-server-rollup';
 
 const finalConfig = metaConfigToWebDevServerConfig(currentConfig, defaultMetaPlugins, {
-  wrapperFunction: fromRollup,
+  rollupWrapperFunction: fromRollup,
 });
 ```
 

packages/cli/src/RocketBuild.js

@@ -24,11 +24,6 @@ async function buildAndWrite(config) {
 }
 
 async function productionBuild(config) {
-  // const serviceWorkerFileName =
-  //   config.build && config.build.serviceWorkerFileName
-  //     ? config.build.serviceWorkerFileName
-  //     : 'service-worker.js';
-
   const mpaConfig = createMpaConfig({
     input: '**/*.html',
     output: {

packages/cli/src/RocketStart.js

@@ -39,10 +39,11 @@ export class RocketStart {
         clearTerminalOnReload: false,
         ...this.config.devServer,
 
-        setupPlugins: [...this.config.setupDevAndBuildPlugins, ...this.config.setupDevPlugins],
+        setupRollupPlugins: this.config.setupDevAndBuildPlugins,
+        setupPlugins: this.config.setupDevPlugins,
       },
       [],
-      { wrapperFunction: fromRollup },
+      { rollupWrapperFunction: fromRollup },
     );
 
     this.devServer = await startDevServer({

packages/cli/test-node/RocketCli.e2e.test.js

@@ -122,7 +122,7 @@ describe('RocketCli e2e', () => {
   });
 
   describe('setupDevAndBuildPlugins in config', () => {
-    it('can add a rollup plugin to build', async () => {
+    it('can add a rollup plugin via setupDevAndBuildPlugins for build command', async () => {
       cli = new RocketCli({
         argv: [
           'build',
@@ -135,7 +135,7 @@ describe('RocketCli e2e', () => {
       expect(inlineModule).to.equal('var a={test:"data"};console.log(a);');
     });
 
-    it('can add a rollup plugin to dev', async () => {
+    it('can add a rollup plugin via setupDevAndBuildPlugins for start command', async () => {
       cli = new RocketCli({
         argv: [
           'start',

packages/plugins-manager/src/metaConfigToRollupConfig.js

@@ -7,6 +7,10 @@ import { executeSetupFunctions } from 'plugins-manager';
  * @param {MetaPlugin[]} [metaPlugins]
  */
 export function metaConfigToRollupConfig(config, metaPlugins = []) {
+  if (config.plugins) {
+    delete config.setupPlugins;
+    return config;
+  }
   const _metaPlugins = executeSetupFunctions(config.setupPlugins, [...metaPlugins]);
 
   const plugins = _metaPlugins.map(pluginObj => {

packages/plugins-manager/src/metaConfigToWebDevServerConfig.js

@@ -1,28 +1,53 @@
-/** @typedef {import('../types/main').MetaPlugin} MetaPlugin */
+/** @typedef {import('../types/main').MetaPluginWrapable} MetaPluginWrapable */
 
 import { executeSetupFunctions } from 'plugins-manager';
 
 /**
  * @param {any} config
- * @param {MetaPlugin[]} metaPlugins
+ * @param {MetaPluginWrapable[]} metaPlugins
  * @param {object} [options]
- * @param {function | null} [options.wrapperFunction]
+ * @param {function | null} [options.rollupWrapperFunction]
  */
-export function metaConfigToWebDevServerConfig(config, metaPlugins, { wrapperFunction = null }) {
-  const _metaPlugins = executeSetupFunctions(config.setupPlugins, [...metaPlugins]);
+export function metaConfigToWebDevServerConfig(
+  config,
+  metaPlugins,
+  { rollupWrapperFunction = null } = {},
+) {
+  if (config.plugins) {
+    delete config.setupPlugins;
+    delete config.setupRollupPlugins;
+    return config;
+  }
 
-  const plugins = _metaPlugins.map(pluginObj => {
-    const usePlugin =
-      typeof wrapperFunction === 'function' ? wrapperFunction(pluginObj.plugin) : pluginObj.plugin;
+  const metaPluginsNoWrap = metaPlugins.map(pluginObj => {
+    pluginObj.__noWrap = true;
+    return pluginObj;
+  });
+
+  const rollupPlugins = /** @type {MetaPluginWrapable[]} */ (executeSetupFunctions(
+    config.setupRollupPlugins,
+    [...metaPluginsNoWrap],
+  ));
+
+  const wrappedRollupPlugins = rollupPlugins.map(pluginObj => {
+    if (typeof rollupWrapperFunction === 'function' && pluginObj.__noWrap !== true) {
+      pluginObj.plugin = rollupWrapperFunction(pluginObj.plugin);
+    }
+    return pluginObj;
+  });
 
+  const _metaPlugins = executeSetupFunctions(config.setupPlugins, [...wrappedRollupPlugins]);
+
+  const plugins = _metaPlugins.map(pluginObj => {
     if (pluginObj.options) {
-      return usePlugin(pluginObj.options);
+      return pluginObj.plugin(pluginObj.options);
     } else {
-      return usePlugin();
+      return pluginObj.plugin();
     }
   });
   config.plugins = plugins;
 
   delete config.setupPlugins;
+  delete config.setupRollupPlugins;
   return config;
 }

packages/plugins-manager/test-node/metaConfigToRollupConfig.test.js

@@ -28,4 +28,16 @@ describe('metaConfigToRollupConfig', () => {
     expect(config.plugins).to.deep.equal(['firstPlugin', '-- insertPlugin --']);
     expect(config.setupPlugins).to.be.undefined;
   });
+
+  it('prefers a user set config.plugins', async () => {
+    const config = metaConfigToRollupConfig(
+      {
+        setupPlugins: [addPlugin({ name: 'insert', plugin: insertPlugin })],
+        plugins: ['user-set'],
+      },
+      threeExistingPlugin,
+    );
+    expect(config.plugins).to.deep.equal(['user-set']);
+    expect(config.setupPlugins).to.be.undefined;
+  });
 });