Skip to content

Made it possible to overwrite the arguments we send to the 'node' and… #10

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

Merged
merged 3 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 65 additions & 14 deletions Css/PreProcessor/Adapter/Less/Processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Baldwin\LessJsCompiler\Css\PreProcessor\Adapter\Less;

use Magento\Framework\App\Config\ScopeConfigInterface;
use Magento\Framework\App\ProductMetadataInterface;
use Magento\Framework\Css\PreProcessor\File\Temporary;
use Magento\Framework\Exception\LocalizedException;
Expand All @@ -25,6 +26,7 @@ class Processor implements ContentProcessorInterface
private $productMetadata;
private $filesystem;
private $directoryList;
private $scopeConfig;

public function __construct(
LoggerInterface $logger,
Expand All @@ -33,7 +35,8 @@ public function __construct(
ShellInterface $shell,
ProductMetadataInterface $productMetadata,
Filesystem $filesystem,
DirectoryList $directoryList
DirectoryList $directoryList,
ScopeConfigInterface $scopeConfig
) {
$this->logger = $logger;
$this->assetSource = $assetSource;
Expand All @@ -42,6 +45,7 @@ public function __construct(
$this->productMetadata = $productMetadata;
$this->filesystem = $filesystem;
$this->directoryList = $directoryList;
$this->scopeConfig = $scopeConfig;
}

/**
Expand Down Expand Up @@ -89,24 +93,28 @@ public function processContent(AssetFile $asset)
*/
protected function compileFile($filePath)
{
$nodeCmdArgs = $this->getNodeArgsAsString();
$lessCmdArgs = $this->getCompilerArgsAsString();
$cmd = "%s $nodeCmdArgs %s $lessCmdArgs %s";
$nodeCmdArgs = $this->getNodeArgsAsArray();
$lessCmdArgs = $this->getCompilerArgsAsArray();

$cmd = '%s ' . str_repeat(' %s', count($nodeCmdArgs)) . ' %s' . str_repeat(' %s', count($lessCmdArgs)) . ' %s';
$arguments = [];
$arguments[] = $this->getPathToNodeBinary();
$arguments = array_merge($arguments, $nodeCmdArgs);
$arguments[] = $this->getPathToLessCompiler();
$arguments = array_merge($arguments, $lessCmdArgs);
$arguments[] = $filePath;

// to log or not to log, that's the question
// also, it would be better to use the logger in the Shell class,
// since that one will contain the exact correct command, and not this sprintf version
// $logArguments = array_map('escapeshellarg', $arguments);
// $this->logger->debug('Less compilation command: `'
// . sprintf($cmd, $this->getPathToNodeBinary(), $this->getPathToLessCompiler(), $filePath)
// . sprintf($cmd, ...$logArguments)
// . '`');

return $this->shell->execute(
$cmd,
[
$this->getPathToNodeBinary(),
$this->getPathToLessCompiler(),
$filePath,
]
$arguments
);
}

Expand All @@ -117,9 +125,23 @@ protected function compileFile($filePath)
*/
protected function getCompilerArgsAsString()
{
$args = ['--no-color']; // for example: --no-ie-compat, --no-js, --compress, ...
$args = $this->getConfigValueFromPath('dev/less_js_compiler/less_arguments');
if ($args === null) {
// default supplied args
$args = '--no-color'; // for example: --ie-compat --compress --math="always", ...
}

return implode(' ', $args);
return $args;
}

/**
* Get all arguments which will be used in the cli call to the lessc compiler
*
* @return array<string>
*/
protected function getCompilerArgsAsArray()
{
return explode(' ', $this->getCompilerArgsAsString());
}

/**
Expand Down Expand Up @@ -154,9 +176,23 @@ protected function getPathToLessCompiler()
*/
protected function getNodeArgsAsString()
{
$args = ['--no-deprecation']; // squelch warnings about deprecated modules being used
$args = $this->getConfigValueFromPath('dev/less_js_compiler/node_arguments');
if ($args === null) {
// default supplied args
$args = '--no-deprecation'; // squelch warnings about deprecated modules being used
}

return implode(' ', $args);
return $args;
}

/**
* Get all arguments which will be used in the cli call with the nodejs binary
*
* @return array<string>
*/
protected function getNodeArgsAsArray()
{
return explode(' ', $this->getNodeArgsAsString());
}

/**
Expand Down Expand Up @@ -207,4 +243,19 @@ protected function outputErrorMessage($errorMessage, AssetFile $file)

$this->logger->critical($errorMessage);
}

/**
* @param string $path
*
* @return ?string
*/
private function getConfigValueFromPath($path)
{
$value = $this->scopeConfig->getValue($path);
if (is_string($value)) {
return $value;
}

return null;
}
}
33 changes: 27 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ You'll also need [composer](https://getcomposer.org/) to add this module to your
First, we recommend you to install the less compiler itself, and save it into your `package.json` file as a production dependency:

```sh
npm install --save less@1.7.5
npm install --save less@3.13.1
```

> Watch out: from Magento 2.1 onwards, the `package.json` file is being renamed to `package.json.sample` to enable you to have your own nodejs dependencies without Magento overwriting this every time with its own version each time you update Magento. So if you use Magento >= 2.1 make sure you copy the `package.json.sample` file to `package.json` before running the above command if you want to work with Magento's `grunt` setup.
Expand Down Expand Up @@ -53,18 +53,39 @@ bin/magento setup:upgrade

As the last step, in your deploy scripts, make sure you call `npm install --production`. You should execute this somewhere between `composer install` and `bin/magento setup:static-content:deploy`

## Configuration

You have the opportunity to configure the arguments we send to the `lessc` and `node` commands.
The defaults are:

- `lessc --no-color`
- `node --no-deprecation`

If you want to override the default arguments, you can do this by modifying your `app/etc/config.php` or `app/etc/env.php` file and adding something like this:

```php
'system' => [
'default' => [
'dev' => [
'less_js_compiler' => [
'less_arguments' => '--no-color --ie-compat',
'node_arguments' => '--no-deprecation --no-warnings',
]
]
]
]
```

## Debugging less compilation errors

When your `.less` files have a syntax error or contain something which doesn't allow it to compile properly, please have a look at the `var/log/system.log` file, it will contain the error what causes the problem.

## Remarks

1. Installing this module will effectively replace the default less compiler from Magento2. If you want to go back to the default less compiler, you need to disable this module or uninstall it.
2. We strongly recommend you to install less.js version 1.7.5, and not the very latest version (which is 2.7.1 at the time). Magento's built-in less.php library is based on less.js version 1.x and isn't compatible with 2.x. This means Magento has only tested their less files with a less compiler which is compatible with version 1.x of less. If you want to use version 2.x (which you certainly can), be aware that there might be subtle changes in the resulting css.
Also: if you use `grunt` or `gulp` in your frontend workflow, you are probably also using less.js version 1.7.5, so using this module makes sure what you see on the server is 100% exactly the same as on your developer machine.
3. This module expects the less compiler to exist in `{MAGENTO_ROOT_DIR}/node_modules/.bin/lessc`, this is a hard coded path which is being used in the module. The compiler will end up there if you follow the installation steps above, but if for some reason you prefer to install your nodejs modules someplace else, then this module won't work. If somebody actually has this problem and has an idea how to make this path configurable (preferably without getting it from the database), please let me know!
4. The default less processor in Magento 2 passes an option to the less compiler, which says it should [compress the resulting css file](https://github.com/magento/magento2/blob/6a40b41f6281c7d405cd78029d6becab1d837c87/lib/internal/Magento/Framework/Css/PreProcessor/Adapter/Less/Processor.php#L73). In this module, we have chosen not to do so, as we believe this isn't a task to be executed while compiling less files. It should be done further down the line, like for example during the minification phase. If someone disagrees with this, please let me know, I'm open to discussion about this.
5. This module was tested against Magento versions 2.0.7, 2.1.0 - 2.1.9 and 2.2.0
2. This module expects the less compiler to exist in `{MAGENTO_ROOT_DIR}/node_modules/.bin/lessc`, this is a hard coded path which is being used in the module. The compiler will end up there if you follow the installation steps above, but if for some reason you prefer to install your nodejs modules someplace else, then this module won't work. If somebody actually has this problem and wants us to make this configurable, please let me know!
3. The default less processor in Magento 2 passes an option to the less compiler, which says it should [compress the resulting css file](https://github.com/magento/magento2/blob/6a40b41f6281c7d405cd78029d6becab1d837c87/lib/internal/Magento/Framework/Css/PreProcessor/Adapter/Less/Processor.php#L73). In this module, we have chosen not to do so, as we believe this isn't a task to be executed while compiling less files. It should be done further down the line, like for example during the minification phase. If someone disagrees with this, please let me know, I'm open to discussion about this.
4. This module was tested against Magento versions 2.0.7, 2.1.x, 2.2.x, 2.3.x, 2.4.x

## Benchmarks

Expand Down