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

ensure mounted directories are unique #98

Closed

Conversation

inmanturbo
Copy link

@inmanturbo inmanturbo commented Mar 5, 2024

This PR filters the given $paths, $uses in MountedDirectories::mount() to ensure there isn't already a MountedDirectory with the exact same properties.

This won't break anything since it doesn't change any functionality. The same views as before will still be mounted, we just won't add a duplicate MountedDirectory instance for them to the $paths array if one already exists.

closes #96

EDIT (more context):

One might expect that mounting the same directory more than once with volt wouldn't change anything, and it doesn't. However there is an unfortunate and unexpected side effect: each call will increase memory usage by adding a new MountedDirectory instance to the manager's footprint with the same properties as the previous one, and for no useful reason as far as I can tell.

Of course it looks like the $paths property isn't even being used anyway: the local $paths variable created directly from the given$path, $uses is being used instead to create the views using only the given parameters and ignoring the existing properties. But that is a separate issue and this pr just aims to decrease unwanted/unneeded memory usage regardless of the variable or scope used for view creation.

It may be also worth looking at whether the views should be created from the given $paths, $uses, or all of the paths in the paths property as proposed in #95.

A few of us have been looking into how to use livewire/volt in a package and provide components to a laravel application. One of the ways we've found to do this is by calling Volt::mount() from a package's service provider. In order to prevent overwriting paths already mounted by the application, we've found that we can merge in the existing paths.

However in doing so I've noticed that whether existing paths are merged in or not, Volt stores all the paths from all the calls to mount() regardless of whether or not they are being used. This includes duplicates, which serve no discernible purpose.

#94
#92
#87

@taylorotwell
Copy link
Collaborator

Hey there - I'm not sure this is actually solving a problem people are encountering in reality because Volt::mount is only called from service providers.

@inmanturbo
Copy link
Author

inmanturbo commented Mar 6, 2024

@taylorotwell when calling Volt::mount from service providers, every time it's called it doubles the paths. So the memory usage will grow exponentially with every package installed which uses volt.

EDIT:
Actually in a way #95 fixes this for me. Because packages will no longer need to "re-register" the existing or default paths, so it won't keep doubling. So there's no longer any need to call Volt::mount() with the same path(s) more than once.

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.

Volt::mount() merges duplicate paths [Memory leak]
2 participants