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

[5.1] Make alias resolution recursive #13976

Merged
merged 1 commit into from
Jun 14, 2016

Conversation

theofidry
Copy link
Contributor

If you have a service foo, which has an alias baz, which itself has an alias bat, then trying to resolve bat with the Container will fail.

Now in practice that's not much of an issue, because you more commonly using Application which extends Container, and Application::make() will try to resolve the alias first, before calling its parent make() method. So the scenario above is actually resolved.

However there is still two cases where it will fail:

  • When you are using more than 2 alias, i.e. we would have another alias bap for bat for example. [1]
  • When your original service is declared in a deferred provider. [2]

[1]: Not very common as you rarely need that many aliases.

[2]: If foo is declared in a deferred provider, when making bat, the abstract got will be baz, which may not be declared as a service provided by the deferred provider, resulting in a service not found.

A simple fix would be to make the alias resolution recursive, not big changes required, no BC break AFAICS and no big (if any) performance impact either.

@theofidry theofidry force-pushed the bugfix/recursive-alias branch from 75a16ba to d236b89 Compare June 14, 2016 12:05
@taylorotwell taylorotwell merged commit d236b89 into laravel:5.1 Jun 14, 2016
@theofidry theofidry deleted the bugfix/recursive-alias branch June 14, 2016 13:38
@GrahamCampbell GrahamCampbell changed the title Make alias resolution recursive [5.1] Make alias resolution recursive Jun 14, 2016
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.

2 participants