ampproject / rollup-plugin-closure-compiler

Leverage Closure Compiler to minify and optimize JavaScript with Rollup.
Apache License 2.0
292 stars 30 forks source link

How to keep both generated externs and user defined externs? #49

Closed maxmilton closed 6 years ago

maxmilton commented 6 years ago

What's the issue?

As it stands you can either have externs generated by this plugin OR externs which you pass via the plugin options. As far as I'm aware it's not possible to keep both because the generated externs are overriden here: https://github.com/ampproject/rollup-plugin-closure-compiler/blob/master/src/options.ts#L100.

How do we reproduce the issue?

output: {
  format: 'iife',
  name: 'MyComponent',
}

...

compiler({
  externs: [
    join(__dirname, 'component-externs.js'),
  ],
})

The contents of component-externs.js will override the generated extern containing the output name.

kristoferbaxter commented 6 years ago

Thanks for the bug report @MaxMilton. I've submitted a PR to address the issue #50.

A review would be great!

kristoferbaxter commented 6 years ago

Released as 0.6.2

maxmilton commented 6 years ago

I've run into a bug with this. If you have multiple bundles in your rollup config and are passing the same options object to to compiler() then subsequent runs of the plugin will not have any externs applied, e.g.:

import compiler from '@ampproject/rollup-plugin-closure-compiler';

const compilerOpts = {
  externs: [
    require.resolve('google-closure-compiler/contrib/externs/chrome.js'),
  ],
  compilation_level: 'ADVANCED',
};

export default [
  {
    input: 'src/entry1.js',
    output: {
      format: 'esm',
      file: 'dist/bundle1.js',
    },
    plugins: [
      compiler(compilerOpts), // <-- this run deletes compilerOpts.externs
    ],
  },
  {
    input: 'src/entry2.js',
    output: {
      format: 'esm',
      file: 'dist/bundle2.js',
    },
    plugins: [
      compiler(compilerOpts), // <-- never gets compilerOpts.externs :(
    ],
  },
  {
    input: 'src/entry3.js',
    output: {
      format: 'esm',
      file: 'dist/bundle3.js',
    },
    plugins: [
      compiler(compilerOpts), // :(
    ],
  },
];

Probably not a good idea to mutate the plugin options object right?

A workaround for anyone running into this is to clone of the options object for each plugin definition:

compiler({ ...compilerOpts }),
kristoferbaxter commented 6 years ago

Sorry for modifying the plugin options, fixed in 0.7.1.

maxmilton commented 6 years ago

Confirmed fix :+1:

Along with the recent logging changes it's making this plugin very enjoyable to work with. Thank you.