less / less.js

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

Less doesn't compile grid-column forward slash shorthand correctly, outputting incorrect CSS #3678

Closed rowanlovejoy closed 2 years ago

rowanlovejoy commented 2 years ago

To reproduce:

// In a .less file, add a rule containing the below property
.some-class {
    grid-column: 1 / -1
}

Current behavior:

The grid-column CSS property (part of the CSS Grid) supports a shorthand form using a forward slash, as shown in the example above, which sets the grid lines a column starts (before the slash) and ends on (after the slash). (See https://developer.mozilla.org/en-US/docs/Web/CSS/grid-column) Less currently doesn't process this shorthand correctly; Less compiles the example above to grid-column: -1, which sets column to start from line -1.

Expected behavior:

The rule grid-column: 1 / -1 should be not altered during compilation; it is standard CSS. In the example above, the property in the output CSS should appear exactly as it does in the Less source file.

Environment information:

A workaround I have been using for this problem is to escape the property value, so instead of grid-column: 1 / -1, I will write grid-column: ~'1 / -1'.

My theory is that this issue occurs because Less is trying to do a division operation using the numbers of either side of the slash.

iChenLei commented 2 years ago

2021-12-26 10 37 29

Please read the less v4 changelog -> https://github.com/less/less.js/releases/tag/v4.0.0

Why less online playground show incorrect output ? Because playground not using latest less. @matthew-dean Sir, How to update less for less-preview?

rowanlovejoy commented 2 years ago

@iChenLei Thanks for your response.

In that changelog, I found a link to this docs page: https://lesscss.org/usage/#less-options-math. I think it's saying that in Less 4.0 or later, executing x / y as a division operation by default requires surrounding it with parentheses, i.e., (x / y). However, I'm experiencing the incorrect behaviour in version 4.1.2 without parentheses.

parens-division (4.0 default) - No division is performed outside of parens using / operator (but can be "forced" outside of parens with ./ operator - ./ is deprecated)

I haven't changed any Less settings, so believe my version should exhibit the behaviour described in the above quote. Have I understood this correctly?

iChenLei commented 2 years ago

@rowanlovejoy Can you provide a minimal reproducation repo for your issue ? I test it but can't reproduce (see sceenshot in above comment)

rowanlovejoy commented 2 years ago

@iChenLei Here's a minimal reproduction repo (or at least as minimal as I know how to do): https://github.com/rowanlovejoy/less_test/tree/main

Inside styles.less, the currently broken code is uncommented; the working code is commented out above it.

iChenLei commented 2 years ago

https://github.com/rowanlovejoy/less_test/blob/652ff774b3c98351bdaaf538ae0ab749c05d5d0a/package-lock.json#L8145-L8150

  "snowpack-plugin-less": {
      "version": "1.0.7",
      "resolved": "https://registry.npmjs.org/snowpack-plugin-less/-/snowpack-plugin-less-1.0.7.tgz",
      "integrity": "sha512-cuFVPltfS9nf0EayN0uigoelbGrhZ6DlEAPqnIT4sv2v0mRE+f+06eHfk4J/9wKz/5E2bYsLFXED9hjnf11b4Q==",
      "dev": true,
      "requires": {
          "less": "^3.12.2",
          "tslib": "^2.1.0"
      },
      "dependencies": {

snowpack-plugin-less using less v3 to compile less file, so cause unexpected css output. @rowanlovejoy

BTW: please using vite rather than snowpack, vite is better and actively maintained.

rowanlovejoy commented 2 years ago

@iChenLei Oh, I see. Thank you for looking into this. It didn't ever occur to me that the loader would not use the latest version of Less, or that it would even depend on a specific version at all; I thought it would use whatever version of Less was installed.

Interestingly, I was having this issue at work as well, and there we use Webpack. (This was another reason I didn't think the bundler would be causing the problem.) I thought I had the latest less and less-loader packages there, too. However, I just tried it again on my machine, and now it's working.

Thanks for suggesting Vite. I thought Snowpack was new and still being worked on, but checking the repo, I can see it's been months since the last release or even file change.