Normalize path relatively to CSS file when using separateCSS option #130

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@gseguin

gseguin commented Jan 24, 2014

Hi,

On the project I'm working on we have a bunch of modules that are output in a subdirectory of config.dir.
With the following requirejs options:

{
  appDir: "src",
  baseUrl: "foo",
  dir: "dist",

  separateCSS: true,

  modules: [ "bar/baz" ],
  // [ ... ]
}

Source src/foo/bar/baz.css

#image {
  background-image: url(images/kitten.png);
}

Would be compiled into:

#image {
  background-image: url(foo/bar/images/kitten.png);
}

which doesn't work since the real base url for that module is dist/foo/bar.

@guybedford

This comment has been minimized.

Show comment Hide comment
@guybedford

guybedford Jan 24, 2014

Owner

This behaviour is actually correct, see https://github.com/guybedford/require-css#siteroot-configuration

What are you expecting the output to be?

Owner

guybedford commented Jan 24, 2014

This behaviour is actually correct, see https://github.com/guybedford/require-css#siteroot-configuration

What are you expecting the output to be?

@gseguin

This comment has been minimized.

Show comment Hide comment
@gseguin

gseguin Jan 24, 2014

The expected output is:

#image {
  background-image: url(images/kitten.png);
}

but for css files that live in src/bower_components/** for instance when they get consolidated into dist/foo/bar/baz.css the paths would be transformed from images/myImage.png to ../../bower_components/myComponent/images/myImage.png.

The issue is for each modules in config.modules we have an associated index.html that lives in the same directory as the module so we want url to be relative to that directory. Another way of doing so would be to be able to provide a function to config.siteRoot but the computation of it would need to move to the onLayerEnd function where we know which module we're working on. Does that make sense?

gseguin commented Jan 24, 2014

The expected output is:

#image {
  background-image: url(images/kitten.png);
}

but for css files that live in src/bower_components/** for instance when they get consolidated into dist/foo/bar/baz.css the paths would be transformed from images/myImage.png to ../../bower_components/myComponent/images/myImage.png.

The issue is for each modules in config.modules we have an associated index.html that lives in the same directory as the module so we want url to be relative to that directory. Another way of doing so would be to be able to provide a function to config.siteRoot but the computation of it would need to move to the onLayerEnd function where we know which module we're working on. Does that make sense?

@guybedford

This comment has been minimized.

Show comment Hide comment
@guybedford

guybedford Jan 25, 2014

Owner

What is wrong with having a path like ../../bower_components/myComponent/images/myImage.png? It is the only way to truly know how to find those files absolutely. It is still finding the correct path from within the dist folder.

CSS URLs resolve relative to the CSS file itself, so as long as you are not moving the location of the CSS file, this path resolution will work no matter where your html file is.

Does that make sense? Let me know if I am missing something?

Owner

guybedford commented Jan 25, 2014

What is wrong with having a path like ../../bower_components/myComponent/images/myImage.png? It is the only way to truly know how to find those files absolutely. It is still finding the correct path from within the dist folder.

CSS URLs resolve relative to the CSS file itself, so as long as you are not moving the location of the CSS file, this path resolution will work no matter where your html file is.

Does that make sense? Let me know if I am missing something?

@gseguin

This comment has been minimized.

Show comment Hide comment
@gseguin

gseguin Jan 25, 2014

That's the issue. Here is how our directories are set up:

src/
  |-- bower_components/
  |    \-- myComponent/
  |        |-- myComponent.css
  |        \-- images/
  |           \-- myImage.png
  \-- foo
     \-- bar
       |-- main.js
         \-- baz.css

When we build we do a whole project build and basically have the same structure in the destination folder dist. The requirejs config as indicated earlier is:

{
  appDir: "src",
  baseUrl: "foo",
  dir: "dist",
  //[...]
}

The resulting dist/foo/bar/main.css has image urls for the bower components pointing to bower_components/myComponent/images/myImage.png which does not work, it should be ../../bower_components/myComponent/images/myImage.png.

I hope I clarified the issue a bit more.

gseguin commented Jan 25, 2014

That's the issue. Here is how our directories are set up:

src/
  |-- bower_components/
  |    \-- myComponent/
  |        |-- myComponent.css
  |        \-- images/
  |           \-- myImage.png
  \-- foo
     \-- bar
       |-- main.js
         \-- baz.css

When we build we do a whole project build and basically have the same structure in the destination folder dist. The requirejs config as indicated earlier is:

{
  appDir: "src",
  baseUrl: "foo",
  dir: "dist",
  //[...]
}

The resulting dist/foo/bar/main.css has image urls for the bower components pointing to bower_components/myComponent/images/myImage.png which does not work, it should be ../../bower_components/myComponent/images/myImage.png.

I hope I clarified the issue a bit more.

@guybedford

This comment has been minimized.

Show comment Hide comment
@guybedford

guybedford Jan 25, 2014

Owner

Ok, this is a bug then - it should be ../../bower_components/myComponent/images/myImage.png.

The issue is that siteRoot needs to be treated dynamically based on the output location. I'll aim to get a fix in soon.

Owner

guybedford commented Jan 25, 2014

Ok, this is a bug then - it should be ../../bower_components/myComponent/images/myImage.png.

The issue is that siteRoot needs to be treated dynamically based on the output location. I'll aim to get a fix in soon.

guybedford added a commit that referenced this pull request Jan 26, 2014

@guybedford

This comment has been minimized.

Show comment Hide comment
@guybedford

guybedford Jan 26, 2014

Owner

The latest commit should resolve this for you. Let me know if that works ok.

Owner

guybedford commented Jan 26, 2014

The latest commit should resolve this for you. Let me know if that works ok.

@gseguin

This comment has been minimized.

Show comment Hide comment
@gseguin

gseguin Jan 26, 2014

Fixed by 9a73e1b
Thanks!

gseguin commented Jan 26, 2014

Fixed by 9a73e1b
Thanks!

@gseguin gseguin closed this Jan 26, 2014

@guybedford

This comment has been minimized.

Show comment Hide comment
@guybedford

guybedford Jan 26, 2014

Owner

Great, thanks for reporting.

Owner

guybedford commented Jan 26, 2014

Great, thanks for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment