Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple cdn modules for multiple pages app #13

Merged
merged 5 commits into from
Jan 31, 2018
Merged

Multiple cdn modules for multiple pages app #13

merged 5 commits into from
Jan 31, 2018

Conversation

likun7981
Copy link
Contributor

@likun7981 likun7981 commented Jan 29, 2018

webpack.config.js

plugins:[
// ...otherConfig
new HtmlWebpackPlugin({
      title: config.title,
      favicon: path.resolve(__dirname, './favicon.png'),
      template: path.resolve(__dirname, './index.html'),
      filename: './generator/index.html',
      chunks: ['generator', 'vendor']
 }),
 new HtmlWebpackPlugin({
      title: config.title,
      // This value corresponds to the configuration key inside the modules
      cdnModule: 'react', 
      favicon: path.resolve(__dirname, './favicon.png'),
      template: path.resolve(__dirname, './index.html'),
      filename: './admin/index.html',
      chunks: ['admin', 'vendor']
  }),
 new WebpackCdnPlugin({
   modules: {
      'vue': [
        { name: 'vue', var: 'Vue', path: 'dist/vue.min.js' },
      ],
      'react': [
        { name: 'react', var: 'React', path: `umd/react.${process.env.NODE_ENV}.min.js` },
        { name: 'react-dom', var: 'ReactDOM', path: `umd/react-dom.${process.env.NODE_ENV}.min.js` },
      ]
    }
 })
]
  • If you do not give cdnModule this value, the default is to take the first one
  • If you set cdnModule = false, it will not inject cdn

Do not worry, it is still compatible with the previous configuration 😀

@codecov
Copy link

codecov bot commented Jan 29, 2018

Codecov Report

Merging #13 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #13   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          45     58   +13     
=====================================
+ Hits           45     58   +13

@shirotech
Copy link
Owner

shirotech commented Jan 29, 2018

@likun7981 This is a very good feature indeed. Nice one! Thank you for contributing, I have reviewed and seems good to merge after removing some comment debug code.

Also I think cdnModule can support arrays and we do a loop and remove dupes. But that can be for another time.

Also if you have some time, just need to update the docs on README.md, thanks.

@likun7981
Copy link
Contributor Author

Updated README.md

module.js Outdated
const mods = this.modules[key];
mods.forEach((p) => {
// if (externals[p.name]) {
// console.warn(`The key '${p.name}' of module ${key === DEFAULT_MODULE_KEY ? '' : `'${key}'`} already exists `); // eslint-disable-line
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't mind, can you please remove comment code? Thanks. Rest seems good to merge 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed already

@shirotech shirotech merged commit 037b0c0 into shirotech:master Jan 31, 2018
@shirotech
Copy link
Owner

Merged and published :)

@likun7981 likun7981 changed the title Mutiple cdn modules for mutiple pages app Multiple cdn modules for multiple pages app Jan 31, 2018
@jayasme
Copy link

jayasme commented Feb 20, 2019

Impressive solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants