Skip to content

Commit 24201e7

Browse files
Krinkleimorland
andauthored
feat: update wikimedia/less.php from 4.x to 5.x (#4225)
* chore: Prep for upgrade to Less.php 5 The only two major changes in Less.php 5.0 are: * Remove `import_callback`. * Change default math from "always" (strictMath: false) to "parens-division". The import_callback option was deprecated in 4.x, because it exposed a number of internal classes, all of which had to be used in order to work, including carefully obtaining and then returning the `pathAndUri[1]` value. Upstream change: https://gerrit.wikimedia.org/r/1010962 Docs: https://github.com/wikimedia/less.php/blob/v4.4.1/lib/Less/Parser.php#L592-L617 == import_callback == The SetImportDirs() method has been available for a long time already, and is instead given a simple string path (already resolved), and must return `null|array{0:?string,1:?string}`. Thus if it doesn't want to override, it can simply not return (implied null), which is equiv to `[null,null]` and the second URI value is optional, defaulting to the default. == math == In Less.js 1.x-3.x the default was `strictMath: false`, which evaluates math eagerly, instead of only "strictly" between parenthesis. This had the downside of interpreting "/" slashes in newer CSS syntax as Less division, instead of as separator between values in those CSS features (aspect-ratio, border-radius, font). As such, Less.js 4.x and Less.php 5.x, rename "strictMath" to "math", with a new third option "parens-division" as the default alongside "always" (aka `strictMath: false`) and "parens" (aka `strictMath: true`). This new option still eagerly evaluated math in most cases (plus, minus, multiply) but no longer for division. I don't know if or when Flarum wants to adopt this new default, but for now, to minimise disruption and unblock the Less.php upgrade, I suggest simply setting `strictMath: false` explicitly so that Less.php 5 behaves the same as before. This decouples the trivial Less.php upgrade from the (potentially non-trivial, or unwanted) math migration. Ref https://phabricator.wikimedia.org/T366445. Ref https://github.com/wikimedia/less.php/blob/master/CHANGES.md. == Custom properties == In Less.js 3.5, custom properties were natively supported which included a change to no longer evaluate math expressions there. This is separate from the general case, which remains eagerly evaluated under the `strictMath:false` setting. You can still perform calclations in custom properties with `calc()`. Ref less/less.js#3317 * feat: update wikimedia/less.php from 4.x to 5.x * fix: workaround the removed import_callback so that replaced less is still processed * chore: styleci --------- Co-authored-by: IanM <[email protected]>
1 parent 813bff7 commit 24201e7

File tree

5 files changed

+47
-41
lines changed

5 files changed

+47
-41
lines changed

composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@
172172
"symfony/postmark-mailer": "^7.0",
173173
"symfony/translation": "^7.0",
174174
"symfony/yaml": "^7.0",
175-
"wikimedia/less.php": "^4.1",
175+
"wikimedia/less.php": "^5.3",
176176
"plesk/ratchetphp": "v1.0.4"
177177
},
178178
"require-dev": {

framework/core/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@
9191
"symfony/translation-contracts": "^2.5",
9292
"symfony/yaml": "^7.0",
9393
"flarum/json-api-server": "^0.1.0",
94-
"wikimedia/less.php": "^4.1"
94+
"wikimedia/less.php": "^5.3"
9595
},
9696
"require-dev": {
9797
"flarum/testing": "^2.0",

framework/core/src/Frontend/Compiler/LessCompiler.php

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,8 @@
1111

1212
use Flarum\Frontend\Compiler\Source\FileSource;
1313
use Illuminate\Support\Collection;
14-
use Illuminate\Support\Str;
1514
use Less_Exception_Compiler;
16-
use Less_FileManager;
1715
use Less_Parser;
18-
use Less_Tree_Import;
1916

2017
/**
2118
* @internal
@@ -83,9 +80,9 @@ protected function compile(array $sources): string
8380
try {
8481
$parser = new Less_Parser([
8582
'compress' => true,
83+
'strictMath' => false,
8684
'cache_dir' => $this->cacheDir,
8785
'import_dirs' => $this->importDirs,
88-
'import_callback' => $this->lessImportOverrides ? $this->overrideImports($sources) : null,
8986
]);
9087

9188
if ($this->fileSourceOverrides) {
@@ -94,7 +91,15 @@ protected function compile(array $sources): string
9491

9592
foreach ($sources as $source) {
9693
if ($source instanceof FileSource) {
97-
$parser->parseFile($source->getPath());
94+
// If we have import overrides, parse the file content and apply them
95+
if ($this->lessImportOverrides && $this->lessImportOverrides->isNotEmpty()) {
96+
$content = file_get_contents($source->getPath());
97+
$content = $this->applyImportOverridesToContent($content);
98+
// Pass the original file path to maintain proper import resolution context
99+
$parser->parse($content, $source->getPath());
100+
} else {
101+
$parser->parseFile($source->getPath());
102+
}
98103
} else {
99104
$parser->parse($source->getContent());
100105
}
@@ -137,6 +142,35 @@ protected function finalize(string $parsedCss): string
137142
return str_replace('url("../webfonts/', 'url("./fonts/', $parsedCss);
138143
}
139144

145+
/**
146+
* Apply import overrides by replacing @import statements with inline content.
147+
*/
148+
private function applyImportOverridesToContent(string $content): string
149+
{
150+
foreach ($this->lessImportOverrides as $override) {
151+
$file = $override['file'];
152+
$fileWithoutExt = preg_replace('/\.less$/i', '', $file);
153+
$quotedFile = preg_quote($fileWithoutExt, '/');
154+
155+
// Match @import "path" or @import 'path' (with or without .less extension)
156+
$pattern = '/@import\s+["\']'.$quotedFile.'(\.less)?["\'];?/i';
157+
158+
if (preg_match($pattern, $content)) {
159+
// Read the override file content
160+
$overrideContent = file_get_contents($override['newFilePath']);
161+
162+
// Replace the @import statement with the actual content
163+
$content = preg_replace(
164+
$pattern,
165+
'/* Flarum override: '.$file.' */'."\n".$overrideContent."\n".'/* End override */',
166+
$content
167+
);
168+
}
169+
}
170+
171+
return $content;
172+
}
173+
140174
protected function overrideSources(array $sources): array
141175
{
142176
foreach ($sources as $source) {
@@ -155,38 +189,6 @@ protected function overrideSources(array $sources): array
155189
return $sources;
156190
}
157191

158-
protected function overrideImports(array $sources): callable
159-
{
160-
$baseSources = (new Collection($sources))->filter(function ($source) {
161-
return $source instanceof Source\FileSource;
162-
})->map(function (FileSource $source) {
163-
$path = realpath($source->getPath());
164-
$path = Str::beforeLast($path, '/less/');
165-
166-
return [
167-
'path' => $path,
168-
'extensionId' => $source->getExtensionId(),
169-
];
170-
})->unique('path');
171-
172-
return function (Less_Tree_Import $evald) use ($baseSources): ?array {
173-
$pathAndUri = Less_FileManager::getFilePath($evald->getPath(), $evald->currentFileInfo);
174-
175-
$relativeImportPath = Str::of($pathAndUri[0])->split('/\/less\//');
176-
$extensionId = $baseSources->where('path', $relativeImportPath->first())->pluck('extensionId')->first();
177-
178-
$overrideImport = $this->lessImportOverrides
179-
->where('file', $relativeImportPath->last())
180-
->firstWhere('extensionId', $extensionId);
181-
182-
if (! $overrideImport) {
183-
return null;
184-
}
185-
186-
return [$overrideImport['newFilePath'], $pathAndUri[1]];
187-
};
188-
}
189-
190192
protected function getCacheDifferentiator(): ?array
191193
{
192194
return [

framework/core/tests/fixtures/less/custom_function.less

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
color: green;
33
}
44
.dummy_func_test2 {
5-
--x: is-flarum("not flarum") * 10;
5+
width: is-flarum("not flarum") * 10px;
66
--y: is-gt(1, 2);
77
}

framework/core/tests/integration/extenders/ThemeTest.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ public function theme_extender_override_import_works()
3636

3737
$response = $this->send($this->request('GET', '/'));
3838

39+
if ($response->getStatusCode() !== 200) {
40+
echo "\n\nResponse body:\n".$response->getBody()->getContents()."\n\n";
41+
}
42+
3943
$this->assertEquals(200, $response->getStatusCode());
4044

4145
$cssFilePath = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets')->path('forum.css');
@@ -119,7 +123,7 @@ public function theme_extender_can_add_custom_function()
119123
$contents = file_get_contents($cssFilePath);
120124

121125
$this->assertStringContainsString('.dummy_func_test{color:green}', $contents);
122-
$this->assertStringContainsString('.dummy_func_test2{--x:1000;--y:false}', $contents);
126+
$this->assertStringContainsString('.dummy_func_test2{width:1000px;--y:false}', $contents);
123127
}
124128

125129
#[Test]

0 commit comments

Comments
 (0)