-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
refactor: use afterResolving
instead of Blade facade
#97
Conversation
…gister the directives after resolving `blade.compiler`
I'm kind of conflicted with that PR. First, the tests don't pass (but I think it's just a typing issue). I also have zero experience with Lumen, and I'm pretty sure it is slowly dying, so I don't really want to support it. It seems that you would be able to make it work with Lumen by replacing the That being said, the changes are not that much, so I think it's okay to merge if you fix the code so the tests pass. |
Hi @innocenzi, I know that Lumen is dying, but I think that installing Laravel just for an API is a bit over-power. Octane is amazing, but it seems like it doesn't work with Apache, so it cuts out a lot of applications made with it. Yeah, I think that it may bring even some optimizations for Laravel, so I'm glad that my PR will be accepted! |
src/ViteServiceProvider.php
Outdated
if (! $entryName) { | ||
return '<?php echo vite_tags() ?>'; | ||
} | ||
$this->app->afterResolving('blade.compiler', function ($compiler) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you type-hint $compiler
? You removed it in abd0100
but all was needed was to import it.
afterResolving
instead of Blade facade
afterResolving can't register in bootingPackages. related issue |
This reverts commit 19468b7.
I tried the package with Lumen and after configuring everything, I got this error:
By googling, I found a link to this PR on laravel/framework and looking to the last commit and tried to include the
$this-app->afterResolving()
approach, that worked.By the same googling session, I found a lot of suggestions about the use of
ServiceProvider::boot()
instead ofServiceProvider::register()
when declaring singletons and Blade directives, so I moved replaced theregisteringPackage()
method withbootingPackage()
.