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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add cache option (options.cache) #86

Merged
merged 1 commit into from
Dec 13, 2017
Merged

Conversation

alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented Dec 12, 2017

No test here, but tested locally, all works fine 馃槃

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

馃憤

src/index.js Outdated
@@ -58,6 +62,9 @@ class CompressionPlugin {

apply(compiler) {
compiler.plugin('emit', (compilation, callback) => {
const cacheDir =
Copy link
Member

Choose a reason for hiding this comment

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

const { cache,  algorithm, ... } = this.options 
// or const { options } = this 

const cacheDir = this.options.cache === true 
  ? findCacheDir({ name: 'compression-webpack-plugin' }) 
  : this.options.cache;

src/index.js Outdated
if (this.options.cache) {
const cacheKey = serialize({
// Invalidate cache after upgrade `zlib` module (build-in in `nodejs`)
node: process.version,
Copy link
Member

Choose a reason for hiding this comment

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

馃憤

src/index.js Outdated
query: parse.query || '',
};

let newFile = this.options.asset.replace(/\[(file|path|query)\]/g, (p0, p1) => sub[p1]);
Copy link
Member

Choose a reason for hiding this comment

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

newFile => asset

README.md Outdated
@@ -40,6 +40,7 @@ module.exports = {
|**`test`**|`{RegExp\|Array<RegExp>}`|`.`|All assets matching this `{RegExp\|Array<RegExp>}` are processed|
|**`include`**|`{RegExp\|Array<RegExp>}`|`undefined`|Files to `include`|
|**`exclude`**|`{RegExp\|Array<RegExp>}`|`undefined`|Files to `exclude`|
|**`cache`**|`{Boolean\|String}`|`false`|Enable file caching|
Copy link
Member

Choose a reason for hiding this comment

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

Could you do me favor and update the options table with the snippet below (add links to option in README)

|**[`name`](#anchor)**|`{Type}`|`<default>`|<description>|
|Name|Type|Default|Description|
|:--:|:--:|:-----:|:----------|
|**[`test`](#test)**|`{RegExp\|Array<RegExp>}`|`.`|All assets matching this `{RegExp\|Array<RegExp>}` are processed|
|**[`include`](#include)**|`{RegExp\|Array<RegExp>}`|`undefined`|Files to `include`|
|**[`exclude`](#exclude)**|`{RegExp\|Array<RegExp>}`|`undefined`|Files to `exclude`|
|**[`cache`](#cache)**|`{Boolean\|String}`|`false`|Enable file caching|
|**[`asset`](#asset)**|`{String}`|`[path].gz[query]`|The target asset name. `[file]` is replaced with the original asset. `[path]` is replaced with the path of the original asset and `[query]` with the query|
|**[`filename`](#filename)**|`{Function}`|`false`|A `{Function}` `(asset) => asset` which receives the asset name (after processing `asset` option) and returns the new asset name|
|**[`algorithm`](#algorithm)**|`{String\|Function}`|`gzip`|Can be `(buffer, cb) => cb(buffer)` or if a `{String}` is used the algorithm is taken from `zlib`|
|**[`threshold`](#threshold)**|`{Number}`|`0`|Only assets bigger than this size are processed. In bytes.|
|**[`minRatio`](#minratio)**|`{Number}`|`0.8`|Only assets that compress better than this ratio are processed|
|**[`deleteOriginalAssets`](#deleteoriginalassets)**|`{Boolean}`|`false`|Whether to delete the original assets or not|

@michael-ciniawsky michael-ciniawsky added this to the 1.1.0 milestone Dec 13, 2017
@alexander-akait
Copy link
Member Author

@michael-ciniawsky done. newFile => newAssetName because it is not asset it is asset name. Also we can rename asset option to assetName, it is more right and not confusing, but it is breaking change 馃槥

@michael-ciniawsky michael-ciniawsky merged commit 49a8a77 into master Dec 13, 2017
@michael-ciniawsky
Copy link
Member

@evilebottnawi Yep, deleteOriginalAssets could also be renamed with a better name, but as you said it's sadly a major 馃槥

@michael-ciniawsky michael-ciniawsky deleted the cache-option branch December 13, 2017 09:56
@michael-ciniawsky michael-ciniawsky changed the title feat: add cacheoption (options.cache) feat: add cache option (options.cache) Dec 13, 2017
@alexander-akait
Copy link
Member Author

@michael-ciniawsky @d3viant0ne we need release 馃槃

@michael-ciniawsky michael-ciniawsky removed this from the 1.1.0 milestone Dec 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants