search

componentjs / builder.js

v0.x of the builder. v1.0.0+ is located at:
https://github.com/component/builder2.js
71 stars 39 forks source link

css url rewrite for fonts is broken in dev mode #142

Open amasad opened 10 years ago

amasad commented 10 years ago

After updating to component v0.19.1 from v0.18 the url rewrite for font (and assuming images) is broken because of the change from being relative to the css file to absolute. Note the addition of the leading / . Try:

component install FortAwesome/Font-Awesome
component build --dev
cat build/build.css | grep src

see:

  src: url("/FortAwesome-Font-Awesome/fonts/fontawesome-webfont.eot?v=4.0.3");
  src: url("/FortAwesome-Font-Awesome/fonts/fontawesome-webfont.eot?#iefix&v=4.0.3") format('embedded-opentype'), url("/FortAwesome-Font-Awesome/fonts/fontawesome-webfont.woff?v=4.0.3") format('woff'), url("/FortAwesome-Font-Awesome/fonts/fontawesome-webfont.ttf?v=4.0.3") format('truetype'), url("/FortAwesome-Font-Awesome/fonts/fontawesome-webfont.svg?v=4.0.3#fontawesomeregular") format('svg');
jonathanong commented 10 years ago

what did you expect it to be? looks fine to me if you serve /build from /. IIRC the only difference is that the new builder removes a /../ in the middle

amasad commented 10 years ago

Ideally, it should be relative to the css file, i.e. no leading /. So if I'm serving from root and my css is in /build/css/build.css then FortAwesome-Font-Awesome/fonts works fine but /FortAwesome-Font-Awesome/fonts won't work. Thoughts?

amasad commented 10 years ago

I usually structure my apps in that there is an index.html at the root: https://github.com/max99x/guesshub/tree/master/app https://github.com/amasad/debugjs.com If you think that's valid I'll look into fixing and sending a PR

jonathanong commented 10 years ago

oh yeah that's a bug. there shouldn't be a leading /.

it should be fixed though since component reverted to an older version of builder. we're probably going to just rewrite the builder instead of releasing a new version, so i'm not sure if a PR would be helpful. maybe a test case? i'll leave this open for now though.

ideally, the registry would also be a server, so instead of rewriting these assets to relative paths, we can just rewrite them against the registry like //component.io/FortAwesome/Font-Awesome/v4.0.3/fonts and component wouldn't ever have to bother downloading them at all. same would go for other assets like images. would increase installation times :)

amasad commented 10 years ago

oh yeah that's a bug. there shouldn't be a leading /.

it should be fixed though since component reverted to an older version of builder. we're probably going to just rewrite the builder instead of releasing a new version, so i'm not sure if a PR would be helpful. maybe a test case? i'll leave this open for now though.

Cool, I'll update this issue to make it clear. out of curiosity where do you guys discuss component issues etc?

ideally, the registry would also be a server, so instead of rewriting these assets to relative paths, we can just write them against the registry like //component.io/FortAwesome/Font-Awesome/v4.0.3/fonts and component wouldn't ever have to bother downloading them at all.

That'd be nice, but many would still prefer local for development.

jonathanong commented 10 years ago

there's https://github.com/component/spec for specs.

yeah, it would be opt-in.

jonathanong commented 10 years ago

haven't seen this error in the latest component and i'm using font-awesome as well. can you confirm?

amasad commented 10 years ago

on 0.19.5 leading / still there

jonathanong commented 10 years ago

interesting. i can't reproduce =/

amasad commented 10 years ago

remember it only happens with --dev

ianstormtaylor commented 10 years ago

+1 seeing this too. also not just fonts, but any urls (in case that wasn't obvious haha)

matthewmueller commented 10 years ago

running into this as well