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

Custom-output-path-aware relative-URLs #2084

Closed
am11 opened this issue Jul 1, 2014 · 21 comments
Closed

Custom-output-path-aware relative-URLs #2084

am11 opened this issue Jul 1, 2014 · 21 comments

Comments

@am11
Copy link
Contributor

am11 commented Jul 1, 2014

Please change toCSS() so it takes custom output file path and make it produce the output with correct relative url()s.

Currently its producing urls relative to the source less file, whereas it should incorporate the intended location of css to be generated.

@lukeapage
Copy link
Member

See existing options (http://lesscss.org/usage/#command-line-usage-rootpath) and then if it doesn't fit your criteria, explain more about what your situation is and what you want to happen.

e.g. imports or normal paths? give an example of whats in your less and what you want to see in your css.

@am11
Copy link
Contributor Author

am11 commented Sep 22, 2014

Hello, sorry for the late response. I was busy with other stuff.

In Web Essentials, we are calling less as a service to node-server: https://github.com/madskristensen/WebEssentials2013/blob/eea5cd4d82ab36c6fc7bab4e44fa123e11a3d723/EditorExtensions/Resources/server/services/srv-less.js#L52-L75.

Now, given the following LESS:

foo {
    + bar {
        color: lime;
        background-image: url('../7up-fido-dido.png');
    }
}

It generates:

foo + bar {
  color: lime;
  background-image: url(../7up-fido-dido.png);
}
/*# sourceMappingURL=Site.css.map */

Now, let's consider if filename: params.sourceFileName at line 28, points to c:\\project\\content\\less\\input.less and rootPath is set to path.basename(params.targetFileName) which points to c:\\project\\content\\less\\nested\\folder\\, the resultant CSS would be the same.

Also, if I add rootPath: path.basename(params.targetFileName) at line 57 (as a setting to toCSS) or line 28 (in less.Parser's settings), the result is still same.

However, the expected output in the latter case should be:

foo + bar {
  color: lime;
  background-image: url(../../../7up-fido-dido.png);
}
/*# sourceMappingURL=Site.css.map */

Is there any setting for this to generate URLs relative to target file (as opposed to input/source file)?

(For this matter, we need to carry out this post-processing step, using VS' CSS AST, which is a bit expensive op).


On a slightly different note, would be nice if the file and sources members of source-map JSON are also relative to each other to avoid this hack. We fixed the similar issue in libsass some time ago. The gist is:

  • SourceComment in CSS file should point to sourcemap-filename relative to CSS file path.
  • file in sourcemap JSON file should point to CSS file path relative to .map file path.
  • Each member in sources array in sourcemap JSON should point to source .less files, relative to .map file path.

Thanks in anticipation.

@seven-phases-max
Copy link
Member

... points to c:\project\content\less\input.less and rootPath is set to path.basename(params.targetFileName) which points to c:\project\content\less\nested\folder\ ,...

If I understand it right you'll get the desired result if you set relative path for rootPath instead of absolute path.basename(params.targetFileName).

@am11
Copy link
Contributor Author

am11 commented Sep 22, 2014

@seven-phases-max, unfortunately it doesn't work with:

rootPath: path.dirname(path.relative(params.sourceFileName, params.targetFileName))

or

rootPath: path.relative(params.sourceFileName, params.targetFileName)

or

rootPath: path.dirname(params.targetFileName)

both in toCSS({ /* here */ }) and / or new (less.Parser)({ /* here */ }).. Still giving me ../7up-fido-dido.png.

@seven-phases-max
Copy link
Member

@am11

Hmm, it's interesting because specifying (for example) -rp="../../" for lessc or rootpath="../../" for client-side less.js both give the expected result. E.g. for:

x {
   y: url(../z.png);
}

both produce:

x {
   y: url(../../../z.png);
}

@seven-phases-max
Copy link
Member

I've also tested the following minimal node script with Less 1.6.3 and 1.7.5:

var parser = new(require('less').Parser)({
  rootpath: "../../"
});

parser.parse('x {y: url("../z.png")}', function (e, tree) {
   console.log(tree.toCSS());
});

and it compiles to:

x {
  y: url("../../../z.png");
}

P.S. Ah, @am11 Sorry I did not notice it initially, it looks like the problem in your tests is just in rootPath - should be rootpath instead.

@am11
Copy link
Contributor Author

am11 commented Sep 22, 2014

Lol, yeah that was the issue. Thanks!

I had to use this:

rootpath: path.dirname(path.relative(path.dirname(params.targetFileName), params.sourceFileName)).replace(/\\/g, '/') + "/"

Looks bit ugly but, less expensive than the one we have! :)

@am11 am11 closed this as completed Sep 22, 2014
@am11
Copy link
Contributor Author

am11 commented Sep 22, 2014

@seven-phases-max, regarding the source-map issue described above (after the horizontal-row), is there a workaround for that too?

@seven-phases-max
Copy link
Member

@am11, Sorry no idea about source maps, I guess it would make sense to create a separate ticket for that.

@lukeapage
Copy link
Member

On a slightly different note, would be nice if the file and sources members of source-map JSON are also relative to each other to avoid this hack. We fixed the similar issue in libsass some time ago. The gist is:

SourceComment in CSS file should point to sourcemap-filename relative to CSS file path.
file in sourcemap JSON file should point to CSS file path relative to .map file path.
Each member in sources array in sourcemap JSON should point to source .less files, relative to .map file path.

I've been fixing this on the v2 branch, but my fix is in lessc, so I think you do not need v2 to get it working.

for reference, that commit was c07531a

for calling less programmatically, just make sure the following are set correctly.

sourceMapFilename - this comes out as the filename in the css which points to the map
sourceMapOutputFilename - this comes out as the css filename in the file property of the map

The only complication is that sourceMapFilename (but not outputFilename) gets put through the following options

sourceMapBasepath - a path to remove from all urls in the sourcemap, and the path to the sourcemap
sourceMapRootpath - a path to add to all urls in the sourcemap and the path to the sourcemap

I think in v2 I am going to change it so that the sourcemap filename is not processed by the above options and they only effect the mapping of urls in the sourcemaps content.

@lukeapage
Copy link
Member

On a different note, it may be of interest to you that v2 plans to be a pure javascript library that talks to its environment through a set interface.. making it easier to embed in a .net assembly through a v8 interpretter

@am11
Copy link
Contributor Author

am11 commented Sep 22, 2014

@lukeapage, thanks for your reply and your patience. I will keep an eye on v2. 👍

We will keep using node.js to interconnect less, sass and other services in WE (as we have one set base for NPMs).

@am11
Copy link
Contributor Author

am11 commented Oct 12, 2014

@seven-phases-max, I have yet another test, which is failing:

In /temp/projects/less/test1.less:

@import 'test2';
@import 'less_includes/test3';

In /temp/projects/less/test2.less:

.a {
  background-image: url('../images/a.png');
}

In /temp/projects/less/less_includes/test3.less:

.b {
  background-image: url('../images/b.png');
}

So, there are two images folders: /temp/projects/less/images/ and /temp/projects/images/.
Now running:

/temp/projects/less$ node node_modules/less/bin/lessc --rootpath="../" test1.less output/style.css

yields this output:

.a {
  background-image: url('../../images/a.png');
}
.b {
  background-image: url('../../images/b.png');
}

Whereas, the expected output is:

.a {
  background-image: url('../../images/a.png');
}
.b {
  background-image: url('../images/b.png');  // <-- this 
}

Apparently, it doesn't account for the relevance of the location of imported source file.

The requirement is: Resolve all assets paths w.r.t. output file location.

@lukeapage, should I reopen this issue? :)

@lukeapage
Copy link
Member

@am11 use the flag --relative-urls to make the urls relative to the file they are in. Try that.

@am11
Copy link
Contributor Author

am11 commented Oct 12, 2014

@lukeapage, you rock!!! This worked and thank you! 😃 👍

node node_modules/less/bin/lessc --rootpath="../" --relative-urls test1.less output/style.css

am11 added a commit to am11/WebEssentials2013 that referenced this issue Jul 27, 2015
This is for Less compiler can resolve the relative path.
I somehow forgot the incorporate this bit.. 😞

Related discussion: less/less.js#2084
Fixes madskristensen#1922.
@stevenvachon
Copy link

stevenvachon commented Aug 28, 2016

I have this:

// ~/project/node_modules/css-framework/less/include/variables.less
@css-framework-fonts: "../../fonts/";
// ~/project/node_modules/css-framework/less/include/fonts.less
@font-face {
  src: url("@{css-framework-fonts}some-font.woff2") format("woff2");
}
// ~/project/assets/less/include/variables.less
@import "../../../node_modules/css-framework/less/include/variables";
@css-framework-fonts: "../../../../assets/vendor/css-framework/fonts/";
// ~/project/assets/less/include/icon.less
.classname {
  background-image: url("../../images/icons/some-icon.svg");
}

With relativeUrls:true and rootpath:":" in ~/project/gulpfile.js, the end result is:

/* ~/project/style.css */
@font-face {
  src: url("assets/vendor/css-framework/fonts/some-font.woff2") format("woff2");
}
.classname {
  background-image: url("../images/icons/some-icon.svg");
}

when this was expected:

/* ~/project/style.css */
@font-face {
  src: url("assets/vendor/css-framework/fonts/some-font.woff2") format("woff2");
}
.classname {
  background-image: url("assets/images/icons/some-icon.svg");
}

@am11
Copy link
Contributor Author

am11 commented Aug 29, 2016

@stevenvachon, I ended up using via API like this:

rootpath: path.dirname(
  path.relative(
    path.dirname(absoluteTargetFileName),
    absoluteSourceFileName)
  ).replace(/\\/g, '/') + '/',

Full story: https://github.com/madskristensen/WebEssentials2013/blob/master/EditorExtensions/Resources/server/services/srv-less.js

I guess this is also applicable in gulpfile. If not then please share that file, might help spotting the issue. :)

@stevenvachon
Copy link

stevenvachon commented Aug 29, 2016

@am11, that produces:

@font-face {
  src:url("../../../../assets/vendor/css-framework/fonts/some-font.woff2") format("woff2");
}
.classname {
  background-image: url("../../../images/icons/some-icon.svg");
}

which is even less correct.

My gulpfile wouldn't help as it's full of plugins and the actual Less.js part is very small. The important parts are mentioned in my previous comment.

@stevenvachon
Copy link

stevenvachon commented Aug 29, 2016

I think the problem is that relativeUrls is relative to the "base imported file" and not the file itself. This is a problem. CommonJS does not work this way and neither should Less.js

@am11
Copy link
Contributor Author

am11 commented Aug 29, 2016

Alright, I was under the impression that if the target and source files are given as absolute paths, and relativeUrls: true (just a plain boolean value), then setting rootpath like that would yield the correct result in all the cases.

@stevenvachon
Copy link

For now, I'm using paths relative to the project root because relativeUrls is not behaving as expected. Can't wait for Sass v4.

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

No branches or pull requests

4 participants