less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.03k stars 3.41k forks source link

Remove (or advise against) JavaScript interpolation #939

Closed leafo closed 10 years ago

leafo commented 12 years ago

I run the PHP version of LESS. I'm starting to see people asking for support for the JavaScript backtick syntax. Sadly my options are quite limited here. (Yes, I'm aware of http://php.net/manual/en/book.v8js.php)

I think a lot of the cases where people fall back to using the JavaScript inside of their LESS can be avoided with some utility function. I can start collecting snippets where people are using the backtick syntax.

For example, this snippet has crept its way into Bootstrap 2.1.1: http://www.toekneestuck.com/blog/2012/05/15/less-css-arguments-variable/

It's very cool what putting JavaScript into LESS gives us, but I think ideally LESS should not be dependent on any specific runtime.

lukeapage commented 12 years ago

I agree...

the solution discussed was to add a args which is comma seperated and keep arguments space seperated. I think we should do this asap and ask bootstrap to not use javascript - it should be a last resource imo.

cloudhead commented 12 years ago

The php version should probably run php inside of backticks..

For comma-delimitted, probably something like @arguments-list

lukeapage commented 12 years ago

adding arguments-list was easy, will push soon.. but I can't find any use of javascript in bootstrap.. was it a momentary thing?

leafo commented 12 years ago

Maybe we should have a function to convert the delimiter of a list type instead of having another magic variable name?

Also, here's where the JavaScript is in bootstrap: https://github.com/twitter/bootstrap/blob/2.1.1-wip/less/mixins.less#L253

matthew-dean commented 12 years ago

@cloudhead - I disagree that the language interpreted should change based on the compiler that's compiling LESS. That seems counter-intuitive, as LESS definitions should be identical regardless of parser / interpreter, especially if you're producing a LESS library. A Bootstrap with LESS/embedded-JS wouldn't also have a Bootstrap LESS/embedded-PHP version. It should all just be LESS.

I'm in favor of this request. IMHO JavaScript interpretation should be deprecated in LESS, because a) it unnecessarily couples the LESS language definition with the parser that interprets, b) not all LESS parsers are JS, c) not all JavaScript parsers can eval embedded JS for security reasons (like Adobe AIR), d) most importantly, it breaks the declarative nature of LESS.

Furthermore, we should immediately reach out to the Bootstrap people and demonstrate non-JavaScript methods to achieve the same goal, such as escaping the entire string sent to a .box-shadow function .box-shadow(~"shadow1, shadow2"), or using a plugin syntax, or adding a small feature to change how arguments are interpreted by mixins. But, to me, JavaScript interpretation is by far the least elegant of all those options. I'd rather see us make LESS robust enough that interspersing the syntax of other languages is really not necessary.

krnlde commented 12 years ago

@agatronic I totally agree with this. Bootstrap shouldn't rely on JS methods but instead make use and extend built-in LESS functions.

barryvdh commented 12 years ago

@matthewdl It originally used such a syntax, but that gave problems with using mixins inside that string. See the commit that changed it; https://github.com/twitter/bootstrap/pull/4763

Also agree that it shouldn't use javascript.

WilliamStam commented 12 years ago

agreed with no js inside less

lukeapage commented 12 years ago

lots of discussion over this particular problem in #35 and #941

I have also created a bootstrap bug:

https://github.com/twitter/bootstrap/issues/4997

lukeapage commented 12 years ago

linking to first issue on bootstrap

https://github.com/twitter/bootstrap/issues/4976

lukeapage commented 10 years ago

closing due to inactivity and I think we have done this - we do advise against and we are replacing features where people use js.

matthew-dean commented 10 years ago

Okay, although I would still recommend targeting a version where javascript support is off by default, and gets turned on via a switch. Have we done this yet?

lukeapage commented 10 years ago

There is a switch, but its on by default. 2.0 with a message ala we don't think you need js.. see plugins and new features etc. On 14 Feb 2014 01:33, "Matthew Dean" notifications@github.com wrote:

Okay, although I would still recommend targeting a version where javascript support is off by default, and gets turned on via a switch. Have we done this yet?

Reply to this email directly or view it on GitHubhttps://github.com/less/less.js/issues/939#issuecomment-35047339 .

matthew-dean commented 10 years ago

Right. When will we turn it off by default?

lukeapage commented 10 years ago

I missed a bit out of that last message. Insert "we could switch it off in" in front of 2.0. I'll make an issue to discuss default options like this

On 14 February 2014 16:09, Matthew Dean notifications@github.com wrote:

Right. When will we turn it off by default?

Reply to this email directly or view it on GitHubhttps://github.com/less/less.js/issues/939#issuecomment-35097263 .