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.

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

All attributes starting with"@" are removed after minification. #12

Closed
SaeedHeydari opened this issue Mar 5, 2024 · 6 comments · Fixed by #13 or #17
Closed

All attributes starting with"@" are removed after minification. #12

SaeedHeydari opened this issue Mar 5, 2024 · 6 comments · Fixed by #13 or #17
Labels
bug Something isn't working

Comments

@SaeedHeydari
Copy link

SaeedHeydari commented Mar 5, 2024

The following Alipine code:
<button @click="alert('test')">Test</button>

After minification becomes:
<button>Test</button>
This applies to all attributes starting with @

We have to use its equivalent to be able to have it:
<button x-on:click="alert('test')">Test</button>

"@" is shortened version of "x-on"

@SaeedHeydari SaeedHeydari changed the title In Alpine, all attributes with "@" syntax are removed after minification. In Alpine, all attributes starting with"@" are removed after minification. Mar 5, 2024
@SaeedHeydari SaeedHeydari changed the title In Alpine, all attributes starting with"@" are removed after minification. All attributes starting with"@" are removed after minification. Mar 5, 2024
@fahlisaputra fahlisaputra linked a pull request Mar 7, 2024 that will close this issue
fahlisaputra added a commit that referenced this issue Mar 7, 2024
fix : (#12) directives auto replacement
@fahlisaputra
Copy link
Owner

fahlisaputra commented Mar 7, 2024

Hi @SaeedHeydari, thank you for reporting the issue.

The problem is from this code.

protected function loadDom(string $html, bool $force = false)
{
    .....

    static::$dom = new DOMDocument();
    @static::$dom->loadHTML($html, LIBXML_HTML_NODEFDTD | LIBXML_SCHEMA_CREATE);
}

Method loadHTML from class DOMDocument automatically clear unnecessary characters. See: https://www.php.net/manual/en/domdocument.loadhtml.php. There is no option for this case.

I took the approach to replace the directives '@' with x-on. But, this solution may break on another JS framework. So, I added new config that devs can change what characters to change.

Please update your laravel-minify dependency to 1.1.3, run:
composer update fahlisaputra/laravel-minify

After that, please add this line at the bottom in file config/minify.php

  /*
  |--------------------------------------------------------------------------
  | Custom Directives Replacement
  |--------------------------------------------------------------------------
  |
  | Here you can specify the directives that you want to replace. For example,
  | you can replace the @ symbol with x-on: for AlpineJS with '@' => 'x-on:'.
  | Minify use preg_replace to replace the directives.
  |
  */
  
  'directives' => [
      "@" => "x-on:",
  ],

image

And clear the config cache, run:
php artisan optimize

If you encounter any issue, please report it to me. Thanks!

@fahlisaputra fahlisaputra reopened this Mar 7, 2024
@SaeedHeydari
Copy link
Author

SaeedHeydari commented Mar 9, 2024

Thank you @fahlisaputra
But in code below:
image
You have used @preg_match required a valid regex. but in config the '@' has been used. so it doesn't work.

I changed the above code and it works. I think you should change code to this:
image

Also there is a mistake in the README line below:
"You can replace it by adding @ => x-click: to the directives array. For example:"
This is correct:
"You can replace it by adding @ => x-on: to the directives array. For example:"

@SaeedHeydari
Copy link
Author

SaeedHeydari commented Mar 9, 2024

I see another problem:
The Vite use scripts like this in the development
<script type="module" src="http://%5B::1%5D:5173/@vite/client"></script>
That contains @vite! so it becomes: x-on:vite and throws error.

I solved it by adding new config as:
"keep_directives" => [ '@vite' ],

and code below in Minifier:

       $keepDirectivesKeys = config('minify.keep_directives', []);

        // make $keepDirectives array: keys = keep_directives values, values = '____'. uniqid(). '____'
        foreach ($keepDirectivesKeys as $key) {
            $keepDirectives[$key] = '____'. uniqid(). '____';
        }

        // replace directive must be kept with unique strings
        foreach ($keepDirectives as $search => $replace) {
            $html = str_replace($search, $replace, $html);
        }

        // replace custom directives, issue #12
        $directives = config('minify.directives', []);
        foreach ($directives as $search => $replace) {
            $html = str_replace($search, $replace, $html);
        }

        // replace unique strings with directives must be kept
        foreach ($keepDirectives as $replace => $search) {
            $html = str_replace($search, $replace, $html);
        }

At first I replaced all words in keep_directives array to a unique names, then replaced all '@' directives to 'x-on:' then replaced unique names with original keep_directives values. by this way words like @vite will be kept.

This was my solution, maybe you have a better solution.

@fahlisaputra
Copy link
Owner

fahlisaputra commented Mar 13, 2024

Have you tested it? When I try to use str_replace directly, it causes the page layout to change.

Before:
image

After:
image

or maybe we may add new config that devs can choose to use str_replace or preg_replace?

@fahlisaputra
Copy link
Owner

Sorry, in the previous commit it turns out I made a mistake when testing this feature. Fixed soon!

@fahlisaputra fahlisaputra added the bug Something isn't working label Mar 13, 2024
@fahlisaputra fahlisaputra linked a pull request Mar 13, 2024 that will close this issue
@fahlisaputra
Copy link
Owner

Hi, I added new function to process the custom directives

protected function replaceDirectives($html) : string {
    if (!config('minify.enable_directive_replacement', false)) {
        return $html;
    }

    // split the html into head and body
    $body = explode('<body', $html);

    // mask the directive that want to keep, solution by @SaeedHeydari #12
    $keepDirectivesKeys = config('minify.keep_directives', []);
    $keepDirectives = [];
    foreach ($keepDirectivesKeys as $key) {
        $keepDirectives[$key] = '____'. uniqid(). '____';
        $body[1] = str_replace($key, $keepDirectives[$key], $body[1]);
    }

    // replace custom directives, issue #12
    $directives = config('minify.directives', []);
    foreach ($directives as $search => $replace) {
        $body[1] = str_replace($search, $replace, $body[1]);
    }

    // unmask the directive that want to keep
    foreach ($keepDirectives as $replace => $search) {
        $body[1] = str_replace($search, $replace, $body[1]);
    }

    // rejoin the html
    $html = $body[0]. '<body'. $body[1];

    return $html;
}

I was forced to break the html into two parts so as not to affect anything in the <head> section. This version runs fine on my local laptop. I will merge this version tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants