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

cdnify ignores paths in base URL #18

Open
billpatrianakos opened this issue Jan 13, 2015 · 14 comments
Open

cdnify ignores paths in base URL #18

billpatrianakos opened this issue Jan 13, 2015 · 14 comments
Labels

Comments

@billpatrianakos
Copy link

I'm having an issue where if my CDN URL contains a path like in the example (/stuff/) the URLs in the generated files do have the CDN URL prepended to them but they're missing the path. This is happening on images using root-relative paths.

My config looks like this:

cdnify: {
      dist: {
        options: {
          base: 'http://my.cdn.com/marketing/'
        },
        files: [{
          expand: true,
          cwd: '<%= app.dist %>',
          src: '**/*.{css,html,js}',
          dest: '<%= app.dist %>'
        }]
      }
    }

The generated URLs end up as http://my.cdn.com/img/whatever, missing the /marketing part. I made sure there were no other tasks doing work before or after CDNify that would interfere.

Right now I only notice it on images because they're the only thing referencing external assets in my project (I'm building a bunch of HTML emails).

Any ideas what could be wrong? For now my workaround is to use the rewriter function and match to any path where indexOf('/img') === 0 but that won't work when assets aren't root-relative and its more than just images to worry about.

@Climax777
Copy link
Contributor

This is a major problem for us +1

@kirkhoff
Copy link

kirkhoff commented Aug 5, 2015

@Climax777 and @billpatrianakos I created a PR for the fix. It's linked above.

@billpatrianakos
Copy link
Author

That's awesome. Thanks, @kirkhoff I think I might start using your fork until its officially merged. Thanks again!

@snowbillr
Copy link

👍 on this issue for us, we need this for our deployment

@XhmikosR
Copy link
Contributor

Does this still happen with the latest master?

@XhmikosR XhmikosR added the bug label Oct 17, 2015
@billpatrianakos
Copy link
Author

I've not used it since reporting the issue. I haven't seen any word on an official fix yet. I guess the best way to find out is to try and see.

@XhmikosR
Copy link
Contributor

We switched to ulr.resolve for the paths that is why I ask.

@kirkhoff
Copy link

I don't believe the fix I put in was ever merged. @callumlocke any chance we could have this added? I referenced my PR above.

@XhmikosR
Copy link
Contributor

Well the url fix is added.

Otherwise make clear pull requests with one feature per change and CC me.

Also make sure you add a test for the failing cases.
On Oct 19, 2015 16:38, "Curt" [email protected] wrote:

I don't believe the fix I put in was ever merged. @callumlocke
https://github.com/callumlocke any chance we could have this added? I
referenced my PR above.


Reply to this email directly or view it on GitHub
#18 (comment)
.

@kirkhoff
Copy link

@XhmikosR this is what I was referring to. If it's fixed, then 👍

@infomofo
Copy link

infomofo commented Dec 9, 2015

This does not seem to be fixed on version 1.0.2

@albogdano
Copy link

Not fixed! It worked fine in v0.1.1 then it got broken and it's the reason why I'm still on 0.1.1.

This actually works as expected, my CDN url in the base option was missing a slash at the end and that failed to resolve properly.

@XhmikosR
Copy link
Contributor

@albogdano: Then we'll need a clear test case.

@albogdano
Copy link

Ok, so I had the same problem but after investigating the issue it turns out the behavior of url.resolve() is correct and as expected. The problem people have comes from the fact the their relative URLs begin with /, for example:

<a href="/img/whatever/icon.png">

points to the root folder so it becomes

<a href="http://my.cdn.com/img/whatever/icon.png">

even if you set CDN URL to be http://my.cdn.com/marketing/.

The solution is to either remove the slash from the beginning of your relative URLs, or have an option that tells this module to remove these slashes for you. I think this should be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants