-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[HttpFoundation] Make dependency on Mime component optional #35642
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
[HttpFoundation] Make dependency on Mime component optional #35642
Conversation
Thank you @atailouloute. |
…ional (atailouloute) This PR was merged into the 5.1-dev branch. Discussion ---------- [HttpFoundation] Make dependency on Mime component optional | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | | License | MIT | Doc PR | Make the Mime component dependency optional /cc @nicolas-grekas Commits ------- 11cef32 [HttpFoundation] Make dependency on Mime component optional
I think this should be reverted. It breaks laravel/framework's 7.x branch which depends on symfony/http-foundation: ^5.0. When v5.1.0 comes out, it will be broken. I'd argue Laravel was not using symfony/mime implicitly, since it called code in http-foundation. It didn't make use of a non-direct dependency by bypassing http-foundation code. I think Symfony should wait until 6.0 before making this an optional dependency. |
// cc @taylorotwell |
Is Laravel 7 released? I'd suggest explicitly requiring symfony/mime on your side. |
Laravel 7 will depend on ^5.0, which means once 5.1.0 is tagged, we'd have unexpected breakage. |
What was more concerning is that this change might be wanted in a symfony 4.4 patch release? |
Even if you add symfony/mime in your deps explicitly? |
We should be fine once we do that. My objection was to the principal of such changes. |
I missed the PR that made it optional recently, sorry. This is for master. |
this won't happen, @chalasr must have been mistaken :)
that's the way to go then, this change is desired on our side (to me at least.) |
Ok. :) |
…ahDude) This PR was merged into the 5.1-dev branch. Discussion ---------- [HttpFoundation] Fixed Mime dependency missing error | Q | A | ------------- | --- | Branch? | master | Bug fix? | kinda | New feature? | no | Deprecations? | no | Tickets | ~ | License | MIT | Doc PR | ~ Follows #35642, by adding a missing exception and a note in the UPGRADE file (CHANGELOG in HttpFoundation was already up to date). Reported in symfony/symfony-docs#1307 Commits ------- fef0de3 [HttpFoundation] Fixed Mimes dependency missing error
Make the Mime component dependency optional
/cc @nicolas-grekas