Skip to content

[FrameworkBundle] Simpler Kernel setup with MicroKernelTrait #57408

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

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Jun 15, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

Minor improvements to create minimalistic Symfony apps with zero config (initially) + invokable controller using the Kernel class only:

// index.php
<?php

require_once __DIR__.'/vendor/autoload_runtime.php';

// ...

class StripeWebhookEventSubscriber extends Kernel
{
    use MicroKernelTrait;

    #[Route('/', methods: 'GET')]
    public function __invoke(Request $request, NotifierInterface $notifier): Response
    {
        // ...

        return new Response('OK');
    }
}

return static function (array $context) {
    $kernel = new StripeWebhookEventSubscriber($context['APP_ENV'], (bool) $context['APP_DEBUG']);

    return \PHP_SAPI === 'cli' ? new Application($kernel) : $kernel;
};

Note

Ideal for one-file apps or workers.

What exactly does this PR improve?

  • It makes the config/ directory optional. Then you can add extension configs by redefining the configureContainer() method or by importing external files in config/packages/* as usual.
  • It adds support for service arguments when an invokable controller is defined within the same kernel class (as shown in the example)
  • Fall back to yield new FrameworkBundle() in registerBundles() method in case the $this->getBundlesPath() file does not exist.

What do you think?

@yceruto
Copy link
Member Author

yceruto commented Jun 17, 2024

Wondering if we could also go with a SingleAppKernel class like this:

abstract class SingleAppKernel extends Kernel
{
    use MicroKernelTrait;

    return static function init(): \Closure
    {
        return static function (array $context) {
            return new static($context['APP_ENV'], (bool) $context['APP_DEBUG']);
        };
    }

    public function registerBundles(): iterable
    {
        yield new FrameworkBundle();
    }
}

and then reduce this use case to:

final class App extends SingleAppKernel
{
    #[Route('/')]
    public function __invoke(): Response
    {
        return new Response('Hello World!');
    }
}

return App::init();

?

@nicolas-grekas
Copy link
Member

Not convinced by SingleAppKernel. Could be OK to provide a fallback for registerBundles.

@wouterj
Copy link
Member

wouterj commented Jun 17, 2024

What is the use-case for defining an invokable controller in the kernel? This is a convention for action controllers with a class per controller, I don't think we should apply that convention to other classes. Replacing __invoke with index or controller or whatever is already fully supported.

I'm also not sure about the SingleAppKernel class. To me, the simple micro kernel is a nice gimmick but has a very limited use-case (it requires more work to create and maintain an application using this than it is to use a normal Symfony app). For this specific niche, the existing (with the fix for optional config/ directory) solution seems more than good enough to me.

@yceruto
Copy link
Member Author

yceruto commented Jun 17, 2024

@wouterj, the __invoke method is already supported. I'm working on a Cloud worker app that only requires one controller and it's nice because the class name already provides the semantics that I need for this small piece of code. The only thing missing was enabling the service arguments capability (which is being added here).

@yceruto yceruto force-pushed the simple_kernel branch 2 times, most recently from 96b5e5b to 33bac4c Compare June 17, 2024 14:45
Copy link
Member

@dunglas dunglas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@yceruto
Copy link
Member Author

yceruto commented Jun 17, 2024

Well, I'm really happy with how small it is now, even though we still have to set up the closure function ourselves:

return static function (array $context) {
    $kernel = new StripeWebhookEventSubscriber($context['APP_ENV'], (bool) $context['APP_DEBUG']);

    return \in_array(\PHP_SAPI, ['cli', 'phpdbg', 'embed'], true) ? new Application($kernel) : $kernel;
};

Any idea to simplify it more? In a nutshell, I'd love to just initialize my kernel/app and run. Random proposals:

return fn () => StripeWebhookEventSubscriber::class; // and then the Runtime does the rest new $kernelClass(...)

return StripeWebhookEventSubscriber::init(); // implement a default in `MicroKernelTrait`

return new StripeWebhookEventSubscriber(); // and set up the env and debug vars using setter through the Runtime?

@yceruto
Copy link
Member Author

yceruto commented Jun 17, 2024

I've updated the PR description to include my use case as code example

@yceruto yceruto force-pushed the simple_kernel branch 2 times, most recently from 08ed13a to fd7d3b9 Compare June 18, 2024 13:51
@yceruto
Copy link
Member Author

yceruto commented Jun 18, 2024

It's ready on my side. Thank you all for your reviews!

@fabpot
Copy link
Member

fabpot commented Jun 19, 2024

Thank you @yceruto.

@fabpot fabpot merged commit 5279a30 into symfony:7.2 Jun 19, 2024
7 of 10 checks passed
@yceruto yceruto deleted the simple_kernel branch June 19, 2024 11:20
@plandolt
Copy link
Contributor

Not sure why, but after this commit my controllers and commands are no longer loaded. Can be reproduced with a fresh installation with the symfony cli with --version=next.

@yceruto
Copy link
Member Author

yceruto commented Jun 19, 2024

Oops! Fixed in #57455

@fabpot fabpot mentioned this pull request Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants