less / less.js

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

// comments inside .less files will stop applying the rulesets #3803

Closed Neohiro79 closed 11 months ago

Neohiro79 commented 1 year ago

To reproduce: Just make a rule inside a .less file and start "commenting" via "//" above or inside the rulesets. Those rulesets won't work. As soon as you "change" or encapsule those comments via the usual "/ /" comments, the rule gets applied as expected.

Current behavior: CSS rules won't be applied as expected - Chrome Dev Tools reports them as "unknown property name"

Expected behavior: As far as I know, "less" allowes "//" comments inside the ".less-Files". When using .less Parser, I expect the CSS rules to work as intended (at least that's what I am aware of).

Environment information:

Neohiro79 commented 1 year ago

To be more precise here, this is an example (.less File)

// disables the whole ruleset, just because it's // instead of /**/
h1 {
        font-size: 20px; /* works, when above // is changed into normal css-comments */
        /*
               // color: #FFFFFF;
        */
        background-color: #000000; /* rule does not work, once the above comment is // only */
}
Neohiro79 commented 1 year ago

Sorry, problem solved, since everything else less normally offers also didn't work either with that specific file I got suspicious.

If anybody has had the same problem, make sure you include the .less file as link rel="stylesheet/less" since the less-parser actually won't trigger a warning or error of a false less-type include.

I recently renamed a .css into a .less file and also included the less-parser properly, which went through without any errors, so I didn't catch the problem firsthand.

So, here is a feature request: Since it's really hard to get even find a problem firsthand or get to the problem in this case, is there a way the less-parser can detect a wrongly embedded less-filetype (ie. get link rel content via JS and compare if "stylesheet/less" is correctly placed and triggers a fat warning, since sometimes one just forgets to change this type when including more than one .less file in a document?

Cyddharth-Gupta commented 12 months ago

@Neohiro79 i guess the problem is resolved please close the issue.

Neohiro79 commented 12 months ago

So, there is no thought or comment on my follow-up feature-request to make the less-parser detect a potentially wrongly embedded less-filetype through ie.

var styleSheetList = document.styleSheets;

then comparing the suffix of the filename

if (".less" file && if "text/less") { fine } else { trigger additional warning into console } ?

Cyddharth-Gupta commented 12 months ago

@Neohiro79 my apologies, missed a couple of lines, thought the issue to be resolved

Neohiro79 commented 12 months ago

@Cyddharth-Gupta No problem - I just thought that it would make sense after I discovered that a google search completely mislead me to find/locate this error which took me some hours. It get's "parsed" without an error, in case you include a .less file but with text/css inside the rel attribute.

Also, additional to my proposal there should be also the opposite case covered (ie. ".css" is there but "text/less" is the rel/type value):

if (".css" file-suffix && "text/less") { fine } else { trigger additional warning into console }

and also in case ".less" is there but only "text/css" is written in the rel/type-attribute:

if (".less" file-suffix && "text/css") { trigger additional warning into console } else { fine }

That would cover the basic pitfalls which can always happen when including more than one file (or even in case you just don't know or forgot)

matthew-dean commented 11 months ago

In web requests, a file extension cannot actually tell you what type of file it is, because you can easily set up something like a .php file that provides the appropriate content and headers for a particular file type. So you need a way to signal, in your markup, what type you expect. Adding a warning in those cases would just possibly add a bunch of noise to the console.