svgstore ignores `base` gulp.SRC option

[wc-matteo]

Given this pipeline:

gulp.src('foo/**/*.svg', { base: '.' })
  .pipe(svgstore())
  .pipe(gulp.dest('.'))

the plugin should output to foo, as do all plugins I tried (e.g. uglify); svgstore save its output on foo parent directory, instead.

Files

/
|--> gulpfile.js
|--> foo/
     |--> *.svg

[w0rm]

@wc-matteo this indeed looks to be the wrong behavior. Can you open a pull request to fix this together with the test case?

[wc-matteo]

@w0rm I'll try do something sometime later today.

[w0rm]

cool, thank you!

[wc-matteo]

Sorry. I'm very new to this kind of collaboration; and I really don't have time right now to figure it all out. Spending some time thinking (and tinkering) about the issue, I arrived at this:

var baseDir = path.resolve(file.path, '..') // file.dirname is undefined
fileName = file.clone(false)
fileName.path = path.join(baseDir, (baseDir.split(path.sep).pop() || 'svgstore') + '.svg')
fileName.contents = null

It should replace the code that assigns to fileName at 54-59; and fileName should become a vinyl file.

Then later, at 121-12:

fileName.contents = new Buffer($.xml())
this.push(fileName)

Disclaimer

I'm also very new to Gulp (and Node), so the code above might not cover all cases (or any but mine [or any at all, seeing that I did not test it thoroughly]). This should also fix the name of the output file: it was svgstore.svg and should have been foo.svg.

[w0rm]

@wc-matteo ok, I will check this when i have time, unfortunately it will be the earliest in two weeks. This is not the critical issue, and as a workaround you can pipe the result through gulp-rename to fix the output path.

[wc-matteo]

@w0rm Thank you.

Yes, I'm using gulp-rename like this for now:

var svgstoreBaseDir

...

.pipe(through(function (file, enc, callback) {
    if (!svgstoreBaseDir) svgstoreBaseDir = path.resolve(file.path, '..') // file.dirname is undefined
    callback(null, file)
}))
.pipe(svgstore())
.pipe(rename(function (file) {
    file.dirname = svgstoreBaseDir
    file.basename = (svgstoreBaseDir.split(path.sep).pop() || 'svgstore')
}))

[w0rm]

@wc-matteo in your original example

gulp.src('foo/**/*.svg', { base: '.' })
  .pipe(svgstore())
  .pipe(gulp.dest('.'))

base is the current directory for each file. If you want it to be foo then it should be:

gulp.src('foo/**/*.svg')
  .pipe(svgstore())
  .pipe(gulp.dest('.'))

I've pushed the change to support-base-option branch, please let me know if it works as expected.

[w0rm]

@wc-matteo I'm still not sure if this is the correct way. The way I thought of it before - was to use the last part of the base path for the target filename. e.g. foo/**/*.svg becomes foo.svg, src/icons/**/*.svg becomes src/icons.svg.

So I'm still not sure if foo/**/*.svg should become foo/foo.svg.

[wc-matteo]

@w0rm Yes, I chose a misleading example. The fact is that if I had other directories nested inside foo, I'd still want the file to be saved beside the svgs (e.g. foo/icons/*.svg => foo/icons/icons.svg). A base of . tells Gulp to keep the full path for the destination (and not just the path starting at the first glob).

Regarding the correct way, I agree, it doesn't seem something that'll work for all cases (I'm not convinced that the path before a glob should automatically be considered a base [I know that's what Gulp does. Still...]). In my code, I put the generated file in the icons' parent directory (e.g. foo/bar/icons/*.svg => foo/bar/icons.svg).

After work, I'll check out the branch. Thank you!

[w0rm]

I thought about this issue again, and I will keep the original implementation. I think it makes sense to have the combined file in the parent folder from the sources. If I change this logic then I have to bump the major version and educate the users about this breaking change and I don't really want to do this.

In any case, the destination path can be easily modified with gulp-rename