Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[DependencyInjection][Routing] Access environment in PHP config #40808

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
Apr 14, 2021

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Apr 13, 2021

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations?
Tickets
License MIT
Doc PR

This will allow me to write config like:

use Symfony\Config\FrameworkConfig;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (FrameworkConfig $framework, ContainerConfigurator $container) {
    if ('test' === $container->env()) {
        $framework->test(true);
        $framework->session()->storageFactoryId('session.storage.mock_file');
    }
};

This PR will also revert parts of #40214. It is much simpler to maintain and write PHP config without ->when(). Instead we add ContainerConfigurator::env(): ?string and RoutingConfigurator::env(): ?string.

use App\Controller\BlogController;
use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;

return function (RoutingConfigurator $routes) {
    if ($routes->env() === 'dev') {
        $routes->add('debug-stats', '/blog-debyg')
            ->controller([BlogController::class, 'debug'])
        ;
    }

    $routes->add('blog_list', '/blog')
        ->controller([BlogController::class, 'list'])
    ;
};

yceruto
yceruto previously approved these changes Apr 13, 2021
Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

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

Would it make sense for you to have a ->when('test') method as well? Like we recently did for ContainerConfigurator.

@Nyholm
Copy link
Member Author

Nyholm commented Apr 13, 2021

I will actually work on remove the when() method. It is not really needed in PHP, right?

I like it in yaml and XML because it allows me to write simple logic, however, in PHP I do not need a feature to "help me with simple logic".

@yceruto
Copy link
Member

yceruto commented Apr 13, 2021

I was thinking about this config based on your example:

use Symfony\Config\FrameworkConfig;

return static function (FrameworkConfig $framework)
{
    $framework->when('test')
        ->test(true)
        ->session()->storageFactoryId('session.storage.mock_file')
    ;
};

Not sure if possible though (I didn't check if the generated Config is fluent).

@Nyholm
Copy link
Member Author

Nyholm commented Apr 13, 2021

Yes, That is the alternative way of doing things. The implementation is more difficult and it is less obvious what is happening.

@Nyholm Nyholm dismissed yceruto’s stale review April 13, 2021 21:14

Sorry for opening the PR too early. I would be super happy to get your review later

@Nyholm
Copy link
Member Author

Nyholm commented Apr 14, 2021

Hm.. So we are loading callbacks on 4 places.

  1. DI: Microkernel trait
  2. DI: PhpFileLoader
  3. Router: Microkernel trait
  4. Router: PhpFileLoader

For 2, we have a fancy resolver of the parameters. So order does not matter, but parameter name does. For the others, order does matter but not parameter name.

The fancy resolver is needed for the ConfigBuilders. I would like to use fancy resolving for all of these, but either I write (almost) the same code on 3 more places or I try to do something complex...

@Nyholm Nyholm changed the title [DependencyInjection] Allow to type hint for string $env in PhpFileLoader [DependencyInjection][Routing] Access environment in PHP config Apr 14, 2021
@Nyholm Nyholm added the Routing label Apr 14, 2021
@Nyholm Nyholm requested a review from yceruto April 14, 2021 07:13
@Nyholm
Copy link
Member Author

Nyholm commented Apr 14, 2021

So, instead of doing fancy argument resolving on all places. I added ContainerConfigurator::env(): ?string and RoutingConfigurator::env(): ?string.

I've updated the description. This PR is ready for a review.

@nicolas-grekas
Copy link
Member

Thank you @Nyholm.

@nicolas-grekas nicolas-grekas merged commit 48ae511 into symfony:5.x Apr 14, 2021
@Nyholm Nyholm deleted the string-env branch April 14, 2021 16:40
@Nyholm
Copy link
Member Author

Nyholm commented Apr 14, 2021

Woho. Let's start write php config

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.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.