output paths should be relative to source paths

[bendavis78]

Take this directory structure for example:

src/index.html
src/foo/bar.html
src/baz/qux/corge.html

When you have a task like this:

gulp.task('build-vulcanize', function() {
  return gulp.src('src/**/*.html')
    .pipe(vulcanize({
      dest: 'build',
      csp: true,
      inline: false
    }));
});

It puts all output files under the build directory, instead of in their respective folders:

build/index.html
build/bar.html
build/corge.html

[sindresorhus]

// @ragingwind

[ragingwind]

@sindresorhus I think, It would be good if dest option will be removed. We can use gulp.dest instead of. also gulp.rename

[sindresorhus]

@ragingwind The dest option is used to calculate the relative paths in the actual source (which isn't possible without it AFAIK) and has nothing to do with outputting the files:

gulp-vulcanize/index.js

Line 28 in 5e83292

options.output = path.join(options.dest, path.basename(file.path));

[ragingwind]

options.output would be ignored while file writing If outputSrc(outputHandler) was passed. (they have changed the name to outputHandler). we don't have to pass options.output If this plugin supports only stream out. (this plugin pass outputSrc option to vulcanize always).

[sindresorhus]

Oh, I thought options.output was used to calculate relative paths, but if it's not used I guess we can remove the dest option, which would be really nice.

I guess we should also change outputSrc to outputHandler then.

// @azakus In case we're missing something.

[ragingwind]

ok. I'll make a new PR for this.

[ebidel]

cc @brendankenny

[ragingwind]

@sindresorhus @azakus Sorry. I have misunderstood, output option must be passed to vulcanize that used to calculated relative paths in vulcanized html. I had concerned a path of vulcanized html's path. To some extent, user should be passed vulcanize options If it seems to be not gulp way. And I'll push a PR this issue with grunt-vulcanize test fixtures. please review that.

[bendavis78]

I was curious to see why gulp-vulcanize even needed to write anything to the filesystem (as opposed to just using vynyl). I went ahead and refactored the plugin to do this -- it has the added benefit of fixing this bug, and doesn't leave behind unecessary dot-files. I'm not sure whether or not this breaks any features, but it seems to work for my use cases. https://github.com/bendavis78/gulp-vulcanize/blob/refactor/index.js

[ragingwind]

@bendavis78 Did you remove that bendavis78@f3d1f36#diff-168726dbe96b3ce427e7fedce31bb0bcL28 with unused modules? Yes It's almost same with my version.

My concern is that in case of vulcanize couldn't calculate relative path correctly if grunt-vulcanize doesn't pass output options to vuclanize. case is that https://github.com/Polymer/grunt-vulcanize/blob/master/test/expected/csp/vulcanized.html#L4

in your case(or cases that doesn't need to changed). have no problems(maybe). <script src="../bower_components/..."></script> works well

\bower_components
\src\index.html
\build\vulcanized.html

\src\statics
\src\index.html
\build\statics // copied from src
\build\vulcanized.html

but we have to consider abspath(maybe) option and the all of other cases see below. If we don't pass output path it would be not generated script src path correctly. vulcanize try to calculate relative path by `/fixtures/vulcanized.html'

\fixtures\bower_components\
\fixtures\index.html  `<script src="bower_components/..."></script>`
\temp\vulcanized.html scirpt src should be changed to  `<script src="../fixtures/bower_components/..."></script>`

[bendavis78]

@ragingwind, I'm confused -- are we talking about grunt or gulp?

[bendavis78]

Sorry, I was just looking at your link and got confused. I think I see what you mean by the relative path not being considered in my version. Perhaps vulcanize needs a patch to allow for this option while still allowing the use of buffers only. I may dig into vulcanize a little more to see what's possible.

[ragingwind]

@bendavis78 absolutely gulp. What point is that makes you confuse? To give you an idea. I use a testcase of grunt-vulcanize for gulp-vulcanize

[sindresorhus]

I was curious to see why gulp-vulcanize even needed to write anything to the filesystem (as opposed to just using vynyl).

Because Vulcanize only recently supported string in/out.

[sindresorhus]

Fixed by 2179cef