adambullmer / vue-cli-plugin-browser-extension

Browser extension development plugin for vue-cli 3.0
GNU Lesser General Public License v3.0
427 stars 76 forks source link

Fix missing chunks in background and content scripts when in production mode #16

Closed siphomateke closed 6 years ago

siphomateke commented 6 years ago

When in production mode, Vue CLI splits chunks into separate files. Since the background and content scripts are single js files and don't have corresponding HTML files, they shouldn't be split up into separate chunks.

This pull request is made to be compatible with and is based on my other pull request: #14

adambullmer commented 6 years ago

Looks like after merging #13 this PR needs to be rebased off master now

adambullmer commented 6 years ago

Would you be able to make this branch isolated so that it is mergable on it's own without building on #14?

adambullmer commented 6 years ago

Putting this here for discussion. It looks like removal of the codesplitting on particular entries can be achieved in this manner, rather than disabling splitting for all entries. I've tested this to work in scenarios where an entrypoint was added via the pages attribute in the vue.config.js file.

// webpack-chain
config.plugins.delete('html-<entryname>').delete('prefetch-<entryname>').delete('preload-<entryname>');
siphomateke commented 6 years ago

I've rebased the branch off master and isolated it.

siphomateke commented 6 years ago

I couldn't get your code to work. If I understand correctly, with the following in index.js, code splitting of the background entry should be disabled.

api.chainWebpack((webpackConfig) => {
    webpackConfig.entryPoints
      .delete('app').end()
      .entry('background').add(backgroundFile).end()
      .when(pluginOptions.components.contentScript, (config) => {
        config.entry('content-script').add(contentScriptFile).end()
      })
    webpackConfig.plugins.delete('html-background').delete('prefetch-background').delete('preload-background')
  })

This did not disable code splitting on the background entry when I tested it. I used a clean install of the plugin with the following options:

// vue.config.js
options: {
  components: {
    contentScript: true,
    standalone: true,
  },
  api: 'browser',
  usePolyfill: true,
  autoImportPolyfill: true,
},
adambullmer commented 6 years ago

the split will still happen for other components, like the popup. I'd look into what the background.js file that was output had in it to see if it is still waiting on other chunks to be loaded. I think if it is still splitting, then a search for chunk-vendor should com back empty.

For similar reasons as explained in #17's fingerprinting explanation, codesplitting might only help keep the final extension size down by creating reusable chunks between extension features (popup and standalone for example). But that should probably be a stretch goal past a well functioning, general purpose framework. I'd be ok with completely disabling codesplitting for now, and circle back some time in the future when the use case for doing so would be better understood.

That would simplify your PR to just needing this line, as suggested in https://forum.vuejs.org/t/disable-code-splitting-in-vue-cli-3/36295/2.

config.optimization.delete('splitChunks')

Further explaining my previous suggestion I think the full setup should be as follows to selectively disable codesplitting per entry. Note the added pages to vue.config.js, and removed entry in the webpack chain.:

// vue.config.js
module.exports = {
  pages: {
    background: { entry: 'background.js' },  // chunk names are the key of this object
  },
  options: {
    components: {
      contentScript: true,
      standalone: true,
    },
    api: 'browser',
    usePolyfill: true,
    autoImportPolyfill: true,
  },
}

api.chainWebpack((webpackConfig) => {
    webpackConfig.entryPoints
      // .delete('app').end()
      // .entry('background').add(backgroundFile).end()
      .when(pluginOptions.components.contentScript, (config) => {
        config.entry('content-script').add(contentScriptFile).end()
      })
    webpackConfig.plugins.delete('html-background').delete('prefetch-background').delete('preload-background')
  })
``
siphomateke commented 6 years ago

Oh, I see what you meant now. I was trying to figure out a way to use the page attribute to add the background and content scripts a while back but couldn't figure out how.

I agree that this should be re-visited at a later time. I've updated the pull request to just disable code splitting altogether as you suggested.

adambullmer commented 6 years ago

welp, one last conflict :sweat_smile:

siphomateke commented 6 years ago

Fixed