gulp-rev icon indicating copy to clipboard operation
gulp-rev copied to clipboard

Option `base` do nothing

Open smoliakov opened this issue 9 years ago • 2 comments

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.

smoliakov avatar Feb 02 '16 14:02 smoliakov

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

matikun86 avatar Feb 17 '16 16:02 matikun86

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 :)

ruanyl avatar Mar 11 '16 10:03 ruanyl