Skip to content
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.

Sign up for GitHub

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

Adds a mix Twig filter for Laravel Mix theme assets. #1179

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft

Adds a mix Twig filter for Laravel Mix theme assets. #1179

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d00b9f6
Adds a `mix` Twig filter for Laravel Mix theme assets.
ericp-mrel Aug 4, 2024
4f89e79
Started adding working tests for `Mix` helper class.
ericp-mrel Aug 18, 2024
24caf18
Generate asset URLs directly rather than relying on the Theme's `asse…
ericp-mrel Aug 22, 2024
5692e71
Finish creating Mix class tests.
ericp-mrel Aug 22, 2024
ead0189
Add test for Laravel Mix Twig filter.
ericp-mrel Aug 22, 2024
5fb257f
Add additional tests for Mix class.
ericp-mrel Aug 22, 2024
3100bbc
Fix code styling.
ericp-mrel Sep 13, 2024
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
11 changes: 11 additions & 0 deletions modules/cms/tests/fixtures/npm/package-mixtheme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"type": "module",
"workspaces": {
"packages": [
"modules/cms/tests/fixtures/themes/mixtest"
]
},
"devDependencies": {
"laravel-mix": "^6.0.41"
}
}
3 changes: 3 additions & 0 deletions modules/cms/tests/fixtures/themes/mixtest/assets/src/css/theme.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
color: red;
}
10 changes: 10 additions & 0 deletions modules/cms/tests/fixtures/themes/mixtest/winter.mix.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const mix = require('laravel-mix');

mix.setPublicPath(__dirname)
.options({
manifest: true,
})
.version()

// Render Tailwind style
.postCss('assets/src/css/theme.css', 'assets/dist/css/theme.css')
73 changes: 73 additions & 0 deletions modules/cms/tests/twig/MixFilterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

namespace Cms\Tests\Twig;

use Cms\Classes\Controller;
use Cms\Classes\Theme;
use Cms\Twig\Extension;
use System\Tests\Bootstrap\TestCase;
use Winter\Storm\Support\Facades\Config;
use Winter\Storm\Support\Facades\Event;
use Winter\Storm\Support\Facades\File;
use Winter\Storm\Support\Facades\Twig;

class MixFilterTest extends TestCase
{
protected function setUp(): void
{
parent::setUp();

if (!is_dir(base_path('node_modules'))) {
$this->markTestSkipped('This test requires node_modules to be installed');
}

if (!is_file(base_path('node_modules/.bin/mix'))) {
$this->markTestSkipped('This test requires the mix package to be installed');
}

$this->originalThemesPath = Config::get('cms.themesPath');
Config::set('cms.themesPath', '/modules/cms/tests/fixtures/themes');

$this->themePath = base_path('modules/cms/tests/fixtures/themes/mixtest');

Config::set('cms.activeTheme', 'mixtest');

Event::flush('cms.theme.getActiveTheme');
Theme::resetCache();
}

protected function tearDown(): void
{
File::deleteDirectory('modules/cms/tests/fixtures/themes/mixtest/assets/dist');
File::delete('modules/cms/tests/fixtures/themes/mixtest/mix-manifest.json');

Config::set('cms.themesPath', $this->originalThemesPath);

parent::tearDown();
}

public function testGeneratesAssetUrl(): void
{
$theme = Theme::getActiveTheme();

$this->artisan('mix:compile', [
'theme-mixtest',
'--manifest' => 'modules/cms/tests/fixtures/npm/package-mixtheme.json',
'--disable-tty' => true,
])->assertExitCode(0);

$this->assertFileExists($theme->getPath($theme->getDirName() . '/mix-manifest.json'));

$controller = Controller::getController() ?: new Controller();

$extension = new Extension();
$extension->setController($controller);

$this->app->make('twig.environment')
->addExtension($extension);

$contents = Twig::parse("{{ 'assets/dist/css/theme.css' | mix }}");

$this->assertStringContainsString('/assets/dist/css/theme.css?id=', $contents);
}
}
2 changes: 2 additions & 0 deletions modules/cms/twig/Extension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use Block;
use Cms\Classes\Controller;
use Event;
use System\Classes\Asset\Mix;
use System\Classes\Asset\Vite;
use Twig\Extension\AbstractExtension as TwigExtension;
use Twig\TwigFilter as TwigSimpleFilter;
Expand Down Expand Up @@ -68,6 +69,7 @@ public function getFilters(): array
return [
new TwigSimpleFilter('page', [$this, 'pageFilter'], $options),
new TwigSimpleFilter('theme', [$this, 'themeFilter'], $options),
new TwigSimpleFilter('mix', [Mix::class, 'mix'], $options),
];
}

Expand Down
55 changes: 55 additions & 0 deletions modules/system/classes/asset/Mix.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

namespace System\Classes\Asset;

use Cms\Classes\Theme;
use Illuminate\Support\HtmlString;
use Winter\Storm\Support\Facades\Config;
use Winter\Storm\Support\Facades\Url;
use Winter\Storm\Support\Str;

class Mix
{
public static function mix(string $path, string $manifestDirectory = ''): HtmlString|string
{
static $manifests = [];

$theme = Theme::getActiveTheme();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be in the CMS module instead since it requires the CMS module.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it would be better if I were to update the Mix class, so it acts similar to the Vite class? Where you would pass the path of the asset and then the package name of where it belongs to, so it doesn't have to rely on the Theme class and then can work for plugins as well?

It would then be used like this:

<script src="{{ mix('assets/dist/js/theme-mytheme.js', 'theme-mytheme') }}" defer></script>

Mix class rewrite:

<?php

namespace System\Classes\Asset;

use Exception;
use Illuminate\Support\HtmlString;
use InvalidArgumentException;
use Winter\Storm\Exception\SystemException;
use Winter\Storm\Support\Facades\Url;
use Winter\Storm\Support\Str;

class Mix
{
    protected static array $manifests = [];

    public function __invoke(string $path, string $package): HtmlString|string
    {
        if (!$package) {
            throw new InvalidArgumentException('A package must be passed');
        }

        // Normalise the package name
        $package = strtolower($package);

        if (!($compilableAssetPackage = PackageManager::instance()->getPackages('mix')[$package] ?? null)) {
            throw new SystemException('Unable to resolve package: ' . $package);
        }

        $manifestPath = public_path($compilableAssetPackage['path'] . '/mix-manifest.json');

        if (!isset(static::$manifests[$manifestPath])) {
            if (!is_file($manifestPath)) {
                throw new Exception("The Mix manifest does not exist.");
            }

            static::$manifests[$manifestPath] = json_decode(file_get_contents($manifestPath), true);
        }

        $manifest = static::$manifests[$manifestPath];

        $path = Str::start($path, '/');

        if (!isset($manifest[$path])) {
            $exception = new Exception("Unable to locate Mix file: $path");

            if (!app('config')->get('app.debug')) {
                report($exception);

                return $path;
            } else {
                throw $exception;
            }
        }

        return new HtmlString(Url::asset($compilableAssetPackage['path'] . $manifest[$path]));
    }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericp-mrel any thoughts on the above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukeTowers Did you see my comment / code about re-writing this to work similar to the Vite class?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaxwilko do you have any input on the above?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericp-mrel looked at your suggestion and I like it, I would leave the mix filter being registered by the CMS module however so that the default value of the package argument could be the current theme. Then if anyone wants to use it in plugins they can just use the Mix class directly (or if you see value in it add a addMix() method like we have addVite() in the AssetMaker trait).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the approach I'd go with (in terms of replicating the vite() method functionality), however I'd probably try and keep the signatures the same (add support for array input with different file types) and return the entire tag i.e.

Twig:

{{ mix(['path/to/example.js', 'path/to/example.css'], 'theme-example') }}

Rendered output:

<link rel="modulepreload" href="https://example.com/path/to/example-hash.js">
<script type="module" src="https://example.com/path/to/example-hash.js"></script>
<link rel="preload" as="style" href="https://example.com/path/to/example-hash.css">
<link rel="stylesheet" href="https://example.com/path/to/example-hash.css">

This will allow people to swap between vite and mix easier in the future and makes the documentation simplier as they will both function in the same way.

It's probably worth adding support for passing the manifest file as an extra param on the tag, as the file doesn't necessarily always have the same name (see: #1167) i.e.

{{ mix(['path/to/example.js', 'path/to/example.css'], 'theme-example') }}
// vs
{{ mix(['path/to/example.js', 'path/to/example.css'], 'theme-example', 'my-custom-manifest.json') }}
// or
{{ mix(['path/to/example.js', 'path/to/example.css'], 'theme-example', 'path/to/my-custom-manifest.json') }}

We can assume the default, but if a custom manifest name/path is passed it will allow for user overrides.


$path = Str::start($path, '/');

if ($manifestDirectory && ! str_starts_with($manifestDirectory, '/')) {
$manifestDirectory = "/$manifestDirectory";
} else {
$manifestDirectory = Str::start($theme->getConfigValue('mix_manifest_path', '/'), '/');
}

$manifestPath = $theme->getPath($theme->getDirName() . '/' . $manifestDirectory . '/mix-manifest.json');

if (! isset($manifests[$manifestPath])) {
if (! is_file($manifestPath)) {
throw new \Exception('The Mix manifest does not exist.');
ericp-mrel marked this conversation as resolved.
Show resolved Hide resolved
}

$manifests[$manifestPath] = json_decode(file_get_contents($manifestPath), true);
}

$manifest = $manifests[$manifestPath];

if (! isset($manifest[$path])) {
ericp-mrel marked this conversation as resolved.
Show resolved Hide resolved
$exception = new \Exception("Unable to locate Mix file: $path");

if (! app('config')->get('app.debug')) {
report($exception);

return $path;
} else {
throw $exception;
}
}

$url = Config::get('cms.themesPath', '/themes') . '/' . $theme->getDirName() . $manifest[$path];

return new HtmlString(Url::asset($url));
}
}
161 changes: 161 additions & 0 deletions modules/system/tests/classes/asset/MixTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
<?php

namespace System\Tests\Classes\Asset;

use Cms\Classes\Theme;
use System\Classes\Asset\Mix;
use System\Tests\Bootstrap\TestCase;
use Winter\Storm\Support\Facades\Config;
use Winter\Storm\Support\Facades\Event;
use Winter\Storm\Support\Facades\File;
use Winter\Storm\Support\Facades\Url;

class MixTest extends TestCase
{
protected string $themePath;

protected string $originalThemesPath = '';

protected string $originalThemesPathLocal = '';

protected function setUp(): void
{
parent::setUp();

if (!is_dir(base_path('node_modules'))) {
$this->markTestSkipped('This test requires node_modules to be installed');
}

if (!is_file(base_path('node_modules/.bin/mix'))) {
$this->markTestSkipped('This test requires the mix package to be installed');
}

$this->originalThemesPath = Config::get('cms.themesPath');
Config::set('cms.themesPath', '/modules/system/tests/fixtures/themes');

$this->originalThemesPathLocal = Config::get('cms.themesPathLocal');
Config::set('cms.themesPathLocal', base_path('modules/system/tests/fixtures/themes'));
$this->app->setThemesPath(Config::get('cms.themesPathLocal'));

$this->themePath = base_path('modules/system/tests/fixtures/themes/mixtest');

Config::set('cms.activeTheme', 'mixtest');

Event::flush('cms.theme.getActiveTheme');
Theme::resetCache();
}

protected function tearDown(): void
{
File::deleteDirectory('modules/system/tests/fixtures/themes/mixtest/assets/dist');
File::delete('modules/system/tests/fixtures/themes/mixtest/mix-manifest.json');

Config::set('cms.themesPath', $this->originalThemesPath);

Config::set('cms.themesPathLocal', $this->originalThemesPathLocal);
$this->app->setThemesPath($this->originalThemesPathLocal);

parent::tearDown();
}

public function testThrowsExceptionWhenMixManifestIsMissing(): void
{
$this->expectException(\Exception::class);
$this->expectExceptionMessage('The Mix manifest does not exist');

Mix::mix('assets/dist/foo.css');
}

public function testGeneratesAssetUrls(): void
{
$this->artisan('mix:compile', [
'theme-mixtest',
'--manifest' => 'modules/system/tests/fixtures/npm/package-mixtest.json',
'--disable-tty' => true,
])->assertExitCode(0);

$theme = Theme::getActiveTheme();

$this->assertFileExists($theme->getPath($theme->getDirName() . '/mix-manifest.json'));

$manifest = json_decode(file_get_contents($theme->getPath($theme->getDirName() . '/mix-manifest.json')), true);

foreach ($manifest as $key => $value) {
$mixAssetUrl = Mix::mix($key);

$this->assertStringStartsWith(
Url::asset(Config::get('cms.themesPath', '/themes') . '/' . $theme->getDirName()),
$mixAssetUrl
);
}
}

public function testThemeCanOverrideMixManifestPath(): void
{
$theme = Theme::getActiveTheme();

Event::listen('cms.theme.extendConfig', function ($dirName, &$config) {
$config['mix_manifest_path'] = 'assets/dist';
});

rename(
$theme->getPath($theme->getDirName() . '/winter.mix.js'),
$theme->getPath($theme->getDirName() . '/winter.mix.js.bak')
);

copy(
$theme->getPath($theme->getDirName() . '/winter.mix-manifest-override.js'),
$theme->getPath($theme->getDirName() . '/winter.mix.js')
);

try {
$this->artisan('mix:compile', [
'theme-mixtest',
'--manifest' => 'modules/system/tests/fixtures/npm/package-mixtest.json',
'--disable-tty' => true,
])->assertExitCode(0);

$this->assertFileExists($theme->getPath($theme->getDirName() . '/assets/dist/mix-manifest.json'));

$manifest = json_decode(file_get_contents($theme->getPath($theme->getDirName() . '/assets/dist/mix-manifest.json')), true);

foreach ($manifest as $key => $value) {
$this->assertStringContainsString($key, (string) Mix::mix($key));
}
} catch (\Exception $e) {
throw $e;
} finally {
rename(
$theme->getPath($theme->getDirName() . '/winter.mix.js.bak'),
$theme->getPath($theme->getDirName() . '/winter.mix.js')
);
}
}

public function testThrowsAnExceptionForInvalidMixFileWhenDebugIsEnabled()
{
$this->expectException(\Exception::class);
$this->expectExceptionMessage('Unable to locate Mix file: /assets/dist/foo.css');

$this->artisan('mix:compile', [
'theme-mixtest',
'--manifest' => 'modules/system/tests/fixtures/npm/package-mixtest.json',
'--disable-tty' => true,
])->assertExitCode(0);

Mix::mix('assets/dist/foo.css');
}

public function testDoesNotThrowAnExceptionForInvalidMixFileWhenDebugIsDisabled(): void
{
Config::set('app.debug', false);

$this->artisan('mix:compile', [
'theme-mixtest',
'--manifest' => 'modules/system/tests/fixtures/npm/package-mixtest.json',
'--disable-tty' => true,
])->assertExitCode(0);

$this->assertEquals('/assets/dist/foo.css', Mix::mix('assets/dist/foo.css'));
}
}
11 changes: 11 additions & 0 deletions modules/system/tests/fixtures/npm/package-mixtest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"type": "module",
"workspaces": {
"packages": [
"modules/system/tests/fixtures/themes/mixtest"
]
},
"devDependencies": {
"laravel-mix": "^6.0.41"
}
}
3 changes: 3 additions & 0 deletions modules/system/tests/fixtures/themes/mixtest/assets/src/css/theme.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
h1 {
color: red;
}
1 change: 1 addition & 0 deletions modules/system/tests/fixtures/themes/mixtest/assets/src/js/theme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
window.alert('hello world');
1 change: 1 addition & 0 deletions modules/system/tests/fixtures/themes/mixtest/theme.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
name: 'Mix Test'
13 changes: 13 additions & 0 deletions modules/system/tests/fixtures/themes/mixtest/winter.mix-manifest-override.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const mix = require('laravel-mix');

mix.setPublicPath(__dirname)
.options({
manifest: 'assets/dist/mix-manifest.json',
})
.version()

// Render Tailwind style
.postCss('assets/src/css/theme.css', 'assets/dist/css/theme.css')

// Compile JS
.js('assets/src/js/theme.js', 'assets/dist/js/theme.js');
13 changes: 13 additions & 0 deletions modules/system/tests/fixtures/themes/mixtest/winter.mix.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
const mix = require('laravel-mix');

mix.setPublicPath(__dirname)
.options({
manifest: true,
})
.version()

// Render Tailwind style
.postCss('assets/src/css/theme.css', 'assets/dist/css/theme.css')

// Compile JS
.js('assets/src/js/theme.js', 'assets/dist/js/theme.js');
Loading