Skip to content

Conversation

@franzliedke
Copy link
Contributor

As proposed in #25391 and #25497, this registers a binding in the container that the Schema facade can use.

This means all facades in Laravel's core use strings to describe the facade root, which could let us remove some complexity from the base facade class in a next step. 😃

Open question:

  • Do we want to register an alias for this binding? That would be \Illuminate\Database\Schema\Builder in this case.

@taylorotwell taylorotwell merged commit 0dd0c63 into laravel:master Sep 7, 2018
@franzliedke franzliedke deleted the 5.8-schema-facade branch September 7, 2018 20:48
franzliedke added a commit to franzliedke/framework that referenced this pull request Sep 7, 2018
Because of laravel#25497 and laravel#25512, all facades in core are now properly
using service identifiers (strings) for resolving their "root"
service from the container. This means that the hack for resolving
objects directly can now be removed.

In case you're wondering why this is necessary: As far as I can
tell, facade features like ::swap() did not work with these types
of facades (Blade and Schema), because those methods did not deal
with the possibility of objects being returned.
taylorotwell pushed a commit that referenced this pull request Sep 10, 2018
Because of #25497 and #25512, all facades in core are now properly
using service identifiers (strings) for resolving their "root"
service from the container. This means that the hack for resolving
objects directly can now be removed.

In case you're wondering why this is necessary: As far as I can
tell, facade features like ::swap() did not work with these types
of facades (Blade and Schema), because those methods did not deal
with the possibility of objects being returned.
@driesvints
Copy link
Member

@franzliedke I resent this in to master while keeping in mind the previous breakages. I'd appreciate your review: #38017

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.

3 participants