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

Split vendor css into its own file #598

Closed
ronjouch opened this issue Mar 14, 2017 · 7 comments
Closed

Split vendor css into its own file #598

ronjouch opened this issue Mar 14, 2017 · 7 comments

Comments

@ronjouch
Copy link

ronjouch commented Mar 14, 2017

This is a followup to bootstrap-vue/bootstrap-vue#143 - Document including Bootstrap CSS, in which I'm asking for advice to integrate CSS dependencies into my vuejs-templates/webpack-based app.

In comment bootstrap-vue/bootstrap-vue#143 (comment), I'm advised to configure my webpack build to emit two separate bundles app.css and vendor.css. This for the same reasons it's done for the js, in order to:

  1. Limit the number of network requests, while ensuring to...
  2. ... make the slowly-changing vendor.<hash>.css as cache-friendly as possible.

I'm now looking at my webpack build configuration, which is mostly this template untouched, and I don't think this is currently baked in it.

Thanks.

@asafyish
Copy link

Hi,

I solved that by changing build/webpack.prod.conf.js, lines 69-77, from:

        return (
          module.resource &&
          /\.js$/.test(module.resource) &&
          module.resource.indexOf(
            path.join(__dirname, '../node_modules')
          ) === 0
        )
      }

to:

        return (
          module.resource &&
          (/\.js$/.test(module.resource) ||
            /\.css$/.test(module.resource)) &&
          module.resource.indexOf(
            path.join(__dirname, '../node_modules')
          ) === 0
        )
      }

notice the "/.css$/.test(module.resource)) &&" line.
This cause css that are coming from node_modules to be in vendor file.

Let me know if it's working for you, because I only did minimal testing on it.

@ronjouch
Copy link
Author

ronjouch commented Mar 15, 2017

@asafyish looks good to me, and works as expected here. Thanks! Proposing as PR #603 with this fix.

@bartcorremans
Copy link

When you do this make sure to also include .vue in the regex check, otherwise this will cause issues with css that's extracted from vendor .vue files during build. Found this out the painful way.

@ronjouch
Copy link
Author

"When you do this make sure to also include .vue in the regex check, otherwise this will cause issues with css that's extracted from vendor .vue files during build. Found this out the painful way."

@bartcorremans thanks for the tip. Have you been running this without problem for some time? Should I just amend my PR with /\.(css|js|vue)$/.test(module.resource) and call it a day?

@bartcorremans
Copy link

This is something I just stumbled upon today when importing a third-party .vue file. The generated output seems ok with this change but it's not something that I've been using in production for an extended time.

@ronjouch
Copy link
Author

Alright, leaving this without |vue for now. @bartcorremans (or reviewers considering merging this): if you become confident it should be added, ping me here and I will amend the fix.

@LinusBorg
Copy link
Contributor

Since this doesn't seem to be well-tested I will close this and not merge the PR.

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

No branches or pull requests

4 participants