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

Difficulty using stream method with Gulp and a glob pattern. #119

Closed
Tempurturtul opened this issue Jan 25, 2016 · 5 comments
Closed

Difficulty using stream method with Gulp and a glob pattern. #119

Tempurturtul opened this issue Jan 25, 2016 · 5 comments
Assignees

Comments

@Tempurturtul
Copy link

As demonstrated here, I'm attempting to use the stream method to inline critical CSS for all .html files matched by a glob pattern. My usage differs in that I'm also matching .html files in subdirectories (dist/*.html vs dist/**/*.html).

The problem I'm having is that critical's base option needs to be redefined for each subdirectory in order for relative paths to CSS files to work.

Example folder structure:

dist/
  style.css
  index.html
  views/
    foo.html

Problematic html:

<!-- views/foo.html -->
<link rel="stylesheet" href="../style.css">

My gulp task:

gulp.task('critical', function() {
  gulp.src('dist/**/*.html')
    .pipe(critical({
      inline: true,
      base: 'dist/'
    }))
    .pipe(gulp.dest('dist'));
});

The above returns "Fatal undefined" because ../style.css cannot be found in foo.html with the base being dist/. In order to get this working base needs to be dist/ for index.html and dist/views/ for foo.html (and so on for additional files). I'm not sure how to achieve this. Excluding base doesn't work at all (I'd hoped the default would be the value I'm looking for since this method seems to be included primarily for usage with Gulp).

@bezoerb bezoerb self-assigned this Jan 25, 2016
bezoerb added a commit that referenced this issue Jan 28, 2016
@bezoerb bezoerb mentioned this issue Jan 28, 2016
@bezoerb
Copy link
Collaborator

bezoerb commented Jan 29, 2016

@Tempurturtul i've provided a fix in the vinyl branch. Could you check if this fixes your issue?

npm install --save-dev git://github.com/addyosmani/critical#vinyl

@Tempurturtul
Copy link
Author

Thanks bezoerb. Here's the error I'm now receiving in all cases (without Plumber, output is "Fatal undefined"):

[07:35:29] Starting 'critical'...
[07:35:29] Finished 'critical' after 30 ms
[07:35:29] Plumber found unhandled error:
 Error in plugin 'critical'
Message:
    Path must be a string. Received <File "index.html" <Buffer 3c 21 44 4f ... >>
[07:35:29] Plumber found unhandled error:
 Error in plugin 'critical'
Message:
    Path must be a string. Received <File "views/foo.html" <Buffer 3c 21 44 4f ... >>

@bezoerb
Copy link
Collaborator

bezoerb commented Jan 29, 2016

@Tempurturtul there was an incompatibility to vinyl < v0.5.3. This should be fixed now by 846c233

@Tempurturtul
Copy link
Author

Confirmed; thank you!

addyosmani pushed a commit that referenced this issue Oct 17, 2016
* add testcase for #119

* use vinyl

* Keep relative images relative

We now can campute the path prefix for asset references based on the location of the html file.
This allows us to keep relative asset urls in stylesheets relative. Issues #57 and #95

* consider absolute stylesheet refs

* Guess source path for piped html

try to guess source path of the file by checking relative links inside the html source
show warning to the user when no relative links are available

* tests for auto pathPrefix

* travis tweaks

some weird issue requires a second npm install to add all deps
without this module minimist required by gulp-util is missing

* eslint fixes

* minor changes

* doc blocks

* compatibility to vinyl < v0.5.3

* Check vinyl

* Check vinyl

* remove unused setMaxListeners(0)

* guess path to html source

also consider script sources

* fixed jsdoc

* add 'folder' option

* updated jsdoc

* text change

* WiP

* fixed tests

* run all tests

* some text changes

* fixed tests

* force re-run appveyor after penthouse fix

* fixed tests

* fixed last test
@bezoerb
Copy link
Collaborator

bezoerb commented Jun 30, 2017

Closed as this seems to be fixed. Feel free to reopen if the error still exists

@bezoerb bezoerb closed this as completed Jun 30, 2017
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

2 participants