-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Proposal][WIP][TwigBundle] Extract error handling into separate ErrorPageBundle #22450
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
Conversation
a4db958
to
91b394e
Compare
…undle because its functional tests depend on error handling
91b394e
to
e803a54
Compare
This bundle still requires the TwigBundle to able to render the template, so why should this be extracted to a separate bundle? |
@sstok I think that the error page functionality simply does not belong into TwigBundle. If we separate the bundles, TwigBundle will remain with just one responsibility - to "glue" Twig into Symfony Framework. Similarly ErrorPageBundle will have one responsibility - to bring fancy customizable error pages into Symfony Framework.
In my opinion, "X requires Y" relation does not imply "Y contains X" relation. |
I don't see the point. What value does it bring? |
Recently, PR #20951 that redesigned the exception page was merged. I noticed that the new design looks different from the exception page that Debug component provides. Also, the highlighted code in TwigBundle's error page uses different colour schema than for example WebProfilerBundle's TL;DR: The point is less confusing code. However I would understand if the necessary changes and BC breaks were not worth it. :| |
I don't see the need also, I'd be -1 here |
The reason why the WebProfilerBundle duplicates the exception rendering instead of reusing the TwigBundle one directly is because we want to make the WebProfilerBundle UI available for Silex too for instance. So splitting ErrorPageBundle would not really allow sharing the code |
Ok, I respect your opinions. |
CHANGELOG.md
filesUPGRADE.md
filescomposer.json
in ErrorPageBundle (it was copy-pasted from TwigBundle)composer.json
in TwigBundle (it may contain dependencies that are no longer needed)symfony/error-page-bundle
package on Packagistsymfony/symfony-standard
to use the new bundleI suggest moving error handling (ie. exception listener service, exception controller, error templates and error preview controller) from TwigBundle to new ErrorPageBundle.
Error handling was moved to TwigBundle from FrameworkBundle back in Symfony 2.0 (see 3749ad4) but in my opinion it does not belong there.
This change is BC-breaking so I guess it can be done only in next major release (Symfony 4.0).
What is the correct way to do such BC breaks. For example should there be a deprecated
twig.exception_listener.controller
service that is an alias pointing to the newerror_page.exception_listener.controller
service?And how should I specify that
What do you think about this PR?