Skip to content
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

Alias for services created via dependency injection are not public #384

Closed
sawmurai opened this issue Jan 17, 2018 · 6 comments
Closed

Alias for services created via dependency injection are not public #384

sawmurai opened this issue Jan 17, 2018 · 6 comments
Labels
enhancement Improves existing functionality
Milestone

Comments

@sawmurai
Copy link

First of all: Awesome bundle :)

When an alias is created here:

https://github.com/snc/SncRedisBundle/blob/2.0.6/DependencyInjection/SncRedisExtension.php#L206

it is created in a way that makes it private (it is private by default even if the service is public). By creating it like this

$container->setAlias(sprintf('snc_redis.%s_client', $client['alias']), new Alias(sprintf('snc_redis.%s', $client['alias']), true));

it is possible to create it and make it public.

Are the aliases supposed to be private or do you want a PR? :)

@sawmurai
Copy link
Author

sawmurai commented Jan 29, 2018

Hello there. Any thoughts about this one?

@xabbuh
Copy link
Contributor

xabbuh commented Jan 29, 2018

The best practice when using Symfony 3.4 or higher is to define almost all services private and rely on proper dependency injection instead of using the container as a service locator. So IMO not making them public is the desired behaviour.

@sawmurai
Copy link
Author

I agree but this specific alias is for a service that is also declared public (a few lines above). So I think it would make sense to either declare the service private or declare the alias public to stay consistent.

@curry684
Copy link
Collaborator

curry684 commented Apr 6, 2018

We're preparing a 3.0 release targetting Symfony 3.4+ and PHP 7+ explicitly, currently in master branch. PR making it consistently private is breaking and therefore welcome if targeting master.

@curry684 curry684 added this to the 3.0 milestone Apr 6, 2018
@curry684 curry684 added the enhancement Improves existing functionality label Apr 6, 2018
curry684 added a commit that referenced this issue Apr 9, 2018
Since Symfony 3.4 recommended practice is to keep all services private
and use declarative dependency injection.

Closes #384
@curry684
Copy link
Collaborator

curry684 commented Apr 9, 2018

@sawmurai can you four-eye #409?

@sawmurai
Copy link
Author

sawmurai commented Apr 9, 2018

Looks good imo :)

curry684 added a commit that referenced this issue Apr 9, 2018
Since Symfony 3.4 recommended practice is to keep all services private
and use declarative dependency injection.

Closes #384
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves existing functionality
Projects
None yet
Development

No branches or pull requests

3 participants