baldwin-agency / magento2-module-less-js-compiler

Allows Magento 2 to compile less files using the less nodejs compiler
MIT License
23 stars 4 forks source link

Less.js 4.2.0 and magento 2.4.6 without any less modifications (discussion) #9

Closed piotrkwiecinski closed 3 weeks ago

piotrkwiecinski commented 1 month ago

Hi @hostep, we are playing with this extensions and found a way to get less to compile without any changes to core magento less.

Setting math to always restores a behaviour from 3.X based on https://lesscss.org/usage/#less-options-math

Here is a dirty hack we used.

Index: vendor/baldwin/magento2-module-less-js-compiler/Css/PreProcessor/Adapter/Less/Processor.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vendor/baldwin/magento2-module-less-js-compiler/Css/PreProcessor/Adapter/Less/Processor.php b/vendor/baldwin/magento2-module-less-js-compiler/Css/PreProcessor/Adapter/Less/Processor.php
--- a/vendor/baldwin/magento2-module-less-js-compiler/Css/PreProcessor/Adapter/Less/Processor.php
+++ b/vendor/baldwin/magento2-module-less-js-compiler/Css/PreProcessor/Adapter/Less/Processor.php   (date 1717423713624)
@@ -117,7 +117,7 @@
      */
     protected function getCompilerArgsAsString()
     {
-        $args = ['--no-color']; // for example: --no-ie-compat, --no-js, --compress, ...
+        $args = ['--no-color', '--math="always"']; // for example: --no-ie-compat, --no-js, --compress, ...

         return implode(' ', $args);
     }

Do you have an idea how we could make it more configurable? Maybe we could pass a parameter somehow via environment variables or deployment config.

hostep commented 1 month ago

Hey @piotrkwiecinski: I'm not very keen on introducing configuration settings in this module. As it would need to work as well without a database connection. So it then it should be a config setting in the app/etc/confg.php or app/etc/env.php file I guess. Well actually, it's doable but it will take a bit of time to implement.

An alternative approach to the current patch you are using, is that you use the changes to the less files as a patch from this PR: https://github.com/magento/magento2/pull/38335/files, those changes should also fix your problem with less.js v4 I believe, and already got included in Magento 2.4.7. You may also want to use a patch from these changes https://github.com/magento/magento2/pull/37842/files, which should fix some other smaller issues.

piotrkwiecinski commented 1 month ago

@hostep thank you for the fast reply.

We'll have a look. I agree with keeping this module lean.

Ultimately it's an issue with magento core.

I was thinking more about something tiny like:

[getenv('LESS_COMPILES_OPTIONS')] ?? ['--no-color']
piotrkwiecinski commented 1 month ago

And thank you for an awesome module. Our initial tests shows it shaved ~3.5 min from a deployment run on Github action.

hostep commented 1 month ago

Hi @piotrkwiecinski, could you give the changes from https://github.com/baldwin-agency/magento2-module-less-js-compiler/pull/10 a try? You can use the dev-fix-for-issue-9 version constraint in composer to pull it in. See the new configuration documentation for how to set it up: https://github.com/baldwin-agency/magento2-module-less-js-compiler/tree/fix-for-issue-9?tab=readme-ov-file#configuration

Are you okay with this solution? It's not using environment variables though. I was hoping that we would also be able to use the CONFIG__DEFAULT__DEV__LESS_JS_COMPILER__LESS_ARGUMENTS environment variable like Adobe explains here, but I can't seem to get that working on my machine...

piotrkwiecinski commented 1 month ago

Hi @hostep. We already push config.php to repo to build theme in CI so it seems like a good solution. I'll let you know tomorrow, but it looks promising.

hostep commented 3 weeks ago

Hey @piotrkwiecinski, this feature is now released in version 1.6.0

Thanks for the suggestions!