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

load blade directives after resolving blade compiler #11

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

tabacitu
Copy link
Member

Tries to fix the issue reported in Laravel-Backpack/CRUD#4168 where the way we load blade directives stops OTHER packages from properly loading THEIR blade directives.

Inspiration for this PR came from:

Note to self - it's possible that in packages that's the "proper" way to load Blade directives (not with Blade::directive() as you would do in a Laravel app). Otherwise if any package uses this method and is loaded AFTER our package, they won't have their directives loaded.

@tabacitu
Copy link
Member Author

To use this branch, please do

composer require digitallyhappy/assets:"dev-load-blade-directives-after-resolving-blade-compiler as 2.0.99"
php artisan view:clear

@engel-m
Copy link

engel-m commented Feb 10, 2022

This PR fixed my issues with Laravel Impersonate and Spatie Permissions blade directives. All seems to be working fine.

@tabacitu
Copy link
Member Author

Exceleeeent! Thank you @engel-m . I'll wait for @pxpm to also confirm, and merge this. In the meantime, you should be able to use this branch, no problem.

@tabacitu tabacitu requested a review from pxpm February 10, 2022 09:14
@tabacitu tabacitu added the bug Something isn't working label Feb 10, 2022
Copy link
Contributor

@pxpm pxpm left a comment

Choose a reason for hiding this comment

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

I guess this is something related with the order of the providers or something, the directives work for me with or without this fix. (the spatie ones)

I did some search around too and came to the same conclusion as you, also found an issue on spatie/permission directly related to this spatie/laravel-permission#278

I think the PR is clean, and it works indeed, I was just wondering if developer added some custom Blade::directive in their provider it would break ours. I tested and it does not break, I can't be happy with this and installed the impersonate package to see what was happening.

It happens that without this branch impersonate does not work, but spatie keeps working. With this branch both work ... so yeah. I will leave it here.

@tabacitu tabacitu merged commit a323b8f into main Feb 10, 2022
@tabacitu tabacitu deleted the load-blade-directives-after-resolving-blade-compiler branch February 10, 2022 13:42
@tabacitu
Copy link
Member Author

Awesome! Merged & tagged 🎉
This was a weird one...

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
Development

Successfully merging this pull request may close these issues.

3 participants