klei/gulp-inject

Using `?` tags in `addPrefix` seems to drop all assets

Opened this issue · 7 comments

flovan commented

Hi,

I might have discovered a bug. I first thought it might be related to my personal setup, which has become rather big, but this small test-bed has the same results:

var gulp = require('gulp');
var inject = require('gulp-inject');

gulp.task('default', function (cb) {
    gulp.src('test/index.html')
        .pipe(inject(gulp.src(['test/*.{js,css}']), {
            addRootSlash: false,
            addPrefix: 'test'
        }))
        .pipe(gulp.dest('test/out/test-regularl'));

    gulp.src('test/index.html')
        .pipe(inject(gulp.src(['test/*.{js,css}']), {
            addRootSlash: false,
            addPrefix: '<?php echo stuff(); ?>'
        }))
        .pipe(gulp.dest('test/out/test-php'));

    cb(null);
});

Test HTML document:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Document</title>
    <!-- inject:css --><!-- endinject -->
</head>
<body>
    <!-- inject:js --><!-- endinject -->
</body>
</html>

The first run will result in the files being injected as suspected, where the second run will not inject anything. Through trial & error, I managed to deduce that this problem occurs as soon as you insert a ? into the prefix string.

Edit; In case this might help:

OS X 10.11.1
node v4.1.2
gulp v3.9.0
gulp-inject v3.0.0
flovan commented

This relates to #71 and #107. You split the filestring on ?, which is probably needed for those two scenario's, but it's done after the prefix (and probably suffix) is added.

Looking into this..

humancatfood commented

@flovan

This would be a great fix, the addPrefix option is just perfect for php inserts!

My workaround for now is the transform-option. In your case that would be

gulp.src('test/index.html')
  .pipe(inject(gulp.src(['test/*.{js,css}']), {
    addRootSlash: false,
    transform: function (filepath) {
      return '<?php echo stuff(); ?>/' + filepath;
    }
  }))
  .pipe(gulp.dest('test/out/test-php'));
  • 👍3
humancatfood commented

I put in a pull request for this, but it's super-specific to php-code in the prefix.

Tbh, that might not be a bad thing :)

flovan commented

@humancatfood It might not seem bad atm, but the code that fixed #71 and #107 are directly related to causing this issue. I also put out a PR for this, but my implementation needs a major version bump which might not be wanted by the package author.

Don't know if @klei has any opinions on where we should head from here?

humancatfood commented

No, I meant it might not be too bad that my fix is so specific to php. It feels like less of a risk that it might break anything this way.

I know about #71 and #107 but my version passes both the corresponding tests and I added one for this issue too (#156).

peiledoir commented

If it helps anyone else, this is what I'm currently using for adding the prefix for the Wordpress template directory for css, js & html files. Thanks for the transform workaround @humancatfood! +1

return target.pipe(inject(sources, {
            addRootSlash: false,
            transform: function (filepath) {
                var e = filepath.split('.').pop();
                    if (e === 'css') {
                        return '<link rel="stylesheet" href="<?php echo get_template_directory_uri(); ?>/' + filepath + '">';
                    } else if (e === 'js') {
                        return '<script src="<?php echo get_template_directory_uri(); ?>/' + filepath + '"></script>';
                    } else if (e === 'html') {
                        return '<link rel="import" href="<?php echo get_template_directory_uri(); ?>' + filepath + '">';
                    }
            }
            })).pipe(gulp.dest('.'));
  • 👍3