Fix: Livewire component premature registration #1390
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
REF: livewire/livewire#7076 (comment) In the latest release of Livewire, Laravel Service Container is utilized for Dependency Resolution, which after testing with a new installation of Jetstream found to have a premature component registration due to a race condition. **Explaination:** - Previously, mechanisms were created using standard PHP object creation `(new $mechanism)`, not involving Laravel's service container, hence no service container events were triggered. `LivewireServiceProvider::registerMechanisms()` - With the recent change, these mechanisms are now instantiated via Laravel's IoC service container `app($mechanism)`, causing service container events to be triggered. - 💡 **The core of the problem** arises when the `Livewire\Mechanisms\CompileLivewireTags` mechanism is instantiated. It extends `Illuminate\View\Compilers\ComponentTagCompiler` which has a constructor dependency of `Illuminate\View\Compilers\BladeCompiler` , and its creation via the service container triggers events. On the other hand, **the `JetstreamServiceProvider` prematurely listens to this event in the service provider `register()` method**, leading to a situation where Livewire components are being registered before all necessary mechanisms are set up, particularly the `Livewire\Mechanisms\ComponentRegistry` which comes next in order after `Livewire\Mechanisms\CompileLivewireTags`. **Suggested fix:** - Move the registration of Livewire components in `JetstreamServiceProvider` to the `boot()` method, which is where it should be. This ensures all mechanisms are in place before any component registration begins. It also makes the additional event handling for BladeCompiler resolution unnecessary. `$this->callAfterResolving(BladeCompiler::class, fn () => '');`
|
I reverted the change in Livewire's core (introduced in v3.0.9 and reverted in v3.0.10) to avoid breaking Jetstream. However, if it's no difference, it'd be nice to make this change in Jetstream I suppose so that Livewire can take advantage of the service container for registering its own internals. |
|
Shouldn't Jetstream listen to resolving Livewire's |
Omranic
added a commit
to Omranic/livewire
that referenced
this pull request
Oct 29, 2023
…iceProvider Livewire Reference: livewire#7076 Jetstream Reference: laravel/jetstream#1390 The required update in `laravel/jetstream` has been merged into core, and tagged in [v4.0.4](https://github.com/laravel/jetstream/releases/tag/v4.0.4) ----- This pull request updates the LivewireServiceProvider class to utilize Laravel's container for resolving dependencies and instantiating objects, improving modularity and flexibility for app developers. With this change, developers can easily override core Livewire functionalities in their service providers as needed, such as Mechanisms and Features. These changes have been made with the aim of maintaining backward compatibility and ensuring that there are no side effects or incompatibilities. ----- Code snippet of the improvement and reasons why it's useful: ```php namespace App; use Illuminate\Support\ServiceProvider; class AppServiceProvider extends ServiceProvider { public function register() { $this->app->singleton(\Livewire\Mechanisms\HandleRequests\HandleRequests::class, \App\HandleRequests::class); } } ``` Example of a `HandleRequests` Mechanism override, changing the mechanism boot logic. This is just an example, but there's many other use cases. ```php namespace App; use Livewire\Mechanisms\HandleRequests\HandleRequests as BaseHandleRequests; class HandleRequests extends BaseHandleRequests { function boot() { // ... } } ``` Thanks for your consideration! 🙌
calebporzio
pushed a commit
to livewire/livewire
that referenced
this pull request
Nov 5, 2023
…iceProvider (#7181) Livewire Reference: #7076 Jetstream Reference: laravel/jetstream#1390 The required update in `laravel/jetstream` has been merged into core, and tagged in [v4.0.4](https://github.com/laravel/jetstream/releases/tag/v4.0.4) ----- This pull request updates the LivewireServiceProvider class to utilize Laravel's container for resolving dependencies and instantiating objects, improving modularity and flexibility for app developers. With this change, developers can easily override core Livewire functionalities in their service providers as needed, such as Mechanisms and Features. These changes have been made with the aim of maintaining backward compatibility and ensuring that there are no side effects or incompatibilities. ----- Code snippet of the improvement and reasons why it's useful: ```php namespace App; use Illuminate\Support\ServiceProvider; class AppServiceProvider extends ServiceProvider { public function register() { $this->app->singleton(\Livewire\Mechanisms\HandleRequests\HandleRequests::class, \App\HandleRequests::class); } } ``` Example of a `HandleRequests` Mechanism override, changing the mechanism boot logic. This is just an example, but there's many other use cases. ```php namespace App; use Livewire\Mechanisms\HandleRequests\HandleRequests as BaseHandleRequests; class HandleRequests extends BaseHandleRequests { function boot() { // ... } } ``` Thanks for your consideration! 🙌
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
REF: livewire/livewire#7076 (comment)
REF: livewire/livewire#7093
In the latest release of Livewire v3.0.9, Laravel Service Container is utilized for Dependency Resolution, which after testing with a new installation of Jetstream found to have a premature component registration due to a race condition, resulting in
Unable to find component: [navigation-menu]exceptions.Explaination:
(new $mechanism), not involving Laravel's service container, hence no service container events were triggered.LivewireServiceProvider::registerMechanisms()app($mechanism), causing service container events to be triggered.Livewire\Mechanisms\CompileLivewireTagsmechanism is instantiated. It extendsIlluminate\View\Compilers\ComponentTagCompilerwhich has a constructor dependency ofIlluminate\View\Compilers\BladeCompiler, and its creation via the service container triggers events. On the other hand, theJetstreamServiceProviderprematurely listens to this event in the service providerregister()method, leading to a situation where Livewire components are being registered before all necessary mechanisms are set up, particularly theLivewire\Mechanisms\ComponentRegistrywhich comes next in order afterLivewire\Mechanisms\CompileLivewireTags.Suggested fix:
JetstreamServiceProviderto theboot()method, which is where it should be. This ensures all mechanisms are in place before any component registration begins. It also makes the additional event handling for BladeCompiler resolution unnecessary.$this->callAfterResolving(BladeCompiler::class, fn () => '');