wmr icon indicating copy to clipboard operation
wmr copied to clipboard

Custom plugins buildStart signature in dev/ start mode

Open piotr-cz opened this issue 3 years ago • 7 comments

I'm trying to use custom plugin in dev mode (which basically prepends import 'preact/debug in main index.js file). It relies on the buildStart Rollup hook which accepts object or array of objects.

However when I've added it to wmr config start script fails with an error, as wmr provides function (see rollup-plugin-container.js).

UnhandledPromiseRejectionWarning: TypeError: Cannot convert undefined or null to object
    at Function.values (<anonymous>)
    at Object.buildStart (xxx\node_modules\@piotr-cz\rollup-plugin-prepend-modules\dist\rollup-plugin-prepend-modules.cjs.js:53:18)

Below is my wmr.config.js

import prependModulesPlugin from '@piotr-cz/rollup-plugin-prepend-modules'

export default async function (config) {
  if (config.mode === 'start') {
    config.plugins.push(
      prependModulesPlugin({ modules: ['preact/debug'] })
    )
  }
}

Am I doing something wrong - or it's a bug?

piotr-cz avatar Feb 27 '21 17:02 piotr-cz

This is a tricky one - in development, there is no such think as an entry module, and we don't run any bundle-related plugin methods.

One option would be to inject the import into the first module served, or into all modules. Imports are singletons, so adding them in every module doesn't have a runtime cost.

developit avatar Feb 27 '21 21:02 developit

Thanks for the tip!

In that case I would add a note to the tagline Supports Rollup plugins, even in development where Rollup isn't used. Maybe even show a warning when Rollup plugin has a buildStart hook as per docs it's supposed to be used to read InputOptions, which are not provided.

piotr-cz avatar Feb 27 '21 21:02 piotr-cz

Random thought: We could query all js imports in an HTML file and mark them as entries.

marvinhagemeister avatar Feb 28 '21 08:02 marvinhagemeister

@madmanwithabike I like this idea

This is my workaround based on https://github.com/preactjs/wmr/issues/383#issuecomment-787150204:

export default async function (config) {
  if (config.mode === 'start') {
    config.plugins.push(enablePreactDebugPlugin())
  }
}

function enablePreactDebugPlugin() {
  let isInjected = false

  return {
    name: 'inject-preact-debug',

    transform (code, id) {
      // Assume first id is an entry point defined in index.html
      if (!isInjected) {
        isInjected = true
        return `import 'preact/debug';\n\n${code}`
      }

      return null
    }
  }
}

piotr-cz avatar Feb 28 '21 09:02 piotr-cz

Back to the Rollup plugins buildStart signature, why the buildOptions hook for each plugin is invoked with container.options function instead of object?

Should not line 184 in https://github.com/preactjs/wmr/blob/e8b588054e095bed7dd7bc01601d246cb6b6cd00/packages/wmr/src/lib/rollup-plugin-container.js#L180-L188 be

-  				plugin.buildStart.call(ctx, container.options);
+  				plugin.buildStart(container.options(ctx.options));

I mean - the container.options method returns {import('rollup').InputOptions} and this should be passed to the Rollup hooks, not a function that builds InputOptions.

piotr-cz avatar Mar 04 '21 09:03 piotr-cz

container.options is the current options object, it's different from plugin.options.

developit avatar Mar 10 '21 23:03 developit

I mean - when using following config

// wmr.config.js
export default async function(config) {
  config.plugins.push({
    buildStart(inputOptions) {
      console.log(inputOptions)
    }
  })
}

and running

wmr start
[Function: options]
...

You'll see that inputOptions argument is resolved to a function, while it's supposed to be an object (see Rollup: buildStart) and this is what breaks Rollup plugins which use buildStart hook.

The fact that in case of WMR the inputOptions.input property would not be provided is a different story (it should be null in my opinion).

piotr-cz avatar Mar 11 '21 09:03 piotr-cz