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

Support for .css and .manifest files and cache busting #14

Merged
merged 10 commits into from Mar 29, 2015

Conversation

Projects
None yet
3 participants
@jantimon
Copy link
Owner

jantimon commented Mar 11, 2015

This pull request deprecates the usage of {%=o.htmlWebpackPlugin.assets%} and creates a new representation called {%=o.htmlWebpackPlugin.files%}.

Maybe you can come up with a better name than files.
I had to rename assets to files to support existing (< 1.1.0) templates.

Example structure of the {%=o.htmlWebpackPlugin.files%} object:

{
  "manifest" : "manifest.appcache",
  "css" : [
    "app.css"
  ],
  "js" : [
    "commons.js",
    "index_bundle.js"
  ],
  "chunks" : {
    "commons" : {
      "entry" : "commons.js",
      "css" : [
      ]
    },
    "app" : {
      "entry" : "index_bundle.js",
      "css" : [
         "app.css"
      ]
    }
  }
}

It works well together with the https://github.com/lettertwo/appcache-webpack-plugin and https://github.com/webpack/extract-text-webpack-plugin

Added support for cache busting by using the webpack compilation hash.

This was referenced Mar 11, 2015

@jantimon

This comment has been minimized.

Copy link
Owner Author

jantimon commented Mar 11, 2015

Fixes #1 #6 #9

@jantimon jantimon referenced this pull request Mar 13, 2015

Merged

Feature/append #15

@jantimon

This comment has been minimized.

Copy link
Owner Author

jantimon commented Mar 24, 2015

@ampedandwired do you think this might be the way to go?
If so I would like to prepare some examples.

@jhnns

This comment has been minimized.

Copy link

jhnns commented Mar 25, 2015

That one looks really promising! (Haven't looked at the code though 😀)

@ampedandwired

This comment has been minimized.

Copy link
Collaborator

ampedandwired commented Mar 27, 2015

I really like this approach. Not so sure about the name files but I can't think of anything better.

Would be good to have a spec covering css (extract text plugin) and manifest (appcache) scenarios if you have the time. If not I can add them post merge, I've done some manual testing on this and it all looks good.

Should the manifest have the publicPath prepended?

Should the insertion of the cache-busting query hash be optional? Generally I would have expected cache-busting to be done by including the hash in the output filename using webpack's [name]_[hash].js output convention. Not sure if it's such an issue these days, but apparently the derfault configuration of older versions of Squid actually didn't cache URLs with query params at all.

@jantimon

This comment has been minimized.

Copy link
Owner Author

jantimon commented Mar 27, 2015

I'll add some tests for integration with both plugins.

Squid caches resources with a query string.
But I guess you are right the cache buster should be an option which is off by default.

@jantimon

This comment has been minimized.

Copy link
Owner Author

jantimon commented Mar 27, 2015

The cache buster is now optional.
Added jshint configuration.

ampedandwired added a commit that referenced this pull request Mar 29, 2015

Merge pull request #14 from jantimon/master
Support for .css and .manifest files and cache busting

@ampedandwired ampedandwired merged commit 3097ffb into jantimon:master Mar 29, 2015

mastilver pushed a commit that referenced this pull request Feb 26, 2018

@lock

This comment has been minimized.

Copy link

lock bot commented May 31, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.