ship a forward compatible annotation route loader #2300
Conversation
|
see symfony/symfony#23044 for the PR that introduced this feature in Symfony |
GuilhemN
left a comment
There was a problem hiding this comment.
👍, my only concern is that priorities need to be replaced (didn't exist prior 3.4)
|
|
||
| // FrameworkBundle 3.4+ ships it own AnnotationClassLoader implementation, for older versions, we need to | ||
| // register our own implementation | ||
| if (!class_exists('Symfony\Bundle\FrameworkBundle\Routing\AnnotatedRouteControllerLoader')) { |
There was a problem hiding this comment.
To avoid unreliable behavior (support of @Method not consistent for instance), I think this should be done in a compiler pass to be certain it's registered only if sensio's loader is not. Otherwise annotations support will be different based on the order bundles are registered.
| if (!class_exists('Symfony\Bundle\FrameworkBundle\Routing\AnnotatedRouteControllerLoader')) { | ||
| $container->register('easyadmin.routing.loader.annotation', 'EasyCorp\Bundle\EasyAdminBundle\Router\AnnotatedRouteControllerLoader') | ||
| ->setPublic(false) | ||
| ->addTag('routing.loader', array('priority' => -10)) |
842a67e to
6a77e30
Compare
| return; | ||
| } | ||
|
|
||
| if (!$container->has('routing.resolver')) { |
There was a problem hiding this comment.
We could also return earlier if sensio's annotation loader is registered (not strictly required but I think it'd be cleaner)
|
I'm afraid I'm a bit lost here. I don't fully understand the problem being solved here. But I trust @xabbuh completely, so please tell me if you think this is ready and I'll merge it. Thanks! |
|
@javiereguiluz I try to finish this until the end of this week. |
13b1fc9 to
83a1e71
Compare
In previous versions of EasyAdminBundle, the loading of annotated routes was handled by the SensioFrameworkExtraBundle. With version 5.2 of the SensioFrameworkExtraBundle their route annotation loader was deprecated in favour of the implementation shipped with the Symfony FrameworkBundle 3.4+. This commit provides a BC layer by wiring up the implementation of the annotated route controller for older versions of the FrameworkBundle in the same manner the Symfony core uses nowadays.
83a1e71 to
304e661
Compare
|
Christian, I appreciate a lot your work here ... but I prefer to close this without merging. I want to stop merging things in |
In previous versions of EasyAdminBundle, the loading of annotated routes
was handled by the SensioFrameworkExtraBundle. With version 5.2 of the
SensioFrameworkExtraBundle their route annotation loader was deprecated
in favour of the implementation shipped with the Symfony FrameworkBundle
3.4+.
This commit provides a BC layer by wiring up the implementation of the
annotated route controller for older versions of the FrameworkBundle in
the same manner the Symfony core uses nowadays.