preactjs / wmr

👩‍🚀 The tiny all-in-one development tool for modern web apps.
https://wmr.dev/
MIT License
4.93k stars 110 forks source link

Custom plugins buildStart signature in dev/ start mode #383

Open piotr-cz opened 3 years ago

piotr-cz commented 3 years ago

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?

developit commented 3 years ago

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.

piotr-cz commented 3 years ago

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.

marvinhagemeister commented 3 years ago

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

piotr-cz commented 3 years ago

@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 commented 3 years ago

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.

developit commented 3 years ago

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

piotr-cz commented 3 years ago

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).