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

Root-relative URLs do not work for JS files #287

Closed
davidtheclark opened this issue Feb 22, 2017 · 21 comments
Closed

Root-relative URLs do not work for JS files #287

davidtheclark opened this issue Feb 22, 2017 · 21 comments

Comments

@davidtheclark
Copy link
Contributor

Root-relative URLs for JS files cause errors like this: Error: Could not load script: "file:///test.js"/

Originally posted in #280 (comment), because I thought the issue was specific to the JSDom implementation. @RyanZim informed me, though, that the same error occurs with PhantomJS, it is just quietly ignored.

@vseventer suggested:

uncss allows /css/main.css, so there is no reason why /js/main.js shouldn't be supported also. This could be fixed by having htmlroot (or, more precise, utility.parsePath) apply to stylesheets and scripts (currently, it only applies to stylesheets).

@davidtheclark
Copy link
Contributor Author

Forgot to mention: I created a simple test repo to illustrate the problem at https://github.com/davidtheclark/uncss-jsdom-test.

@RyanZim
Copy link
Member

RyanZim commented Feb 22, 2017

uncss allows /css/main.css, so there is no reason why /js/main.js shouldn't be supported also.

I agree with @vseventer.

I don't know if/how this is possible via PhantomJS, however, in jsdom, this could be done via

config.resourceLoader: a function that intercepts subresource requests and allows you to re-route them, modify, or outright replace them with your own content.

I will submit a patch to the jsdom branch when I get a chance (unless someone gets there before me). When it is merged to the jsdom branch, I will publish a new version of uncss-jsdom.

@RyanZim
Copy link
Member

RyanZim commented Mar 27, 2017

Sorry so long; I finally got the patch released as uncss-jsdom v2.0.0

@mikelambert
Copy link
Collaborator

I've just released uncss 0.15.0, which I believe should include all of the above fixes.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 4, 2017

I'm hitting this with gulp-postcss and uncss 0.15.

var gulp = require('gulp');
var cssmin = require('gulp-clean-css');
var postcss = require('gulp-postcss')
var uncss = require('uncss').postcssPlugin;
var config = require('../../config').optimize.css;

gulp.task('optimize:css', function() {
    return gulp.src(config.src)
        .pipe(postcss(uncss(config.uncss)))
        .pipe(cssmin(config.options))
        .pipe(gulp.dest(config.dest));
});
module.exports = {
    optimize: {
        css: {
            uncss: {
                html: [buildDir + '/**/*.html'],
                htmlroot: buildDir,
                ignore: [
                    /\w\.in/,
                    '.fade',
                    '.collapse',
                    '.collapsed',
                    '.collapsing',
                    /(#|\.)navbar(-[a-zA-Z]+)?/,
                    /(#|\.)dropdown(-[a-zA-Z]+)?/,
                    /(#|\.)(open)/,
                    // injected via JS
                    /disabled/,
                    /\.no-js/,
                    /\.defer/
                ]
            },
            src: buildDirAssets + '/css/*.css',
            dest: buildDirAssets + '/css/',
            options: {
                level: {
                    1: {
                        specialComments: 0
                    }
                }
            }
        }
    }
};
C:\Users\xmr\Desktop\foo>node -v && npm -v
v8.2.1
5.3.0

C:\Users\xmr\Desktop\foo>npm run build

> foo@1.0.0 build C:\Users\xmr\Desktop\foo
> gulp production:build

[19:57:51] Using gulpfile ~\Desktop\foo\gulpfile.js
[19:57:52] Starting 'optimize:css'...

Error: Could not load script: "file:///C:/assets/js/pack.js"
    at C:\Users\xmr\Desktop\foo\node_modules\jsdom\lib\jsdom\browser\resource-loader.js:44:23
    at Object.check (C:\Users\xmr\Desktop\foo\node_modules\jsdom\lib\jsdom\living\nodes\Document-impl.js:89:11)
    at ReadStream.<anonymous> (C:\Users\xmr\Desktop\foo\node_modules\jsdom\lib\jsdom\living\nodes\Document-impl.js:108:12)
    at ReadStream.wrappedEnqueued (C:\Users\xmr\Desktop\foo\node_modules\jsdom\lib\jsdom\browser\resource-loader.js:255:16)
    at emitOne (events.js:115:13)
    at ReadStream.emit (events.js:210:7)
    at fs.js:1940:12
    at FSReqWrap.oncomplete (fs.js:135:15) { Error: ENOENT: no such file or directory, open 'C:\assets\js\pack.js'
  errno: -4058,
  code: 'ENOENT',
  syscall: 'open',
  path: 'C:\\assets\\js\\pack.js' }

I'm simply using <script src="/assets/js/pack.js" async></script> to load the script. Uncss even fails for the google analytics script which is in a JS file itself and is using protocol relative URL.

@mikelambert
Copy link
Collaborator

mikelambert commented Aug 4, 2017

Oh sorry, my comment above about 'fixing this' was wrong. I was overzealous in tagging this one.

No, scheme-relative and host-relative paths will not work when you are accessing local html files. If you are accessing C:/Users/you/Projects/web/file.html that references /assets/js/pack.js, then what is the correct place to look for pack.js? This is not a solvable problem given the information available.

Scheme-relative ones are also tricky, in that it's using a file:// scheme, and google-analytics JS will not work that way. You can just specify https:// in your path-to-google-analytics.js (instead of a scheme-relative path), and things should just work there. Basically, don't use scheme-relative...just use http or https instead, as appropriate (based on what the target resource is served under).

@XhmikosR
Copy link
Member

XhmikosR commented Aug 4, 2017

In the past IIRC htmlroot was being used. Did that change?

htmlroot (String): Where the project root is. Useful for example if you have HTML that references local files with root-relative URLs, i.e. href="/css/style.css"

The thing is that things worked with 0.14.1. And I also have the canvas issue which is why I haven't updated uncss in grunt-uncss.

These two issues are blockers and quite important IMO.

@RyanZim
Copy link
Member

RyanZim commented Aug 4, 2017

Just to clarify, are we talking about protocol-relative URLs or root-relative URLs?

@XhmikosR
Copy link
Member

XhmikosR commented Aug 4, 2017

Root-relative URLs. Protocol-relative URLs don't work too but that's a different issue.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

So, after specifying htmlroot I'm getting this:

[12:15:42] Starting 'optimize:css'...

events.js:182
      throw er; // Unhandled 'error' event
      ^
TypeError: css.walkRules is not a function
    at getUsedSelectors (C:\Users\xmr\Desktop\foo\node_modules\uncss\src\lib.js:129:9)
    at C:\Users\xmr\Desktop\foo\node_modules\uncss\src\lib.js:226:16
    at tryCatcher (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\util.js:16:23)
    at MappingPromiseArray._promiseFulfilled (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\map.js:61:38)
    at MappingPromiseArray.PromiseArray._iterate (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\promise_array.js:114:31)
    at MappingPromiseArray.init (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\promise_array.js:78:10)
    at MappingPromiseArray._asyncInit (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\map.js:30:10)
    at Async._drainQueue (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\async.js:138:12)
    at Async._drainQueues (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\async.js:143:10)
    at Immediate.Async.drainQueues (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\async.js:17:14)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

@mikelambert
Copy link
Collaborator

mikelambert commented Aug 7, 2017

Did you investigate this issue at all, or are you just sharing it here? Do you need help debugging?

What version of postcss are you using, from the postcss/package.json? Do you have a 'walkRules' in your postcss package?

And from the other angle, what is the 'css' object passed into that function? Is it a valid postcss object? What methods does it have on it?

@RyanZim
Copy link
Member

RyanZim commented Aug 7, 2017

@mikelambert This is an internal uncss error coming from https://github.com/giakki/uncss/blob/master/src/lib.js#L129.

The postcss version would be the same as what uncss is using. postcss does have a walkRules function. It seems somehow, uncss is passing that function an object other than a postcss root object.

@RyanZim
Copy link
Member

RyanZim commented Aug 7, 2017

@XhmikosR is it possible to reproduce this issue without using grunt? If so, I'd be willing to investigate if you could make a test case repo or gist.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

That is not with Grunt. It's with gulp-postcss and uncss.

@mikelambert
Copy link
Collaborator

mikelambert commented Aug 7, 2017

Thanks @RyanZim on volunteering on the last bit.

And yes, I'm aware the uncss code is emitting the exception, and that uncss has a dependency on postcss that should contain the walkRules. But I also know @XhmikosR has various uncss checkouts of uncss as a developer with commit privileges, so wanted to do a sanity-check that the dependencies were properly reinstalled in his/her checkouts. Especially since it's not clear what steps @XhmikosR has taken in debugging the issue yet.

But yes, I am also curious what object is actually being passed into the getUsedSelectors function.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

Nah, this is with vanilla uncss 0.15, see #287 (comment)

I can't find why this happens. If I could I would have said so :P

I mean, the css file is there, generated with sass before the uncss task runs. The html files are there too. I can't figure out why this error.

@mikelambert
Copy link
Collaborator

mikelambert commented Aug 7, 2017

Ok thanks. Can you provide a self-contained repro case? Or can you log and share the values I requested above? Namely, what is the css object being passed into getUsedSelectors ?

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

[01:08:50] Starting 'optimize:css'...
>> uncss - css: <File "main.css" <Buffer 2f 2a 21 0d 0a 20 2a 20 42 6f 6f 74 73 74 72 61 70 20 76 34
 2e 30 2e 30 2d 61 6c 70 68 61 2e 36 20 28 68 74 74 70 73 3a 2f 2f 67 65 74 62 6f 6f 74 73 ... >>

events.js:182
      throw er; // Unhandled 'error' event
      ^
TypeError: css.walkRules is not a function
    at getUsedSelectors (C:\Users\xmr\Desktop\foo\node_modules\uncss\src\lib.js:130:9)
    at C:\Users\xmr\Desktop\foo\node_modules\uncss\src\lib.js:227:16
    at tryCatcher (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\util.js:16:23)
    at MappingPromiseArray._promiseFulfilled (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\map.js:61:38)
    at MappingPromiseArray.PromiseArray._iterate (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\promise_array.js:114:31)
    at MappingPromiseArray.init (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\promise_array.js:78:10)
    at MappingPromiseArray._asyncInit (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\map.js:30:10)
    at Async._drainQueue (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\async.js:138:12)
    at Async._drainQueues (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\async.js:143:10)
    at Immediate.Async.drainQueues (C:\Users\xmr\Desktop\foo\node_modules\bluebird\js\release\async.js:17:14)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

Unfortunately I cannot share the repo so if this info doesn't help, I'll need to try to make a small test case.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

Ok, I found it! Stupid mistake...

.pipe(postcss([uncss(config.uncss)]))

It needs to be an array.

@RyanZim
Copy link
Member

RyanZim commented Aug 7, 2017

Glad you found it. Them bugs can be mean. Can we close this now?

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

I think this issue is no longer valid, yeah. Root-relative files work fine with htmlroot.

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

4 participants