jeroennoten / Laravel-AdminLTE

Easy AdminLTE integration with Laravel
MIT License
3.82k stars 1.08k forks source link

Add suport to Vite #1308

Closed ruanpepe closed 6 hours ago

ruanpepe commented 2 weeks ago
Question Answer
Pull request type ENHANCEMENT
License MIT

What's in this PR?

In config: I changed the Laravel Mix config lines to a new patter that can be used with Vite In that new way, the user can choose between 'mix', 'vite' and false.

In master.blade.php: I included a switch to include the css and js files according to the config. It is checking if the user is using the old mix config, so it is not a breaking change.

In components I included a check to prevent scripts from running before jQuery loading finish.

The wiki must be updated https://github.com/jeroennoten/Laravel-AdminLTE/wiki/Menu-Configuration

My tests included

Further tests are recommended

Checklist

ruanpepe commented 2 weeks ago

Related issue https://github.com/jeroennoten/Laravel-AdminLTE/issues/1155

dfsmania commented 2 weeks ago

Thanks for your contribution @ruanpepe , give me some time to review this, could you provide further information about next changes:

In components I included a check to prevent scripts from running before jQuery loading finish.

My doubt is why the already included shorthand check for $( document ).ready() of JQuery won't work for the VITE support you included.

ruanpepe commented 2 weeks ago

Thanks for your contribution @ruanpepe , give me some time to review this, could you provide further information about next changes:

In components I included a check to prevent scripts from running before jQuery loading finish.

My doubt is why the already included shorthand check for $( document ).ready() of JQuery won't work for the VITE support you included.

jQuery is included in my app.js and Vite loads the js files asynchronously. I have tried the $( document ).ready() but i had a lot of work solving "$ is not a function". I placed some logs on my app.js and in the page.blade.php js section. The Vite tag was the first in the code, but the log in the app.js was the last in the console. The components were running before Vite finish loading my app.js. I noticed that after see my app.js log on console, the $ could be normally called.

dfsmania commented 1 week ago

Thanks for your contribution @ruanpepe , give me some time to review this, could you provide further information about next changes:

In components I included a check to prevent scripts from running before jQuery loading finish.

My doubt is why the already included shorthand check for $( document ).ready() of JQuery won't work for the VITE support you included.

jQuery is included in my app.js and Vite loads the js files asynchronously. I have tried the $( document ).ready() but i had a lot of work solving "$ is not a function". I placed some logs on my app.js and in the page.blade.php js section. The Vite tag was the first in the code, but the log in the app.js was the last in the console. The components were running before Vite finish loading my app.js. I noticed that after see my app.js log on console, the $ could be normally called.

Oh, I see. I have read next link about the problem you faced:

https://laracasts.com/discuss/channels/vite/laravel-vite-jquery

Can you try the fix proposed by user: hassan, it requires to add type="module" to the script tags instead of wrapping all the code in document.addEventListener('DOMContentLoaded', ...);. If this workaround works, I would prefer it since it requires less changes on the base code of the componets...

ruanpepe commented 1 week ago

Thanks for your contribution @ruanpepe , give me some time to review this, could you provide further information about next changes:

In components I included a check to prevent scripts from running before jQuery loading finish.

My doubt is why the already included shorthand check for $( document ).ready() of JQuery won't work for the VITE support you included.

jQuery is included in my app.js and Vite loads the js files asynchronously. I have tried the $( document ).ready() but i had a lot of work solving "$ is not a function". I placed some logs on my app.js and in the page.blade.php js section. The Vite tag was the first in the code, but the log in the app.js was the last in the console. The components were running before Vite finish loading my app.js. I noticed that after see my app.js log on console, the $ could be normally called.

Oh, I see. I have read next link about the problem you faced:

https://laracasts.com/discuss/channels/vite/laravel-vite-jquery

Can you try the fix proposed by user: hassan, it requires to add type="module" to the script tags instead of wrapping all the code in document.addEventListener('DOMContentLoaded', ...);. If this workaround works, I would prefer it since it requires less changes on the base code of the componets...

Using 'vite' and 'mix', it woks. Using 'vite_spa', the css is being loaded after js. Because it, the raw content of page becomes visible for a short moment. I just removed this option from that code i wrote.

dfsmania commented 1 week ago

@ruanpepe OK, the problem you faced with your new vite_spa option is called flash of unstyled content. I will check if there's a fix proposed for this problem. On the other hand, could you rollback again the indent of the master.blade.php file.

In the next days, I will give these changes a test on some of my projects, just to check everything still works. Note I do not use Mix or Vite, so testing with Mix and Vite is up to you.

Finally, thanks for all your work here!

ruanpepe commented 1 week ago

@ruanpepe OK, the problem you faced with your new vite_spa option is called flash of unstyled content. I will check if there's a fix proposed for this problem. On the other hand, could you rollback again the indent of the master.blade.php file.

In the next days, I will give these changes a test on some of my projects, just to check everything still works. Note I do not use Mix or Vite, so testing with Mix and Vite is up to you.

Finally, thanks for all your work here!

Great! I fixed the ident. My IDE does it. Sorry! Now, knowing what is the CSS issue, i can get better results looking for a solution.

It's a great pleasure contribute to this project i use for so long and for free. And it's a great experience to be in touch with another dev in another language. Thank you.

ruanpepe commented 1 week ago

@ruanpepe OK, the problem you faced with your new vite_spa option is called flash of unstyled content. I will check if there's a fix proposed for this problem. On the other hand, could you rollback again the indent of the master.blade.php file.

In the next days, I will give these changes a test on some of my projects, just to check everything still works. Note I do not use Mix or Vite, so testing with Mix and Vite is up to you.

Finally, thanks for all your work here!

I did some research about flashing unstyled content in Vite, and after some testing using 'vite_spa' I noticed that this only happens in dev mode (npm run dev). In prod (npm run build) it is not an issue.

dfsmania commented 1 week ago

@ruanpepe Good, you can add that option again if you like, and then we may add a warning on the Wiki for the flash of unstyled content issue when working in dev mode. However I'm not sure if vite_spa is the better name for it, since SPA means Single Page Application, so maybe vite_js_only is more meaningful. Laravel says next for this option:

If you are building an SPA, including applications built using Inertia, Vite works best without CSS entry points. Instead, you should import your CSS via JavaScript. Typically, this would be done in your application's resources/js/app.js.

ruanpepe commented 1 week ago

Nice! I'm goig to do it right now.

dfsmania commented 1 week ago

@ruanpepe I reverted some style fixes added by your editor, updated the config docs, and joined vite and vite_js_only into the same case block for the scripts section. So, I will give a test to these changes locally in the next days. Please, check I didn't break anything, just in case.

ruanpepe commented 1 week ago

Everything is ok

dfsmania commented 1 week ago

@ruanpepe Great, I have update Wiki docs to address these changes, please review it, any suggestion would be accepted.

https://github.com/jeroennoten/Laravel-AdminLTE/wiki/Other-Configuration

ruanpepe commented 5 days ago

@ruanpepe Great, I have update Wiki docs to address these changes, please review it, any suggestion would be accepted.

https://github.com/jeroennoten/Laravel-AdminLTE/wiki/Other-Configuration

There are two Laravel Mix sections (1, 2). I think the second one would be Laravel Vite, right? Other than that, everything is very clear.

dfsmania commented 5 days ago

@ruanpepe Great, I have update Wiki docs to address these changes, please review it, any suggestion would be accepted. https://github.com/jeroennoten/Laravel-AdminLTE/wiki/Other-Configuration

There are two Laravel Mix sections (1, 2). I think the second one would be Laravel Vite, right? Other than that, everything is very clear.

Thanks for pointing this! Fixed now!