winter icon indicating copy to clipboard operation
winter copied to clipboard

Adds a `mix` Twig filter for Laravel Mix theme assets.

Open ericp-mrel opened this issue 1 year ago • 10 comments

This PR adds support for a Laravel mix Twig mix filter.

ericp-mrel avatar Aug 04 '24 02:08 ericp-mrel

@ericp-mrel are you able to add tests for this and a docs PR?

LukeTowers avatar Aug 04 '24 05:08 LukeTowers

@LukeTowers I'm trying to create some tests, but I'm running into an issue where I cannot override the base path that's used by the $theme->getPath() function. I've tried overriding all of the config settings in the setup() function, but this doesn't seem to have any effect on anything that relies on the themes_path() helper function.

Do you have any tips on how I can get this to work?

<?php

namespace System\Tests\Classes\Asset;

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

class MixTest extends TestCase
{
    protected string $themePath;

    protected string $originalThemesPath = '';

    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->themePath = base_path('modules/system/tests/fixtures/themes/mixtest');

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

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

        PackageManager::instance()->registerPackage(
            'theme-mixtest',
            $this->themePath . '/winter.mix.js'
        );
    }

    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);

        parent::tearDown();
    }

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

        $theme = Theme::getActiveTheme();

        dd($theme->getPath()); // This keeps pointing to /modules/cms instead of /modules/system
    }

    public function testThemeCanOverrideMixManifestPath(): void
    {

    }
}

ericp-mrel avatar Aug 17 '24 21:08 ericp-mrel

@ericp-mrel this might be useful to you: https://github.com/wintercms/winter/commit/d27dbbc5f8bb4b2df24c882b9d27dab521213294. There's two separate config keys, cms.themesPath and cms.themesPathLocal.

LukeTowers avatar Aug 18 '24 05:08 LukeTowers

@LukeTowers That didn't seem to make any difference, after poking around more in the container classes I found out there was a function called setThemesPath() that was being run as part of the bootstrap process. So, I had to add the following line to get the themes path to resolve correctly for the tests.

$this->app->setThemesPath(Config::get('cms.themesPathLocal'));

I'm not sure if there's a better way to handle setting that value in the Container?

Full File:

<?php

namespace System\Tests\Classes\Asset;

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

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);

        parent::tearDown();
    }

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

        $theme = Theme::getActiveTheme();

        dd($this->app->make('path.themes'), $theme->getPath(), $theme->assetUrl('assets/dist/css/theme.css'), (string) Mix::mix('assets/dist/css/theme.css'));
    }

    public function testThemeCanOverrideMixManifestPath(): void
    {

    }
}

ericp-mrel avatar Aug 18 '24 23:08 ericp-mrel

@bennothommo or @jaxwilko might have some input here

LukeTowers avatar Aug 19 '24 07:08 LukeTowers

Hi @ericp-mrel, I've just set up a dummy test which might help you.

I created a theme fixture as modules/system/tests/fixtures/themes/mixtest: image

The mix file for this is:

const mix = require('laravel-mix');

mix.setPublicPath(__dirname);

mix
    .css('assets/css/theme.css', 'assets/dist/theme.css')
    .js('assets/javascript/theme.js', 'assets/dist/theme.js')
    .options({
        manifest: true,
    });

Then a custom manifest specifically for this test as modules/system/tests/fixtures/npm/package-mixtheme.json:

{
    "workspaces": {
        "packages": [
            "modules/system/tests/fixtures/themes/mixtest"
        ]
    },
    "devDependencies": {
        "laravel-mix": "^6.0.41"
    }
}

Then I added the following to MixCompileTest::__constructor():

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

// Add our testing theme because it won't be auto discovered
\System\Classes\Asset\PackageManager::instance()->registerPackage(
    'theme-mixtest',
    $this->themePath . '/winter.mix.js',
);

Then finally the test code:

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

    $manifestPath = $this->themePath . '/mix-manifest.json';

    // Validate the file exists
    $this->assertFileExists($manifestPath);

    // Check the contents of the file
    $contents = File::get($manifestPath);
    $this->assertStringContainsString('/assets/dist/theme.css', $contents);

    // Clean up
    File::deleteDirectory($this->themePath . '/assets/dist');
    File::delete($manifestPath);
}

If you have any questions feel free to @ me :)

jaxwilko avatar Aug 19 '24 12:08 jaxwilko

@ericp-mrel what's left on this?

LukeTowers avatar Nov 13 '24 22:11 LukeTowers

@LukeTowers Just waiting on a response to my question from here:

Screenshot 2024-11-13 at 5 18 49 PM

ericp-mrel avatar Nov 13 '24 23:11 ericp-mrel

@ericp-mrel I can't see that response until you submit your review, that's what the "Pending" status flag next to it means 😄

LukeTowers avatar Nov 14 '24 04:11 LukeTowers

@ericp-mrel it's looking good to me, can you do up a PR to the docs for this?

LukeTowers avatar Apr 04 '25 17:04 LukeTowers