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

Buggy options.base behaviour #11

Open
piotr-cz opened this issue Aug 29, 2016 · 3 comments
Open

Buggy options.base behaviour #11

piotr-cz opened this issue Aug 29, 2016 · 3 comments

Comments

@piotr-cz
Copy link

When specyfing options.base, it's usage is being over overriden by options.dest here because options.dest always contains a string.

This is a follow up for #3 (comment)
I came up with exactly similar hack, which worked for me, however it broke tests.

I was not able to setup rev-del as a gulp plugin for following file structure:

public/
    rev-manifest.json
    css/
         styles-497b00d23b.min.css

with the contents of rev-manifest.json:

{
  "css/styles.min.css": "css/styles-497b00d23b.min.css"
}

code I've used:

var pathBuild = './public';

gulp.src('./resources/assets/css/styles.css')
  // It's required to prepend subfolder so gulpRev uses it in manifest
  .pipe(gulpRename({dirname: 'css', basename: 'styles', 'suffix': '.min'}))
  .pipe(gulpRev())
  // Store compiled styles.min.css file
  .pipe(gulp.dist(pathBuild))
  // Configure manifest
  .pipe(gulpRev.manifest({
    path: pathBuild + '/rev-manifest.json'
    base: pathBuild
  }))
  // Clean previous versioned files
  .pipe(revDel({
    base: pathBuild
  }))
  .pipe(gulp.dest(pathBuild))
@silverbackdan
Copy link
Contributor

I'm no expert but I thought I'd share what I've found.

I spent a little while on this as well but realised just now that you can change dest to the base path for the files you'd like to delete and then change your oldManifest to the path of your old JSON manifest to avoid also prepending the dest to your manifest path. I was having a play around with the files and found a couple of ways to possibly address the issue before realising that it can be done already with the options documented and without using 'base'.

It does mean that the base option may be redundant especially on that line where dest is always set. But I'll leave that to the author to explain/decide.

@silverbackdan
Copy link
Contributor

silverbackdan commented Feb 15, 2017

As an alternative if wanted, I've uploaded a fork where fileBase can be specified and therefore would not be prepended to the manifest path as dest is. I also added tests and removed the ability to set base ... honestly though I haven't submitted a PR with the change because I'm not quite sure what the intention of the 'base' variable was initially and am unsure if there are other repercussions (although tests will pass with this solution). https://github.com/silverbackdan/rev-del/blob/options.base/index.js

@julien-mustiere
Copy link

julien-mustiere commented Mar 21, 2017

Hi, maybe too late but I was the same issue.

I use "oldManifest" instead of "base", and I think you should to set up the destination like so:

.pipe(revDel({
    oldManifest: pathBuild + '/rev-manifest.json'
    dest: './css'
  }))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants