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

Allows multiple calls to Volt::mount() #88

Closed
wants to merge 4 commits into from

Conversation

inmanturbo
Copy link

See #87

With this PR all three of the following examples yield the same result:

  /**
   * Bootstrap services.
   */
  public function boot(): void
  {
      Volt::mount(
          config('livewire.view_path', resource_path('views/livewire')),
      );

      Volt::mount(
          resource_path('views/pages'),
      );
  }
Volt::mount([
    config('livewire.view_path', resource_path('views/livewire')),
    resource_path('views/pages'),
]);
Volt::mount([
    config('livewire.view_path', resource_path('views/livewire')),
]);

Volt::mount([
    resource_path('views/pages'),
]);

This means that packages can mount their own components, and developers can use them without having to include them in their VoltServiceProvider.

Laravel is known for "Batteries Included" packages which require very little setup to get started!

Some points to consider:

  • Package developers should use unique and descriptive names for their blade files
  • Blade files by the same name loaded by an app`s provider take precedent over paths loaded by packages

I've included some tests to ensure that Volt::mount() is loading these paths into memory correctly.

@taylorotwell
Copy link
Collaborator

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

@inmanturbo
Copy link
Author

inmanturbo commented Jan 25, 2024

@taylorotwell I honestly find this behavior odd and it feels buggy, but I could be wrong about the intent. Currently, vendors can easily depend on volt, mount an unpublished directory in a service provider and it will work fine, but only as long as the developer using the package never calls Volt::mount() to mount a directory of their own at app level! This seems pretty strange and could be confusing to newcomers, and requires a few extra steps and instructions to get them going with a package that depends on volt.

Also, volt is often used in concert with folio, and folio allows you to call Folio::route() as many times as you like, without overriding all your previous calls. This means packages can easily supply and serve (unpublished) views from their own repositories, however as it stands there is some added difficulty trying to sprinkle in a some volt along with it.

At the very least, are you opposed to us revisiting this again sometime after L11 is released, or is it just not right for volt at all?

Thanks for your time!

EDIT:
Come to think of it, from a package provider one could easily use lifecycle methods to make the mount call come later, then merge in any paths already mounted. I guess it's fine like it is. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants