[WIP] Updated wikimedia/less.php from v3 to v4

[hostep]

Description (*)

Will update description later

TODO's:

• describe PR
• benchmark speed differences (with PHP 8.1 is around ~1 second faster, with PHP 8.2 is ~0.3 seconds faster, sidenote: the difference between 8.1 & 8.2 is insane, never expected this)
• show diff of compiled css and try to explain it (still figuring part of this out)
• complain about Magento's silly coding standards (again)
• B2B Edition contains more differences than Magento Open Source - need to double check:

# both in Magento/blank & Magento/luma frontend themes:

 .negotiable-quote-quote-print .quote-block-content {
-  width: calc(44%);
+  width: calc(100% * 0.5 - 6rem);
 }

# both in Magento/backend & Magento/spectrum backend themes:

 .accordion .config input[name*='expiration_period'] + label {
   position: static;
   top: 5.5rem;
-  width: calc(96%);
+  width: calc(100% - 4rem);
 }
 .quotes-quote-print .quote-negotiated-price {
   float: right;
-  width: calc(44%);
+  width: calc((100%) * 0.5 - 6rem);
 }
 .quotes-quote-print .quote-catalog-price {
   float: left;
-  width: calc(44%);
+  width: calc((100%) * 0.5 - 6rem);
 }

• PageBuilder also has one change - need to double check:

# in Magento/backend backend theme

 .focus-options .pagebuilder-options .pagebuilder-options-wrapper:after {
   background: #ffffff;
   border: 1px solid rgba(153, 153, 153, 0.7);
   bottom: -6px;
   content: '';
   display: block;
   height: 12px;
-  left: calc(49%);
+  left: calc(50% - 1px);
   position: absolute;
   transform: rotate(45deg) translateX(-50%);
   width: 12px;
   z-index: 1;
 }

Update: This looks like a fix, because 50% - 1px is not equal to 49%, so it seems like this is a bug that gets fixed

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes magento/magento2#37841

Manual testing scenarios (*)

1. TODO !!!

Questions or comments

I've chosen to keep the constraints open so that version 3 is also still available to install, the changes in the calc compilation might be problematic for some shops, so this way we give an opportunity to let people stay at version 3. However, the 2 fixes in core magento themes will then cause issues for them...

In case of any protest about potential backwards compatibility breakage: we'll have to move to v4 anyways one day in the future (v3 will probably get abandoned and probably won't support newer PHP versions).

Contribution checklist (*)

• [x] Pull request has a meaningful description of its purpose
• [x] All commits are accompanied by meaningful commit messages
• [ ] All new or changed code is covered with unit/integration tests (if applicable)
• [ ] README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• [ ] All automated tests passed successfully (all builds are green)

[m2-assistant[bot]]

Hi @hostep. Thank you for your contribution! Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

:exclamation: Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s) For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here :information_source: Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation. Join Magento Community Engineering Slack and ask your questions in #github channel.

[hostep]

Just for funsies:

@magento run all tests

[magento-automated-testing[bot]]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

[mrtuvn]

Safer way is fix as you mentioned Second way is

width: ~"calc(100% - (@{checkout-tooltip-icon__font-size} + @{indent__s} + @{indent__xs}))";

=> this will output consistent results no matter how do you used node/grunt or php compilation always output width: calc(100% - (express));

I think it's clean fix problem without broken anything. Upgrade to v4 is worthy (of course php8 compat) but i'm not sure it's may come with breaking changes In case update package you may need to update in this file too https://github.com/magento/magento2/blob/2.4-develop/lib/internal/Magento/Framework/composer.json#L54 for consistent

[mrtuvn]

also i have already created PR for exactly your case here https://github.com/magento/magento2/pull/33643/files

[hostep]

Sorry, I was away on holiday. Also there are some changes in the wikimedia/less.php repository that haven't been published yet in a new release that I want to test first, because there are too much changes in the resulting compiled css which might be fixed by those unreleased changes. But I need to check. I'll probably need 1 or 2 more weeks before I find some time to continue on this.

Thanks for pointing out https://github.com/magento/magento2/pull/33643, I'll take a look at it as well in a couple of weeks.

[hostep]

Just tried again but with dev-master 06ab7ce of wikimedia/less.php as a test, and I still get maybe (?) unexpected changes. See small extract of diff between v3.2.0 and dev-master 06ab7ce of the library:

...
diff -urN /tmp/magento-scd-comparison/adminhtml/Magento/backend/en_US/css/styles.css pub/static/adminhtml/Magento/backend/en_US/css/styles.css
--- /tmp/magento-scd-comparison/adminhtml/Magento/backend/en_US/css/styles.css  2023-08-31 17:48:28
+++ pub/static/adminhtml/Magento/backend/en_US/css/styles.css   2023-08-31 17:50:55
@@ -3051,7 +3051,11 @@
 .action-select-wrap .abs-action-menu .action-submenu,
 .action-select-wrap .action-menu .action-submenu,
 .action-select-wrap .actions-split .dropdown-menu .action-submenu,
-.action-select-wrap .actions-split .action-menu .action-submenu {
+.action-select-wrap .actions-split .action-menu .action-submenu,
+.action-select-wrap .abs-action-menu .action-submenu .action-submenu,
+.action-select-wrap .actions-split .dropdown-menu .action-submenu .action-submenu,
+.action-select-wrap .actions-split .action-menu .action-submenu .action-submenu,
+.action-select-wrap .actions-split .action-menu .action-submenu .action-submenu {
   max-height: 45rem;
   overflow-y: auto;
 }
@@ -3059,14 +3063,22 @@
 .action-select-wrap .abs-action-menu .action-submenu ._disabled:hover,
 .action-select-wrap .action-menu .action-submenu ._disabled:hover,
 .action-select-wrap .actions-split .dropdown-menu .action-submenu ._disabled:hover,
-.action-select-wrap .actions-split .action-menu .action-submenu ._disabled:hover {
+.action-select-wrap .actions-split .action-menu .action-submenu ._disabled:hover,
+.action-select-wrap .abs-action-menu .action-submenu .action-submenu ._disabled:hover,
+.action-select-wrap .actions-split .dropdown-menu .action-submenu .action-submenu ._disabled:hover,
+.action-select-wrap .actions-split .action-menu .action-submenu .action-submenu ._disabled:hover,
+.action-select-wrap .actions-split .action-menu .action-submenu .action-submenu ._disabled:hover {
   background: #ffffff;
 }
 .action-select-wrap .action-menu ._disabled .action-menu-item,
 .action-select-wrap .abs-action-menu .action-submenu ._disabled .action-menu-item,
 .action-select-wrap .action-menu .action-submenu ._disabled .action-menu-item,
 .action-select-wrap .actions-split .dropdown-menu .action-submenu ._disabled .action-menu-item,
-.action-select-wrap .actions-split .action-menu .action-submenu ._disabled .action-menu-item {
+.action-select-wrap .actions-split .action-menu .action-submenu ._disabled .action-menu-item,
+.action-select-wrap .abs-action-menu .action-submenu .action-submenu ._disabled .action-menu-item,
+.action-select-wrap .actions-split .dropdown-menu .action-submenu .action-submenu ._disabled .action-menu-item,
+.action-select-wrap .actions-split .action-menu .action-submenu .action-submenu ._disabled .action-menu-item,
+.action-select-wrap .actions-split .action-menu .action-submenu .action-submenu ._disabled .action-menu-item {
   cursor: default;
   opacity: .5;
 }
...

Need to figure out if this is expected or not, as it makes the css files a bit bigger in filesize which is maybe not something we want.

No idea when I'll find time to continue with this investigation, I'm a bit busy in my afterwork life at the moment.

[ihor-sviziev]

Hi @hostep, I think we should close this PR in vafor to https://github.com/magento/magento2/pull/38335. Could you please confirm that?

[hostep]

No, not at all, these are 2 completely different things.

#38335 deals with compilation using less.js which is used for local development using grunt

This PR deals with compilation using less.php which is used by the setup:static-content:deploy command.

I'll try to find some more time to try to move this forwards.

[hostep]

Still WIP, this whole less compilation is a giant mess (no wonder that the guy who worked for Adobe some years ago to try to fix the frontend stack quit his job).

More observations:

• the changes found in https://github.com/magento/magento2/pull/37842#issuecomment-1701308378 match with the less.js compilation, so those are good changes I think (but haven't verified what they do exactly), see minimal test case at the bottom of this comment
• there's a lot of activity the last few weeks in https://github.com/wikimedia/less.php/commits/master/ in where the less.php library's compilation is starting to become closer and closer to the original less.js compilation, none of this is released in a stable version yet, so I suggest we wait a few weeks longer
• for example https://github.com/wikimedia/less.php/commit/79ae252b5cc17956069c1b1ba99f9fe37813ed71 significantly reduces the filesize of compiled css, as it compresses colors in a smaller form (just like less.js compilation does)
• my note in the description at the top around the page-builder module has been updated, it seems like this is a good change that doesn't require intervention. Also the changes in the B2B modules look to be similar to that, but I don't have the source code of B2B at hand, so not 100% sure.
• there were more things I discovered this evening, but it's hard to remember them all since it's such a complex thing this whole less compilation and I'm starting to doubt myself if I'm actually checking for the right things or not
• ...

Minimal test case for the first bullet point, compile this with less.php v3 vs less.php v4 vs less.js and observe the differences or similarities:

.admin__scope-old {
    //
    //    Data table
    //--------------------------------------
    .data-table {
        thead,
        tfoot,
        th {
            background: red;
        }

        td,
        tbody tr th,
        tbody tr td {
            background: blue;
        }

        tfoot  {
            tr {
                &:last-child th,
                &:last-child td {
                    border: 0;
                }
            }
        }
    }

    .accordion .config .data-table {
        thead th,
        tfoot td {
            &:extend(.data-table thead all);
        }

        td {
            &:extend(.data-table td all);
        }

        tbody tr:nth-child(odd) td {
            &:extend(.data-table tbody tr:nth-child(odd) td all);
        }

        tfoot tr:last-child td {
            &:extend(tfoot tr:last-child td all);
        }
    }
}

If somebody else wants to take a stab at this, feel free to do so, you are free in taking over the changes I send here.

[hostep]

Tried it out with the recently released v4.2.0 of wikimedia/less.php today, but ran into a bug: https://github.com/wikimedia/less.php/issues/107

And while working on this, I ran into another bug in Magento as well:

Error happened during deploy process: Magento\Framework\Exception\LocalizedException::__construct(): Argument #1 ($phrase) must be of type Magento\Framework\Phrase, string given, called in app/code/Magento/Deploy/Service/DeployPackage.php on line 142

This is because the $errorMessage (here) first gets assigned a Phrase but due to the concatenation with a bunch of strings, ends up as a string. And LocalizedException expects a Phrase, so this gives another error.

I'm not exactly sure yet how to deal with this other bug:

• Replace LocalizedException with \Exception?
• Additionally remove the translation for Compilation from source: ?
• Change __('Compilation from source: ') to __('Compilation from source: %1') and put all the concatenated strings as param of the __ method?
• But if we keep translating Compilation from source, I'm wondering if this code won't trigger in case it's a translated message since it references a non-translatable constant...

Anyway, still a lot of work to be done here.

[hostep]

The problem discovered in my last comment is being fixed over here: https://github.com/magento/magento2/pull/38683

[Krinkle]

@hostep I'd love to see Magento on Less.php 4.x, which fully passes all upstream Less.js 2.5.3 specs.

We've also released Less.php 5.0 just now which starts the journey toward Less.js 3.13 compliance. The nominal differences are front-loaded in the 5.0 release, such as differences around calc(), and switching from math=always (Less.js 2.x default) to math=parens-division (Less.js 4.x default), which fixes various bugs with using slashes in newer CSS features.

https://github.com/wikimedia/less.php/blob/master/CHANGES.md https://phabricator.wikimedia.org/T366445 https://lesscss.org/usage/#less-options-math

[hostep]

Yes @Krinkle, I noticed you guys put in quite some effort the last few weeks/months. Great job!

I'll see if I can pick this up in the near future. And try to upgrade to less.php 5.0 and see how that goes.

[hostep]

Upgraded less.php compiler further to version 5.0.0

Haven't performed a deep analysis yet, because the changes in resulting css files are very big when comparing v3 to v5, but most seem to make sense. (Update: manually reverting the code changes from https://github.com/wikimedia/less.php/commit/79ae252b5cc17956069c1b1ba99f9fe37813ed71 makes the diff way smaller and easier to read)

Still WIP until I (or somebody else) finds a bunch of hours to dive deep into this.

@magento run all tests