jonkemp / gulp-useref

Parse build blocks in HTML files to replace references to non-optimized scripts or stylesheets.
MIT License
705 stars 93 forks source link

Assumes all HTML files are at the root? #156

Closed callumlocke closed 8 years ago

callumlocke commented 8 years ago

Directory structure:

src/
  - index.html
  - styles/
    - main.css
  - pages/
    extra.html

Code:

  gulp.src('src/**/*html')
    .pipe($.useref())
    .pipe($.if('*.css', $.minifyCss({compatibility: '*'})))
    .pipe(gulp.dest('dist'))

Both the HTML files load the same main.css file, using relative paths:

This works fine in the browser. But when I use gulp-useref, when it's processing extra.html it gives this error: Error: Error: File not found with singular glob: ~/code/my-project/styles/main.css (note that it has climbed right up out of the src directory).

It looks like useref is ignoring the fact that extra.html is one level deeper than the src root, and just always searches for assets relative to the root (rather than relative to dirname of the actual HTML file).

I tried changing the link tag in extra.html to remove the ../, and this helps useref to find the file, but it means my code doesn't work in development before useref.

I've tried setting base to "dist" but it makes no difference.

The same incorrect assumption (that all HTML files are at root) seems to apply to the path in a build block comment too – a relative output path like <!-- build:css ../styles/.bundle.css --> actually writes the file right outside my project.

jonkemp commented 8 years ago

Related to #118. Reopened it.

callumlocke commented 8 years ago

I'm not sure if this bug can be addressed with the base option, because a project might contain multiple files at various different levels, whereas the base option applies to everything (if I understand correctly).

The solution is to resolve all file-relative asset paths relative to the dirname of the referring HTML file (i.e. what browsers do). Only root-relative asset paths (those starting with /) should be resolved relative to the vinyl base.

jonkemp commented 8 years ago

Anybody want to write a test case?

jonkemp commented 8 years ago

Change 9906172 should fix this issue.

Should be fixed in the next release.

jonkemp commented 8 years ago

high five

lukakemperle commented 8 years ago

@jonkemp, great, this fix works! -Thank you!