swansontec / rollup-plugin-flow-entry

Allows Flow to find the original typed source code for a Rollup bundle
14 stars 4 forks source link

Warning while used as "output plugin" #14

Open pascalduez opened 4 years ago

pascalduez commented 4 years ago

When used as "output plugin" the following warning arise:

(!) The "transform" hook used by the output plugin rollup-plugin-flow-entry is a build time hook and will not be run for that plugin. Either this plugin cannot be used as an output plugin, or it should have an option to configure it as an output plugin.

We ideally want the plugin to be run only for commonjs, if not it will generate a .flow.js file for every output.

Despite the warning, the desired output file is properly created.

Sample config using multiple output:

import resolve from 'rollup-plugin-node-resolve';
import flowEntry from 'rollup-plugin-flow-entry';
import commonjs from 'rollup-plugin-commonjs';

let plugins = [
  resolve(),
  commonjs(),
];

export default {
  plugins,
  input: 'src/index.js',
  output: [
    {
      file: 'name.umd.js',
      format: 'umd',
    },
    {
      file: 'name.cjs.js',
      format: 'cjs',
      plugins: [flowEntry()],
    },
    {
      file: 'name.esm.js',
      format: 'esm',
    },
  ],
};
swansontec commented 4 years ago

Ah, interesting. We use the transform hook to work to snag some information related to rollup-plugin-multi-entry, but that clearly won't work when used as an output plugin. The good news is that this step is only relevant if you are using rollup-plugin-multi-entry, so, you can safely ignore the warning for now.

I'm not sure how to fix this. One option is to just remove the extra rollup-plugin-multi-entry logic, or at least make it something people need to enable explicitly. I don't know how many people are even using that feature at this point.

pascalduez commented 4 years ago

you can safely ignore the warning for now

Yep, thanks!

at least make it something people need to enable explicitly

Sounds like a good option.

BTW the rollup-plugin-multi-entry plugin has been renamed into @rollup/plugin-multi-entry.

swansontec commented 4 years ago

After thinking about this some more, I think it makes sense to remove the @rollup/plugin-multi-entry logic from this plugin (didn't realize it was renamed), and instead add glob support to our config input. That way, if you are using @rollup/plugin-multi-entry, you can just pass the same pattern to both plugins:

import multiEntry from '@rollup/plugin-multi-entry'
import flowEntry from 'rollup-plugin-flow-entry'

const input = 'src/*.js'

export default {
  input,
  output: { ... },
  plugins: [
    multiEntry(),
    flowEntry(input)
  ]
}