From e4e3eb22f4ded776b99091eb1cf86cf1a02383fb Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Fri, 10 Sep 2021 18:45:18 +0100 Subject: [PATCH] [1.x] Theme Extender to Allow overriding `LESS` files (#3008) This PR introduces the ability to just override a LESS file's contents through an extender. This is mainly useful for theme development, as there are times in extensively customized themes where overriding the actual file makes a huge difference vs overriding CSS styles which can turn into a maintenance hell real fast. Overriding styles is more tedious than overriding files. When you're designing an element, you would normally rather start from a blank canvas, than a styled element. With an already styled element you have to first override and undo the styles you do not wish to have, only then can you start shaping it, but even then you'd always end up constantly undoing default styles. This mostly applies for more advanced themes. (example: https://github.com/afrux/asirem/blob/851c55516d2ab279928a0a0318ca40d49644dd98/less/forum/DiscussionList.less) --- framework/core/src/Extend/Frontend.php | 4 +- framework/core/src/Extend/Theme.php | 69 +++++++++++ framework/core/src/Frontend/Assets.php | 28 +++++ .../src/Frontend/Compiler/LessCompiler.php | 77 ++++++++++++- .../Frontend/Compiler/Source/FileSource.php | 18 ++- .../Compiler/Source/SourceCollector.php | 4 +- .../core/tests/fixtures/less/Imported.less | 3 + framework/core/tests/fixtures/less/dummy.less | 1 + framework/core/tests/fixtures/less/forum.less | 5 + .../fixtures/less/override_filesource.less | 3 + .../tests/integration/extenders/ThemeTest.php | 107 ++++++++++++++++++ 11 files changed, 313 insertions(+), 6 deletions(-) create mode 100644 framework/core/src/Extend/Theme.php create mode 100644 framework/core/tests/fixtures/less/Imported.less create mode 100644 framework/core/tests/fixtures/less/dummy.less create mode 100644 framework/core/tests/fixtures/less/forum.less create mode 100644 framework/core/tests/fixtures/less/override_filesource.less create mode 100644 framework/core/tests/integration/extenders/ThemeTest.php diff --git a/framework/core/src/Extend/Frontend.php b/framework/core/src/Extend/Frontend.php index 0cf2938e3..9484a1213 100644 --- a/framework/core/src/Extend/Frontend.php +++ b/framework/core/src/Extend/Frontend.php @@ -153,9 +153,9 @@ class Frontend implements ExtenderInterface } if ($this->css) { - $assets->css(function (SourceCollector $sources) { + $assets->css(function (SourceCollector $sources) use ($moduleName) { foreach ($this->css as $path) { - $sources->addFile($path); + $sources->addFile($path, $moduleName); } }); } diff --git a/framework/core/src/Extend/Theme.php b/framework/core/src/Extend/Theme.php new file mode 100644 index 000000000..74fc4fa0e --- /dev/null +++ b/framework/core/src/Extend/Theme.php @@ -0,0 +1,69 @@ +lessImportOverrides[] = compact('file', 'newFilePath', 'extensionId'); + + return $this; + } + + /** + * This method allows overriding LESS file sources. + * For example `forum.less`, `admin.less`, `mixins.less` and `variables.less` are file sources, + * and can therefore be overriden using this method. + * + * @param string $file : Name of the file to override, for example: `admin.less` + * @param string $newFilePath : Absolute path of the new file. + * @param string|null $extensionId : If overriding an extension file, specify its ID, for example: `flarum-tags`. + * @return self + */ + public function overrideFileSource(string $file, string $newFilePath, string $extensionId = null): self + { + $this->fileSourceOverrides[] = compact('file', 'newFilePath', 'extensionId'); + + return $this; + } + + public function extend(Container $container, Extension $extension = null) + { + $container->extend('flarum.assets.factory', function (callable $factory) { + return function (...$args) use ($factory) { + /** @var Assets $assets */ + $assets = $factory(...$args); + + $assets->addLessImportOverrides($this->lessImportOverrides); + $assets->addFileSourceOverrides($this->fileSourceOverrides); + + return $assets; + }; + }); + } +} diff --git a/framework/core/src/Frontend/Assets.php b/framework/core/src/Frontend/Assets.php index 8b793697e..d52ecdfa4 100644 --- a/framework/core/src/Frontend/Assets.php +++ b/framework/core/src/Frontend/Assets.php @@ -52,6 +52,16 @@ class Assets */ protected $lessImportDirs; + /** + * @var array + */ + protected $lessImportOverrides = []; + + /** + * @var array + */ + protected $fileSourceOverrides = []; + public function __construct(string $name, Filesystem $assetsDir, string $cacheDir = null, array $lessImportDirs = null) { $this->name = $name; @@ -155,6 +165,14 @@ class Assets $compiler->setImportDirs($this->lessImportDirs); } + if ($this->lessImportOverrides) { + $compiler->setLessImportOverrides($this->lessImportOverrides); + } + + if ($this->fileSourceOverrides) { + $compiler->setFileSourceOverrides($this->fileSourceOverrides); + } + return $compiler; } @@ -197,4 +215,14 @@ class Assets { $this->lessImportDirs = $lessImportDirs; } + + public function addLessImportOverrides(array $lessImportOverrides) + { + $this->lessImportOverrides = array_merge($this->lessImportOverrides, $lessImportOverrides); + } + + public function addFileSourceOverrides(array $fileSourceOverrides) + { + $this->fileSourceOverrides = array_merge($this->fileSourceOverrides, $fileSourceOverrides); + } } diff --git a/framework/core/src/Frontend/Compiler/LessCompiler.php b/framework/core/src/Frontend/Compiler/LessCompiler.php index 615c23f22..a88061624 100644 --- a/framework/core/src/Frontend/Compiler/LessCompiler.php +++ b/framework/core/src/Frontend/Compiler/LessCompiler.php @@ -10,6 +10,8 @@ namespace Flarum\Frontend\Compiler; use Flarum\Frontend\Compiler\Source\FileSource; +use Illuminate\Support\Collection; +use Illuminate\Support\Str; use Less_Parser; /** @@ -27,6 +29,16 @@ class LessCompiler extends RevisionCompiler */ protected $importDirs = []; + /** + * @var Collection + */ + protected $lessImportOverrides; + + /** + * @var Collection + */ + protected $fileSourceOverrides; + public function getCacheDir(): string { return $this->cacheDir; @@ -47,6 +59,16 @@ class LessCompiler extends RevisionCompiler $this->importDirs = $importDirs; } + public function setLessImportOverrides(array $lessImportOverrides) + { + $this->lessImportOverrides = new Collection($lessImportOverrides); + } + + public function setFileSourceOverrides(array $fileSourceOverrides) + { + $this->fileSourceOverrides = new Collection($fileSourceOverrides); + } + /** * @throws \Less_Exception_Parser */ @@ -61,9 +83,14 @@ class LessCompiler extends RevisionCompiler $parser = new Less_Parser([ 'compress' => true, 'cache_dir' => $this->cacheDir, - 'import_dirs' => $this->importDirs + 'import_dirs' => $this->importDirs, + 'import_callback' => $this->lessImportOverrides ? $this->overrideImports($sources) : null, ]); + if ($this->fileSourceOverrides) { + $sources = $this->overrideSources($sources); + } + foreach ($sources as $source) { if ($source instanceof FileSource) { $parser->parseFile($source->getPath()); @@ -75,6 +102,54 @@ class LessCompiler extends RevisionCompiler return $parser->getCss(); } + protected function overrideSources(array $sources): array + { + foreach ($sources as $source) { + if ($source instanceof FileSource) { + $basename = basename($source->getPath()); + $override = $this->fileSourceOverrides + ->where('file', $basename) + ->firstWhere('extensionId', $source->getExtensionId()); + + if ($override) { + $source->setPath($override['newFilePath']); + } + } + } + + return $sources; + } + + protected function overrideImports(array $sources): callable + { + $baseSources = (new Collection($sources))->filter(function ($source) { + return $source instanceof Source\FileSource; + })->map(function (FileSource $source) { + $path = realpath($source->getPath()); + $path = Str::beforeLast($path, '/less/'); + + return [ + 'path' => $path, + 'extensionId' => $source->getExtensionId(), + ]; + })->unique('path'); + + return function ($evald) use ($baseSources): ?array { + $relativeImportPath = Str::of($evald->PathAndUri()[0])->split('/\/less\//'); + $extensionId = $baseSources->where('path', $relativeImportPath->first())->pluck('extensionId')->first(); + + $overrideImport = $this->lessImportOverrides + ->where('file', $relativeImportPath->last()) + ->firstWhere('extensionId', $extensionId); + + if (! $overrideImport) { + return null; + } + + return [$overrideImport['newFilePath'], $evald->PathAndUri()[1]]; + }; + } + protected function getCacheDifferentiator(): ?array { return [ diff --git a/framework/core/src/Frontend/Compiler/Source/FileSource.php b/framework/core/src/Frontend/Compiler/Source/FileSource.php index f85c5926e..6fcd9b642 100644 --- a/framework/core/src/Frontend/Compiler/Source/FileSource.php +++ b/framework/core/src/Frontend/Compiler/Source/FileSource.php @@ -21,16 +21,22 @@ class FileSource implements SourceInterface */ protected $path; + /** + * @var string + */ + protected $extensionId; + /** * @param string $path */ - public function __construct(string $path) + public function __construct(string $path, ?string $extensionId = null) { if (! file_exists($path)) { throw new InvalidArgumentException("File not found at path: $path"); } $this->path = $path; + $this->extensionId = $extensionId; } /** @@ -56,4 +62,14 @@ class FileSource implements SourceInterface { return $this->path; } + + public function setPath(string $path): void + { + $this->path = $path; + } + + public function getExtensionId(): ?string + { + return $this->extensionId; + } } diff --git a/framework/core/src/Frontend/Compiler/Source/SourceCollector.php b/framework/core/src/Frontend/Compiler/Source/SourceCollector.php index ec537b26b..452d78e87 100644 --- a/framework/core/src/Frontend/Compiler/Source/SourceCollector.php +++ b/framework/core/src/Frontend/Compiler/Source/SourceCollector.php @@ -23,9 +23,9 @@ class SourceCollector * @param string $file * @return $this */ - public function addFile(string $file) + public function addFile(string $file, string $extensionId = null) { - $this->sources[] = new FileSource($file); + $this->sources[] = new FileSource($file, $extensionId); return $this; } diff --git a/framework/core/tests/fixtures/less/Imported.less b/framework/core/tests/fixtures/less/Imported.less new file mode 100644 index 000000000..b6cdd664b --- /dev/null +++ b/framework/core/tests/fixtures/less/Imported.less @@ -0,0 +1,3 @@ +.Imported { + // ... +} diff --git a/framework/core/tests/fixtures/less/dummy.less b/framework/core/tests/fixtures/less/dummy.less new file mode 100644 index 000000000..e882088e6 --- /dev/null +++ b/framework/core/tests/fixtures/less/dummy.less @@ -0,0 +1 @@ +.dummy_test_case{color:red} diff --git a/framework/core/tests/fixtures/less/forum.less b/framework/core/tests/fixtures/less/forum.less new file mode 100644 index 000000000..ab882c025 --- /dev/null +++ b/framework/core/tests/fixtures/less/forum.less @@ -0,0 +1,5 @@ +@import 'Imported'; + +.dummy { + color: yellow; +} diff --git a/framework/core/tests/fixtures/less/override_filesource.less b/framework/core/tests/fixtures/less/override_filesource.less new file mode 100644 index 000000000..7f69ee733 --- /dev/null +++ b/framework/core/tests/fixtures/less/override_filesource.less @@ -0,0 +1,3 @@ +body { + color: orange; +} diff --git a/framework/core/tests/integration/extenders/ThemeTest.php b/framework/core/tests/integration/extenders/ThemeTest.php new file mode 100644 index 000000000..87b828494 --- /dev/null +++ b/framework/core/tests/integration/extenders/ThemeTest.php @@ -0,0 +1,107 @@ +send($this->request('GET', '/')); + + $this->assertEquals(200, $response->getStatusCode()); + + $cssFilePath = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets')->path('forum.css'); + $this->assertStringNotContainsString('.dummy_test_case{color:red}', file_get_contents($cssFilePath)); + } + + /** + * @test + */ + public function theme_extender_override_import_works() + { + $this->extend( + (new Extend\Theme) + ->overrideLessImport('forum/Hero.less', __DIR__.'/../../fixtures/less/dummy.less') + ); + + $response = $this->send($this->request('GET', '/')); + + $this->assertEquals(200, $response->getStatusCode()); + + $cssFilePath = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets')->path('forum.css'); + + $this->assertStringContainsString('.dummy_test_case{color:red}', file_get_contents($cssFilePath)); + } + + /** + * @test + */ + public function theme_extender_override_import_works_with_external_sources() + { + $this->extend( + (new Extend\Frontend('forum')) + ->css(__DIR__.'/../../fixtures/less/forum.less'), + (new Extend\Theme) + ->overrideLessImport('Imported.less', __DIR__.'/../../fixtures/less/dummy.less', 'site-custom') + ); + + $response = $this->send($this->request('GET', '/')); + + $this->assertEquals(200, $response->getStatusCode()); + + $cssFilePath = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets')->path('forum.css'); + $contents = file_get_contents($cssFilePath); + + $this->assertStringNotContainsString('.Imported', $contents); + $this->assertStringContainsString('.dummy_test_case{color:red}', $contents); + $this->assertStringContainsString('.dummy{color:yellow}', $contents); + } + + /** + * @test + */ + public function theme_extender_override_file_source_works() + { + $this->extend( + (new Extend\Theme) + ->overrideFileSource('forum.less', __DIR__.'/../../fixtures/less/override_filesource.less') + ); + + $response = $this->send($this->request('GET', '/')); + + $this->assertEquals(200, $response->getStatusCode()); + + $cssFilePath = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets')->path('forum.css'); + + $this->assertEquals('body{color:orange}', file_get_contents($cssFilePath)); + } + + /** + * @test + */ + public function theme_extender_override_file_source_works_by_failing_when_necessary() + { + $this->extend( + (new Extend\Theme) + ->overrideFileSource('mixins.less', __DIR__.'/../../fixtures/less/dummy.less') + ); + + $response = $this->send($this->request('GET', '/')); + + $this->assertStringContainsString('Less_Exception_Compiler', $response->getBody()->getContents()); + $this->assertEquals(500, $response->getStatusCode()); + } +}