-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WIP] Route building in the kernel #15948
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
[WIP] Route building in the kernel #15948
Conversation
public function getRouteCollection(LoaderInterface $loader) | ||
{ | ||
$routes = new RouteCollectionBuilder($loader); | ||
$routes->add('/admin', 'AppBundle:Admin:dashboard', 'admin_dashboard'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like some test code left here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely did - fixed now
aa2ac38
to
662ceec
Compare
662ceec
to
62f0ba6
Compare
I don't like the mixed-responsibilities here and the fact that the kernel implements the <?php
namespace Symfony\Bundle\FrameworkBundle\Kernel;
use Symfony\Bundle\FrameworkBundle\Routing\RouteCollectionBuilder;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\MicroKernel as BaseMicroKernel;
abstract class MicroKernel extends BaseMicroKernel
{
abstract protected function configureRoutes(RouteCollectionBuilder $routes);
public function registerContainerConfiguration(LoaderInterface $loader)
{
$loader->load(function (ContainerBuilder $container) use ($loader) {
$container->prependExtensionConfig('framework', array(
'router' => array(
'resource' => function () use ($loader) {
return $this->configureRoutes(new RouteCollectionBuilder($loader))->build();
},
'type' => 'closure',
),
));
});
parent::registerContainerConfiguration($loader);
}
} |
Of course, my example does not work in PHP 5.3, but adding a |
I'm going to submit a PR soon with my own version of a micro-kernel. |
That does not work right now in Symfony as we try to dump the container to PHP (and XML in dev mode); so the router resource being a closure, it explodes. |
see #15990 |
public function registerContainerConfiguration(LoaderInterface $loader) | ||
{ | ||
$loader->load(function (ContainerBuilder $container) { | ||
$container->prependExtensionConfig('framework', array( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loadFromExtension
is enough here as we are always the first to be called.
Closes in favor of #15990 |
This PR was merged into the 2.8 branch. Discussion ---------- added a micro kernel | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Related to #15948 and #15820 Commits ------- eab0f0a added a micro kernel
WIP, because it depends on the unmerged PR's #15742, #15778 and #15820. But since this completes the picture (and I'm pushing this for 2.8), I needed to get this PR created.
The idea is to make a "normal" kernel look like this: https://gist.github.com/weaverryan/e4b19cabb1d9286c4217#file-smallkernel-php, which has some nice benefit that
routing_dev.yml
isn't needed anymore, because you can import those routes conditionally here.But, you could also create an entire kernel without external files. A complex example (probably more complex than you'd have without using external files) is here: https://gist.github.com/weaverryan/e4b19cabb1d9286c4217#file-appkernel-php.
Thanks!