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

Laravel dependency #15

Open
underdpt opened this issue Sep 12, 2018 · 3 comments
Open

Laravel dependency #15

underdpt opened this issue Sep 12, 2018 · 3 comments

Comments

@underdpt
Copy link

Hi,

I'm using Eloquent ouside Laravel, and I found this useful package. In composer.json there are no dependency on the laravel framework, only on illuminate/support, but in Cloneable.php there's a dependency on App which i think is a reference to illuminate/foundation:

	/**
	 * Clone the current model instance
	 *
	 * @return Illuminate\Database\Eloquent\Model The new, saved clone
	 */
	public function duplicate() {
		return App::make('cloner')->duplicate($this);
	}

	/**
	 * Clone the current model instance to a specific Laravel database connection
	 *
	 * @param  string $connection A Laravel database connection
	 * @return Illuminate\Database\Eloquent\Model The new, saved clone
	 */
	public function duplicateTo($connection) {
		return App::make('cloner')->duplicateTo($this, $connection);
	}

It would be good if we could remove that dependency, as then this package would be usable outside laravel. I'm willing to provide a PR but I'm not familiar with laravel's dependency resolver, but maybe we could replace it with something like

use Illuminate\Container\Container;

public function duplicate() {
    return Container::getInstance()->make('Bkwld\Cloner\Cloner')->duplicate($this);
}

I'm overloading the duplicate() function of the Cloneable trait and it seems to work correctly.

Also there's a dependency on illuminate\events that should be added to composer.json

@weotch
Copy link
Member

weotch commented Sep 12, 2018

Yeah, I'm open to that PR.

Do you know if Container::getInstance() will return in the App instance in Laravel?

@underdpt
Copy link
Author

I think yes. Laravel's helper resolve function resolves a given class or interface name to its instance using the service container (doc).

That function only calls another helper, app with the class name and without parameters, which calls directly Container::getInstance()so I think it's safe to use that:

/**
 * Get the available container instance.
 *
 * @param  string  $abstract
 * @param  array   $parameters
 * @return mixed|\Illuminate\Foundation\Application
 */
function app($abstract = null, array $parameters = [])
{
    if (is_null($abstract)) {
        return Container::getInstance();
    }
    return empty($parameters)
       ? Container::getInstance()->make($abstract)
       : Container::getInstance()->makeWith($abstract, $parameters);
}

I'll send a PR.

@Goddard
Copy link

Goddard commented Mar 13, 2020

Did this ever happen? I am also using Eloquent outside of Laravel.

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

No branches or pull requests

3 participants