Skip to content

Difference in less compilation between php & nodejs library (grunt) with complicated calc expressions #37841

Closed
@hostep

Description

@hostep

Preconditions and environment

  • Magento version 2.4-develop
  • PHP 8.1 or 8.2
  • Nodejs 16

Other things to prepare here:

  1. Setup vanilla Magento install
  2. Run the following:
$ cp package.json.sample package.json
$ cp Gruntfile.js.sample Gruntfile.js
$ npm install
$ bin/magento deploy:mode:set developer  # so that when compiling less via php, it's not being minified, so that comparing files is easier
  1. In the backoffice, go to Stores >Attributes > Product, and for both news_from_date and news_to_date perform the following change:
    • Storefront Properties > Use in Search > Yes
    • Storefront Properties > Visible in Advanced Search > Yes

Steps to reproduce

  1. First run the less compilation through the less library in php:
$ rm -R var/view_preprocessed/* pub/static/*
$ bin/magento setup:upgrade # needed to be able to visit frontend without js errors
$ bin/magento setup:static-content:deploy -f en_US
  1. Now copy the resulting files in some other directory:
$ cp -r pub/static/ /tmp/magento-scd-comparison
  1. In one tab in your browser, visit the frontend of the shop and click on Advanced Search in the footer and keep this open
  2. Now run the less compilation through grunt (so via nodejs instead of php):
$ rm -R var/view_preprocessed/* pub/static/*
$ bin/magento setup:upgrade # needed to be able to visit frontend without js errors
$ grunt clean:luma && grunt exec:luma && grunt less:luma
  1. Now compare the resulting files with the ones you copied before, I'm focusing here on something very specific (not all changes are relevant or important here):
$ diff -u /tmp/magento-scd-comparison/frontend/Magento/luma/en_US/css/styles-m.css pub/static/frontend/Magento/luma/en_US/css/styles-m.css  | grep -C3 calc

Output:

-.form.search.advanced .fields.range .field.date input,
-.form-giftregistry-edit .field.date input {
-  margin-right: 10px;
-  width: calc(100% - 33px);
+.form-giftregistry-search .fields-specific-options .datetime-picker {
+  margin-right: 10px;width: calc(100% - 23px + 10px);
 }
 .field .control._with-tooltip {
   position: relative;
 }
 .field .control._with-tooltip input {
-  margin-right: 10px;
-  width: calc(100% - 36px);
+  margin-right: 10px;width: calc(100% - 21px + 10px + 5px);
 }
 .checkout-index-index .modal-popup .field-tooltip .field-tooltip-content,
 .shipping-policy-block.field-tooltip .field-tooltip-content {
  1. In a second tab in your browser, visit the frontend of the shop and click on Advanced Search in the footer and compare it to your first tab

Expected result

The width: calc expressions are the same between the PHP less library & nodejs less library:

less.js:
1. width: calc(100% - 23px + 10px);
2. width: calc(100% - 21px + 10px + 5px);

less.php:
1. width: calc(100% - 23px + 10px); or width: calc(100% - 13px);
2. width: calc(100% - 21px + 10px + 5px); or width: calc(100% - 6px);

Actual result

The width: calc expressions are different between the PHP less library & nodejs less library:

less.js:
1. width: calc(100% - 23px + 10px);   => simplified: width: calc(100% - 13px);
2. width: calc(100% - 21px + 10px + 5px); => simplified: width: calc(100% - 6px);

less.php:
1. width: calc(100% - 33px);
2. width: calc(100% - 36px);

How it looks with less.js:

Screenshot 2023-08-03 at 17 41 03

How it looks with less.php:

Screenshot 2023-08-03 at 17 38 36

Additional information

You may be surprised by the screenshots, but reading the original less code (which contains a bug), it should be rendered like the less.js result (the bug can be fixed by changing + @indent__s to - @indent__s which makes a lot more sense)

I've tried this again but with v4 of wikimedia/less.php (which is being used in Magento as php library to compile less, currently v3 is being installed). And v4 fixes this problem, I'm pretty much convinced this fixed it: wikimedia/less.php@5ca673d

Fix this by recognising calc() higher up as something that server-side LESS math should not be performed on in the first place.

So the solution for Magento is to switch from v3 to v4 of wikimedia/less.php. An additional benefit is that v4 is slightly faster than v3, about 1 second faster average when running bin/magento setup:static-content:deploy with all 3 default Magento themes.

I'm currently working on a PR that will upgrade wikimedia/less.php to v4, but it's more complicated, some less code in Magento is not written correctly (especially around those calc expressions) and we'll need to tweak it bit.
Expect a PR soon (maybe today, maybe later this week)

Release note

No response

Triage and priority

  • Severity: S0 - Affects critical data or functionality and leaves users without workaround.
  • Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
  • Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
  • Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
  • Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

Metadata

Metadata

Assignees

Labels

Area: FrameworkComponent: FrontendIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedPriority: P2A defect with this priority could have functionality issues which are not to expectations.Progress: doneReported on 2.4.xIndicates original Magento version for the Issue report.Reproduced on 2.4.xThe issue has been reproduced on latest 2.4-develop branchTriage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject it

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions