Fix the differences between client-side and server-side handling of relative urls and imports

[loic]

Pull request to follow up on discussion in #331.

[lukeapage]

@MatthewDL @Synchro Can we start a discussion over this?

How about we pull (at least some of) this so that browser behaviour is consistent with serverside? If serverside is the recommended usage then we should break the minimum number?

As a second step we could make it configurable whether less re-writes urls and then make this behaviour work browser side and serverside?

I suggest we start this after releasing 1.3.1 so we do not break anything with what is essentially a bug fix release.

@MatthewDL any chance you could speak to @cloudhead about getting access to allow us to go ahead with a release?

[matthew-dean]

Start a discussion here or on Google Groups?

I've messaged @cloudhead and will let you know.

[lukeapage]

I don't mind where

[kevinpschaaf]

@MatthewDL @agatronic: Did you guys sync on the path forward for normalizing browser & node behavior w.r.t. relative URL's? I'd like to weigh in and make the case for keeping URL-rewriting based on the path of the file that defines the URL (current browser behavior), although I imagine valid cases can be made either way, so I hope we land on a configurable way to choose the behavior if not.

I'm working on the Enyo framework and we plan to use Less as our official method of theming library UI components, so URL rewriting is an important piece when importing library styles (with URL's to assets) into an arbitrary application sheet.

We're willing to contribute, would love to weigh in. Let me know how I can help out.

[SalimBensiali]

Hi all,

Please have a look at my contribution #976 to address issue. I got inspired by @loic's pull request and implemented Solution 2 he mentionned. Any feedback is welcome.

Cheers,
Salim

[loic]

@kevinpschaaf the current browser behavior doesn't perform any URL-rewriting. The generated CSS is loaded as-is and the browser calculates all relative paths based on the URL of the web page (let's say the .html file). Obviously we can't replicate that for server-side usage.

@SalimBensiali I briefly reviewed and tested your patch and it seems to work for server-side usage but not in the browser. I suggest we leave the folder test-relative-paths (or an improved version of it) for manual testing in the upcoming pull requests until we get a working solution. To automate this kind of in-browser / server-side comparative tests maybe we could leverage something like phantomjs?

[lukeapage]

timeline=

1. Release next version of less.s (@MatthewDL organising access, hopefully soon)
2. Decide approach for paths
3. merge/implement

This order because the paths thing will break someones setup whatever happens, so I would rather get the bugfix release out.

[SalimBensiali]

@loic Thanks for the feedback and sorry for not getting back to you earlier. You are definitely right about having a testing set up for the in-browser environment. The manual testing I did with an existing webapp was not that serious and did not allow me to spot that the fix was not working for the browser case. Thanks again for your feedback. I am going to send another pull request with the changes I made, and will start a phantomjs setup.

[lukeapage]

We've decided the approach to take, we will re-write paths on the browser to be relative to the initial less file, not the current less file in order to match behaviour in lessc.

We will introduce a way of specifying a base path that will be applied to all paths, in the browser only in order to help development where the paths may not match production.

[siriux]

Hi @agatronic,

Could you explain a little bit the details of the rewriting? If we have this structure

/
  A.less
  image.png
  somedir/
    B.less
  otherdir/
    nested/
      C.less

where A has url("image.png") and B and C both @import "../A";

would that work with the new system? what would be the url in B.css and C.css?
if that doesn't work, should A contain a path relative to the including file? I think that would not be possible for more than one including file, as in the example.

This use case is really important to me.

Thanks

[lukeapage]

both A and B would write url("image.png")

@MatthewDL could you comment?

[loic]

@siriux with the setup, where do you plan to put your .css files (output of lessc)?

[siriux]

@loic I expect the css to be in the same place as the less. For example, for C, it would be at otherdir/nested/C.css.

[siriux]

Just to make it clear, I'll explain a little bit better my use case.

For me, @import is useful to modularize and reuse. If I want to reuse a less file in different places, the urls on this file have to be written relative to this file (as it's in css). For example, in A.less, I would do url("image.png").

Then, when it's used somewhere else, this url is rewritten. For me, it makes sense to either rewrite relative to the file, for B that would be url("../image.png") or even to a common base path if we are not keeping the css structure, for B url("image.png") that in this case is the same, it could be different.

I hope it's clearer now.

[loic]

@siriux it's an interesting case you've brought up because it would work with the newly fixed browser mode, but not with the current lessc. Fixing lessc would require url rewriting as well. It could however fall into a documented limitation which says that paths should be written relatively to the output .css file which is of course incompatible with "import[ing] a .css or .less file multiple times for different output .css files that could live in arbitrary locations" (as mentioned in #331)

That said, I truly believe the best solution would be to rewrite all URLs relatively to the .less file that defines them.

• It is consistent with how CSS handle relative paths in @import statements.
• It removes all ambiguity to the authors of the stylesheets.
• It allows inheriting the same file from multiple places.
• Since we always rewrite every URL it leaves less room for edge cases that we don't cover.

[siriux]

@loic I completely agree with you. That solution works well for simple and complex cases, and it behaves as expected if you come from CSS.

[kevinpschaaf]

@loic (and @sirux): I'm in agreement with loic's last proposed solution as well. Our use case (in the Enyo framework, which emphasizes modularity and reuse) is very similar to Sirux's: We provide libraries of UI components and we can't know where users will use them. Being able to import library LESS files (that reference assets via relative URLs) into arbitrarily-located app LESS files is important to allow app-level customization of library rules to modify the "theme" for those library components.

Here is a concrete example of how we intend to use LESS to allow customization of library CSS, to give you an idea:
https://github.com/enyojs/bootplate/blob/master/source/Theme.less

[matthew-dean]

The Explanation

Let me clear this up.

When talking with @cloudhead, he asked the straightforward question, "What's the simplest solution?" On the table were proposals for rewriting based on the root LESS file, or from the originating LESS file, similar to what's listed above.

The simplest solution? Don't rewrite URLs.

Once you introduce re-writing, there's a learning curve of the rules of rewriting. Not rewriting means there is no such learning curve.

So, if we do NO rewriting, here is the result:

• CSS consistency: Once you save your CSS file as LESS file, you do not risk the output suddenly and unexpectedly changing on the developer. The compiled CSS saved as LESS should be identical to the source CSS. That's how LESS is defined.
• No ambiguity: The least ambiguous solution for developers is that your code is not changed. Your LESS file has the same output no matter where it is, or where it's used. It's a one-to-one translation of LESS-to-CSS.
• Easier inheritance: You can move your LESS files, or import a library from a subdirectory, and nothing breaks.
• Edge cases: Fewer than in the case of introducing rewriting rules. As stated, you should have none. Your URLs are not touched.

In the browser, it's a slightly different scenario, as the compiled CSS is generated and placed on the page. To have the same result across testing and production, some rewriting is necessary. So, if I will eventually place my compiled CSS here in a css folder:

/
/css
   compiled.css
/images
/less

... and my images are in the images folder, then I need to tell the browser to treat \css as the root folder for my URLs when testing in-browser. The syntax will be something like a data-root property on the tag. Because the dev will typically not see the generated CSS, this should be transparent and make browser-to-server an easy transition.

Theme development

Before LESS, most CSS-based themes and UI libraries were deployed by an expectation that they were served alongside a CSS file. The images need to "travel" with the production CSS file, not the LESS file, because it's the browser that needs them. The LESS parser doesn't. The LESS parser doesn't know anything about your image, nor does it check to see if it even exists. It's the CSS it creates that really matters for pairing with images, that you deploy to your website with those images. That's what the browser is looking at, and so that's what your URLs would be relative to. So, this would be the better way to package your themes:

/
/css
   compiled.css
   /MyUITheme/
      image.png
/less

So, in your LESS library, you would write your URLs like this:

background-image: url('MyUITheme/image.png');

...and instruct users to place the theme folder alongside the deployed CSS file. Now, no matter where / how someone uses your LESS library, it's always a consistent output.

So, the TL;DR version: On the server, URLs will not be re-written. For the in-browser component, URLs will only be rewritten to simulate compiling to a single root CSS directory. Having LESS files in sub-directories, moving LESS files, or changing location of @imports will not and should not ever affect this. This will greatly simplify deploying associated images.

Hope that helps.

[Soviut]

I couldn't agree more. This also means fewer headaches for people who are generating their media urls programmatically or absolutely. That could be really confusing if LESS rewrote URLs that a web application or CMS was generating.

[siriux]

Hi @MatthewDL,

I see your point, and I think it's valid for simple use cases. But more complex situations, like @kevinpschaaf's, mine, or others, are also valid LESS use cases.

So, if you want to keep things simple, you can make no rewriting the default, but you can allow rewriting through configuration. Or even providing a simple hook on the parser where you can provide your own rewriting function. Or both.

LESS has a lot of potential in complex environments, but without rewriting, most non-trivial scenarios involving imports are ruled out. You cannot even post-process the css to rewrite the urls afterwards, as you don't have any information from the imports.

In short, I don't think rewriting has to be the default option for everyone, but having rewriting as a possibility makes LESS useful in many valid scenarios, where otherwise it coud not be used.

what do you think?

[matthew-dean]

That had occurred to me. Since rewriting needs to be within the parser, in order to do browser functionality, it could be exposed as an option. But I think it's a more complex problem than it looks, which is why there are so many URL bugs listed.

Take mixins for example. Say I import a LESS file in a subdirectory that has a mixin that contains a URL. A proposal is to rewrite that URL relative to its position. But if I call that mixin from a LESS file in a higher-level directory, what then? There may be a popular answer either way, but it won't be necessarily intuitive to everyone, and you have to document all those rules. So, that's the risk. Much more complex behavior.

Or you don't rewrite URLs. Theming still works. I'm not seeing how it's not workable, even if not the desired outcome for some. Whereas rewriting seems like a whole can of worms.

[SalimBensiali]

@MatthewDL you have hit a valid point here with mixins. I actually ran into this issue when trying to fix my browser implementation of the URL rewriting. My test app had URLs defined as default value of a mixin param, and the mixin would be called from another LESS file with another URL as its param. So I had to have a rewriting rule for the URLs that were declared in the mixin definition relatively to the LESS file that defines the mixin, and another rule for the URLs passed as parameters of a mixin call relatively to the LESS file that contains the mixin call. I am hoping to get this right on my fork, at least for the mental exercise, but I do understand both concerns. So maybe an option to enable rewriting the URLs is more suitable.

Cheers.

[siriux]

Hi @MatthewDL,

I understand it's a complex problem. But I don't see any problem of having a well thought, and documented, implementation as an option, that would solve many cases. For the really complex ones, a hook where you can provide your custom rewrite function would be enough.

The other option is to leave rewriting out. But people that needs this feature will start forking LESS or moving to something else. It seems to me much cleaner to provide an official way of dealing with this problem (optional rewriting + hook) than having people maintaining different solutions to the same problem in incompatible ways.

[lukeapage]

I have pulled @SalimBensiali pull request, which allow the setting of a root path option which is then inserted before every import. I have then added a relative-urls option which switches on or off the relative url's re-writing.

It will default to off, with no rootpath, meaning no url's will be transformed in node or the browser, but the user will be able to choose. Once I've finished the changes, this pull request will essentially be already implemented, so I am closing.