Magento2: Different results from compiling less with Grunt or with "server side compilation"

[slackerzz]

Preconditions (*)

1. Magento 2.3.x (tried both CE and EE)
2. Ubuntu 16.04, PHP 7.x, node v4.6.1

Steps to reproduce (*)

1. set Magento in developer mode
2. in the backend set css generation in "server side compilation"

While developing a custom template i found some differences between my local dev site (inside a vagrant box) and our staging server. Some css rules in the staging server are not applied, while in my dev box they work as expected. In my local environment i use grunt while in our staging server we use server side compilation.
The produced css files are different, the order in which css rules are processed is different!

So today i made some tests with a clean M2 installations and Luma theme.

| Server Side compilation | Grunt | Diff |
| --- | --- | --- |
| styles-m.css | styles-m.css | diff* |
| styles-l.css | styles-l.css | diff* |

*on the left side of the diff you have the server side compiled css

Expected result (*)

No diff

Actual result (*)

As you can see styles-m.css is ok, there are no breaking differences between the two versions.
Sadly the two versions of styles-l.css are not equivalent, the order of the css rules in the two files is different and this can produce a different result.

[hostep]

If you are able to install and use nodejs on your production server, you can use our module, which uses less.js to compile less files on the server instead of the less.php library (which indeed produces slightly different results).

[slackerzz]

thank you @hostep, i'll try your module

[slackerzz]

@hostep your module works well
However this is a major flaw in the official frontend dev workflow

[tkacheva]

@slackerzz Thanks for reporting that. Internal issue MAGETWO-60689

[slackerzz]

any news here?

[orlangur]

This is expectable by itself since client-side and server-side preprocessing use different library versions.

Providing a reproducible narrowed use case (some visual bug) would simplify investigation.

[magento-engcom-team]

@slackerzz, thank you for your report.
We've created internal ticket(s) MAGETWO-60689 to track progress on the issue.

[joshdavenport]

Am I living in a bubble or is nobody else other than those already partaking in this issue experiencing this?

I have what seems to be a reproducable case, and am happy to pass on some details if anybody is open to working on this.

[orlangur]

Hi @joshdavenport, of course providing a simple test case will simplify the investigation to whoever will work on this.

Note that

This is expectable by itself since client-side and server-side preprocessing use different library versions.

Magento is not doing LESS compilation by itself, in case of bug most likely we will have to patch lessphp library.

[joshdavenport]

Before proceeding here, I've actually discovered that the different results for me between LESS complied via grunt and "server side compilation" was eliminated by downgrading grunt-contrib-less from 1.4.1 (which is using less 2.7.1) to 0.12.0 (which is using less 1.7.2). I think @hostep may have discovered this (based on the comments in their module referenced earlier). grunt-contrib-less is specified as 1.4.1 in the provided package.json.sample so it's fair to assume others using the grunt workflow have a real possibility of stumbling into this issue.

Is it worth me reporting this separately?

[hostep]

@joshdavenport : please do so. This happend when upgrading from Magento 2.1 to 2.2, the nodejs dependencies got updated and now less 2.x is getting installed even though the PHP library to compile less is only compatible with less 1.x

I've reported this through some other channel a long time ago, can't remember when or how, but I failed at reporting it here on github. So please, go ahead and open a new issue.

[slackerzz]

I opened this issue while i was working with Magento 2.1.2 so it was present before any upgrade to 2.2.
The problem, i think, is the css splitting between style-m.css and style-l.css.

In the themes i develop i do not use the .media_width() mixin and set them to use and generate a single css file, that's my workaround.

[mverstraete]

I may have tracked this down to an issue in grunt-contrib-less v1.4.1, where it is not compiling nested extends.

I had this problem specifically with the Add To Wish List button. In the Blank theme, the wishlist button gets its button styles via:
&:extend(.abs-action-addto-product all);

.abs-action-addto-product is defined in _extends.less, and it contains another extend:
&:extend(.abs-action-link-button all);

.abs-action-link-button, which contains the button styles, is never added to the wishlist button when compiling with grunt-contrib-less v1.4.1.

it _does_ work if I downgrade to grunt-contrib-less v0.12.0, and it also _does_ work when LESS is compiled with PHP.

So, it seems there is an issue with grunt-contrib-less v1.4.1 and compiling extends within extends.

[orlangur]

different results for me between LESS complied via grunt and "server side compilation" was eliminated by downgrading grunt-contrib-less from 1.4.1 (which is using less 2.7.1) to 0.12.0 (which is using less 1.7.2)

when upgrading from Magento 2.1 to 2.2, the nodejs dependencies got updated and now less 2.x is getting installed even though the PHP library to compile less is only compatible with less 1.x

@joshdavenport @hostep @mveeramneni can somebody contribute such downgrade of JS library maybe?

[hostep]

even though the PHP library to compile less is only compatible with less 1.x

I'm no longer convinced of this statement I made earlier. If we have to believe the less version specified in the oyejorge/less.php library, it is marked as compatible with less 2.5.3 in the latest version: https://github.com/oyejorge/less.php/blob/v1.7.0.14/lib/Less/Version.php#L12

This was increased from less version 1.7.5 to 2.5.3 in version v1.7.0.12 of oyejorge/less.php by this commit: https://github.com/oyejorge/less.php/commit/712473a175039a121a535e621b24d03dc8143cf0#diff-c2bd6b044fb355e9bc8f661b46eff114

v1.7.0.12 was released on 6 February 2017 which was in-between Magento 2.2.2 and 2.2.3, so Magento 2.2.0 initially shipped with a nodejs package compatible with less v2 which was probably wrong at that time, but in the mean time it is probably fine by upgrading to the latest releases of the oyejorge/less.php package.

I have to little deep knowledge about the differences between v1 and v2 of the less spec to verify if anything of this is true or not. If anybody with more knowledge about this could weigh in, that would be appreciated :)

@mverstraete: what version of the oyejorge/less.php package are you using?

[Brengineer]

I've noticed a similar issue, with the primary difference being that .abs- styles which extend other .abs- styles in _extends.less are not output properly when compiled with Grunt, due to being nested (e.g. .abs-action-addto-product, which extends .abs-action-link-button). This creates differences between production and development sites, which make some style issues invisible in a standard development workflow.

[ghost]

It is not an issue the difference between rendered css is:

• grunt uses shorthand colors where possible instead of full-length hexa-colors. There is css-comb library in npm that can solve this, because grunt-contrib-less doesn't provide such options, look here https://github.com/csscomb/csscomb.js/blob/dev/doc/configuration.md

• grunt doesn't prettifies the output so we rules order is defined by natural files precedence, while server-side library is set to put tag rules before class rules. Solution for this also should exist.

The difference in the example doesn't make any impact on page rendering.

[joshdavenport]

That statement seems to directly contradict what everybody has found in this issue, especially https://github.com/magento/magento2/issues/7231#issuecomment-431164523

I personally originally brought this up when I saw legitimately different appearances (due to missing styles) in the cart

[mverstraete]

@joshdavenport is correct. The issue is 100% the fact that grunt-contrib-less v1.4.1 does not compile nested extends, while the php less compiler does.

[slackerzz]

Looking at the diff that i linked 2 years ago you can see that both php and grunt compile the same set of rules, but the order is different.

In the diff the css in the left comes from php, the right one from grunt.
Take this random css rule, that comes from https://github.com/magento/magento2/blob/2.3-develop/app/design/frontend/Magento/luma/web/css/source/_extends.less#L478:

.abs-margin-for-forms-desktop {
     margin-left: 25.8%;
}

The php compiler put this rule at line 101 while grunt at 561.

Another example that comes from from https://github.com/magento/magento2/blob/2.3-develop/app/design/frontend/Magento/luma/web/css/source/_extends.less#L159:

.abs-button-desktop {       
    width: auto;        
}

The php compiler put this rule at line 27 while grunt at 487.

[mverstraete]

@slackerzz , indeed, the solution to your original problem is simply that the order of css rules is different between PHP and Grunt.

There is a second issue being talked about here, where after Magento 2.2, Grunt does not compile some styles at all due to nested extends. Perhaps we should open a separate issue? I'll try to put together a simple example and open a new issue. Not sure if I'll have time soon, so if somebody else beats me to it, more power to you.

[giacmir]

@engcom-backlog-andrii Even if the example reported do not have "impact on page rendering" this does not mean that there is no issue here.

Having two compile systems that produce different ordered rules is a BIG problem in css, where rule position is important (let's remember that the first C of CSS is for "cascading").

In my opinion this issue should be reopened and treated as bug.

[slackerzz]

@mverstraete the fact the order of css rules is different between PHP and Grunt is the issue i pointed out, could the issue itself be the solution too?

[DanielRuf]

The difference is because of different compilers and different behaviors in the implementations that we use.

Can anybody reproduce this with a simple reproducable setup without Magento but the same dependencies (+ versions)?

[DanielRuf]

oyejorge/less.php and grunt-contrib-less

[DanielRuf]

https://github.com/oyejorge/less.php/issues/340

https://github.com/oyejorge/less.php/issues

I propose to switch to another library (for the PHP stack). We shall not fix a thirdparty dependency which seems to be incomplete.

[DanielRuf]

https://packagist.org/packages/leafo/lessphp
https://github.com/leafo/lessphp
http://leafo.net/lessphp/

[DanielRuf]

But it's probably easier to use another pure and single solution instead of having two frontend stacks.

[DanielRuf]

https://github.com/gruntjs/grunt-contrib-less/compare/v1.4.1...master

The issue is that no one uses the Grunt version and we just use the PHP version in projects.

Would an upgrade to [email protected] fix the issue with the order?

[jasperzeinstra]

We're still having this issue on 2.3. The version of grunt-contrib-less for 2.3 is set on ^2.0.0.

Here are two screenshots that show the impact of this issue:

When downgrading grunt-contrib-less to version 0.12.0 like @joshdavenport is suggesting the problem is resolved. But it feels very bad to downgrade from ^2.0.0 to 0.12.0.

This issue has been reported on 28 Oct 2016 and there's still no real solution for this problem. We should be able to do our frontend developing with grunt. Waiting on PHP on every change is taking up way to much time.

@magento-engcom-team could you reply on how you're looking at this issue and what the state is of MAGETWO-60689 created on 8 november 2016?

[DanielRuf]

We can not easily fix this.

Please read all previous commens and also https://github.com/magento/magento2/issues/7231#issuecomment-449398387

[DanielRuf]

We shall not fix a thirdparty dependency which seems to be incomplete.

[jasperzeinstra]

From my point of view comment #7231 (comment) is talking about the order of CSS. The issue with nested &:extend(…) isn't discussed their. Only @joshdavenport mentioned to lower grunt-contrib-less. That solves the issue, but i'm not sure if we're missing any other functionality.

It kind of weird that Magento says use grunt for local frontend development. But when using grunt we're missing some the nested &:extend(…) in &:extend(…) and we can trust the CSS we're seeing.

[DrewML]

Even if we can't fix this immediately, this is a real bug and we should be leaving this ticket open to reflect that.

We as Magento currently offer 2 different ways to compile your LESS:

1. PHP
2. Grunt

Even if these are separate compilers, we should either:

1. Be shipping versions that are compatible with each-other
2. Only ship 1 compiler (either PHP or LessJS/Grunt)

Appreciate the feedback from everyone so far.

[orlangur]

@DrewML,

1. Teach developers that result of compilation may be different by nature but strive for elimination of any incompatibilities

Wasn't the plan to switch to SASS by the way?

[DrewML]

Wasn't the plan to switch to SASS by the way?

Hmm, that's news to me. I know people want it, but I haven't seen a concrete proposal for it.

[orlangur]

@DrewML there were such discussions much earlier than architecture repo was even introduced, here is a community project somebody from Magento should be aware of: https://community.magento.com/t5/Magento-2-x-Less-to-Sass/bd-p/less-to-sass

[DrewML]

Ah right - forgot this whole arch process was so new.

To be fairly transparent, I can't imagine us being able to make this change in core in the current platform. Because of back-compat requirements, we'd have to support less _and_ sass for a long period of time, which is difficult since styles are notoriously hard to statically verify.

I will say it's _possible_ we could support SASS running in static content deployment at some future time, but don't think we'll re-write existing themes or publish a new sass theme.

[DanielRuf]

which is difficult since styles are notoriously hard to statically verify

Probably not that hard as we can build ASTs and parse in many different ways.

[orlangur]

I may recall it wrong, but if we have a separate theme for PWA the plan was to make it SASS-based.

Why we cannot deprecate LESS core styles in 2.4 and remove in 2.5? LESS compilation then may still be in-place for a longer period.

[DrewML]

Probably not that hard as we can build ASTs and parse in many different ways.

This isn't about validating syntax. The problem would be, any dev doing styling work in core would need to update the luma sass theme and the luma less theme, and then we'd need to always watch for visual regressions across all pages between the two.

I may recall it wrong, but if we have a separate theme for PWA the plan was to make it SASS-based.

PWA isn't a theme but an entirely new rending and compilation pipeline for the front-end outside of core. It doesn't participate in Magento's CSS compilation at all.

Why we cannot deprecate LESS core styles in 2.4 and remove in 2.5? LESS compilation then may still be in-place for a longer period.

I'm not sure I see the value in deprecating it. The only way we'd deprecate in core is if we had something new to replace less with, but that would require us maintaining less theme + {insert styling language here} theme for years.

[orlangur]

I'm not sure I see the value in deprecating it

It depends just on opinion "SASS is better than LESS". I have no opinion on this as I don't do HTML/CSS coding.

but that would require us maintaining less theme + {insert styling language here} theme for years

Why? Just release a SASS theme in 2.4 deprecating LESS theme at the same time. All further fixes are made to SASS only.

[DrewML]

Just release a SASS theme in 2.4 deprecating LESS theme at the same time. All further fixes are made to SASS only.

We would need an _extremely_ good argument for stopping support for a theme almost every m2 store is based upon.

[MichaelThessel]

The fact that this is still a problem in 2019 shows that handling dev and production static asset compilation in 2 different ways has been one of the worst design decisions in M2. Unifying this should be a focus of one of the next releases. Working with M2 in it's current state is a massive pain.

[orlangur]

@MichaelThessel I would not call it "design decision" as the problem lies in third-party components. M2 tried to cover two main use cases: easily generate assets when you don't even touch frontend VS give you an ability to use original JS library. And did it's job pretty well - just that you need to understand natural limitations of both approaches.

The problem is even deeper and lies in LESS itself which, unlike SASS, does not even have a specification.

I don't think if less.php will be dropped backend developers will become really happy having to setup additional tooling. What would you propose as a "unifying"?

My understanding is that "server side compilation" based on lessphp is not production ready in it's current state (and should not be - corresponding library is kinda abandoned) thus it should only be available for development purposes with a big WARNING to use lessjs when you work with frontend intensively.

[hostep]

My understanding is that "server side compilation" based on lessphp is not production ready in it's current state (and should not be - corresponding library is kinda abandoned)

Actually the less.php library was taken over a while ago by wikimedia: https://github.com/wikimedia/less.php/. Recently support for PHP 7.3 & 7.4 was added for example. So it's still kind of maintained. If there are issues with the library, PR's to it will most likely get reviewed and accepted.

Magento 2.3.3 will ship with this new fork btw: https://github.com/magento/magento2/commit/9a4332cb18945772c1458d53cd1a9c191c3f7a10

[MichaelThessel]

@orlangur Since the introduction of CSS pre-processors Magento is the only project that I have come across that has 2 different ways to compile CSS. Any other project uses the same compilation approach for production and development. The regular way is to just compile it the same way you do on your dev system during CI and then bundle the compiled CSS with the rest of the build and deploy it to production. The fact that I have to shut down my site for 2 minutes to compile my static assets would be considered insane in any other project. Just look at this thread and you will see that its not working for tons of people.

[orlangur]

@hostep thanks for sharing! https://www.google.com/search?q=lessphp only showed me oyejorge and leafo, both unmaintained.

@MichaelThessel, sure, I see. Materializing compiled files somewhere is really robust, could be used even for generated PHP code.

[DrewML]

Thanks for the good discussion, y'all.

Just to be incredibly clear from my side on behalf of Magento, I consider this to be a bug. Styles (without a code change) should not change between dev and prod.

We should be using 1 compiler, period. Whether it's the PHP or the JS version is slightly less important.

For the short-term, until we can address this at a code level, we need to make it super clear that you should be using the same compiler for both dev and prod.

Long term, the preference would be for us to move to the official compiler. The majority of front-end tools in 2019 are authored in JS, so attempting to keep things in PHP for the sake of consistency is a losing battle.

[m2-assistant[bot]]

Hi @engcom-Charlie. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

• [ ] 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).
  Details

  If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

• [ ] 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

• [ ] 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

• [ ] 4. Verify that the issue is reproducible on 2.3-develop branch

  Details

  - Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and _stop verification process here_!

• [ ] 5. Add label Issue: Confirmed once verification is complete.

• [ ] 6. Make sure that automatic system confirms that report has been added to the backlog.

[magento-engcom-team]

:white_check_mark: Confirmed by @engcom-Charlie
Thank you for verifying the issue. Based on the provided information internal tickets MC-22208 were created

Issue Available: @engcom-Charlie, _You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself._