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

Fix un-parenthesized nested operation via a variable (Less.js 4.1.1) #114

Closed
kdambekalns opened this issue Sep 27, 2024 · 5 comments
Closed
Labels
Type: Bug Something isn't working

Comments

@kdambekalns
Copy link

I see

Warning: Undefined property: Less_Tree_Operation::$value in /…/wikimedia/less.php/lib/Less/Tree/Dimension.php line 79

Fun fact: At this point $other is no the expected Less_Tree_Dimension but an instance of Less_Tree_Operation (type safety would be great).

I would love to help debugging this, but I can't even tell if this can be caused by faulty LESS input (it should not lead to such a warning, though) or is a programmatic error.

@Krinkle
Copy link
Member

Krinkle commented Sep 27, 2024

@kdambekalns

We'll need a reduced example input that reproduces the error to investigate.

I suggest comparing also to upstream Less.js at http://lesscss.org/less-preview/, to see how the same input behaves there.

@Krinkle Krinkle added the Type: Support Ask for help, and other questions label Sep 27, 2024
@peterpp
Copy link
Contributor

peterpp commented Nov 13, 2024

I've isolated the problem to this example:

p {
    @a: 20px / 2;
    margin: 2 * @a;
}

It does not work even with Less.js. I think it's because the expression 20px / 2 is not evaluated. A simple fix is to add the parenthesis:

p {
    @a: (20px / 2);
    margin: 2 * @a;
}

@Krinkle
Copy link
Member

Krinkle commented Nov 13, 2024

@peterpp Good catch. Note that, while it is not recommended, it is possible to set math=always. This means you will not be able to use the CSS slash operator in Less code, except when escaping it as ./:

div {
  border-radius: 40px / 10px;
  grid-column: 1 / -1;
  font: 12px/1.6 sans-serif;
  background: left 5% / 15% url(images/star.png);
}

If you're okay with requiring a ./ for all of these, and instead want to have slash for division without parenthesis, then the math=always option can be used. This is available in both Less.js and Less.php.

https://lesscss.org/usage/#less-options-math

By default, upstream Less.js indeed also crashes:

Screenshot

But, Less.js with math=always (Example in the Less.js playground):
Screenshot

This works without an error. The main reason that this feature exists, is for backwards-compatibility with Less.js 1.x and Less.js 2.x, where slash without parenthesis was the default. Back then, there were not many CSS features that used a slash. Once this became more common, upstream decided to change this, and hence nowadays parenthesis are required for division.

# less.php

$ cat input.less 
p {
    @a: 20px / 2;
    margin: 2 * @a;
}

$ bin/lessc --math=always input.less 
p {
  margin: 20px;
}

$ bin/lessc --math=parens input.less 
p {
  margin: 2 * 20px / 2;
}

$ bin/lessc --math=parens-divisions input.less 
PHP Warning:  Undefined property: Less_Tree_Operation::$value in /Users/krinkle/Development/less.php/lib/Less/Tree/Dimension.php on line 79

PHP Warning:  Undefined property: Less_Tree_Operation::$unit in /Users/krinkle/Development/less.php/lib/Less/Tree/Dimension.php on line 100

PHP Fatal error:  Uncaught TypeError: array_merge(): Argument #2 must be of type array, null given in /Users/krinkle/Development/less.php/lib/Less/Tree/Dimension.php:100

I still think there is a bug here, because this is not failing with a user-facing Less.php error about an invalid operation in Less syntax. Instead, it is failing the PHP level due to a mistake in our internal code. Even though it is invalid, we should be able to detect that it is invalid and say so explicitly instead of crashing in this way.

@Krinkle Krinkle added Type: Bug Something isn't working and removed Type: Support Ask for help, and other questions labels Nov 13, 2024
@umherirrender
Copy link
Contributor

Also present in less.js with 3.13.1
https://lesscss.org/less-preview/#eyJjb2RlIjoicCB7XG4gICAgQGE6IDIwcHggLyAyO1xuICAgIG1hcmdpbjogMiAqIEBhO1xufSIsImFjdGl2ZVZlcnNpb24iOiIzLjEzLjEiLCJtYXRoIjoicGFyZW5zLWRpdmlzaW9uIiwic3RyaWN0VW5pdHMiOnRydWV9
gives "i.unit is undefined"

Fixed in less.js with less/less.js#3589, that is 4.1.1

@Krinkle Krinkle changed the title Seeing ´Undefined property: Less_Tree_Operation::$value with v5.1.1 on PHP 8.3 Support un-parenthesized nested operation via a variable (Less.js 4.1.1) Jan 30, 2025
@Krinkle Krinkle changed the title Support un-parenthesized nested operation via a variable (Less.js 4.1.1) Fix un-parenthesized nested operation via a variable (Less.js 4.1.1) Jan 30, 2025
@Krinkle
Copy link
Member

Krinkle commented Feb 22, 2025

@Krinkle Krinkle closed this as completed Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Development

No branches or pull requests

4 participants