Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upBetter support for Lumen: Return Illuminate\Http\Response instance #1051
Conversation
benjamindoe
changed the title
|
It his package officially compatible with Lumen? The reason I ask is that the change is breaking backwards compatibility. Before this PR, you could have easily mocked (faked) the Facade for whatever reason, impossible now. However you can enable Facades in Lumen. This should be carefully considered and needs a major version bump if it goes in IMHO. |
|
Good spot. I could easily just add it to another if statement. I assumed the package was compatible with Lumen due to there being a Lumen-specific service provider. You can enable Facades in Lumen but that doesn't fix the issue of incorrect response class being returned. Although it's wise for Lumen support to not rely of facades as it's enabled by default for performance gains. |
My apologies and I assert you're definitely right about this! (doesn't change my technical feedback though) |
|
The latest commit checks whether facades have been "enabled". In Lumen, calling |
Currently, with a Lumen application,
$image->response()returnsSymfony\Component\HttpFoundation\Response. This change compares theapp()container withIlluminate\Container\Containerand correctly returns an Illuminate Response without the use of a facade.Removing the facade use is key as Lumen applications have facades disabled by default.