sindresorhus / gulp-rev

Static asset revisioning by appending content hash to filenames: `unicorn.css` → `unicorn-d41d8cd98f.css`
MIT License
1.54k stars 218 forks source link

Option `base` do nothing #150

Open smoliakov opened 8 years ago

smoliakov commented 8 years ago

Whether you define base option in manifest or not the result is the same. Probably its better to use var revisionedFile = relPath(opts.base || file.base, file.path); instead.

matikun86 commented 8 years ago

Is there any feedback on this issue? As I think I'm having the same problem on v 7.0.0.

ruanyl commented 8 years ago

Just a bit of my personal opinion about this issue. Supply base to rev.manifest() maybe originally used for manifest() to find manifest.json when merge: true

Some investigate of the reason: https://github.com/gulpjs/vinyl-fs/blob/4c0288f3471b4e9dbc716fb10362052377ac320b/lib%2FprepareWrite.js#L45

if you take a look at the implementation of gulp.dest(), the base you provided for rev.manifest() actually doesn't effects the output path directly. what effects the output path is file.relative which is based on file.base and file.path

for example if you have:

.pipe(rev.manifest({
  base: 'public/build'
}))
.pipe(gulp.dest('path/to'))

by default the path of manifest file is rev-manifest.json. The base here is public/build, so now the relative will be ../../rev-manifest.json

And the final path to rev-manifest.json is: path.resolve(process.cwd(), 'path/to', '../../rev-manifest.json'), so it is possible the path is out of your project folder...

So use base in manifest() isn't really a good idea, it is better to also provide path to manifest() also, so that you will know exactly what is the relative instead of having ../ in the path. for example:

.pipe(rev.manifest({
  base: 'public/build',
  path: 'public/build/rev-manifest.json'
}))
.pipe(gulp.dest('path/to'))

now the relative is always rev-manifest.json, and the dest path is what people normally expect: path/to.

It might be better to hidden the details of vinyl to the end users, I mean base,path or relative of vinyl. So that user of gulp plugin don't get confused and it's much straight forward for plugin user to understand.

I did a bit changes to gulp-rev here: https://github.com/ruanyl/gulp-easy-rev It is modified to fit my personal use case. You may want to take a look at :)