jstuckey / gulp-gzip

Gzip plugin for gulp.
MIT License
175 stars 17 forks source link

Threshold may cause unexpected result #18

Closed kimjuan closed 8 years ago

kimjuan commented 8 years ago
  1. hello.js's size is more Threshold, hence, it is gzipped to hello.js.gz.
  2. Changes is made to hello.js, its size is now less than Threshold.
  3. Since it is below Threshold, there is no op.
  4. hello.js.gz still exists. (with the old content).
  5. The presence of hello.js.gz will make my webserver serves it rather than hello.js (with the updated content)

For my own use case, I've patched it to delete existing hello.js.gz when the callback has wasCompressed == false.

jstuckey commented 8 years ago

Hey, @kimjuan!

Could you instead delete hello.* before running your gulp task? Something like this: https://github.com/gulpjs/gulp/blob/master/docs/recipes/delete-files-folder.md

kimjuan commented 8 years ago

Thanks for the suggestion. That example may not be so clear cut for some use case.

Consider this:

Using gulp-newer, not all file will be re-processed, only modified files will be. I want to retain unaffected *.gz files, with the modified date so webserver can 304 appropriately.

So deleting hello.js.gz is kind of conditional. wasCompressed == false is the best indicator whether to delete hello.js.gz.

kimjuan commented 8 years ago

Hi, I had tested my monkey patches in production for a month and it is working fine. If you think it makes sense, I will add test and config gzip({ deleteMode: true }) (default to false) so won't bc break, and request a PR.

Another way to explain myself. It is same rationale as rsync's deleteMode. You have a.js, b.js and rsync to remote server. Without deleteMode, after you rename b.js to c.js, the remote server will ended up with a.js, b.js, c.js instead of a.js, c.js

jstuckey commented 8 years ago

Sounds good! Open a PR and I'll take a look,

jstuckey commented 8 years ago

Merged #20 and published as version 1.3.0. https://www.npmjs.com/package/gulp-gzip