Skip to content

[DependencyInjection] Allow using expressions as service factories #45512

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 26, 2022
Merged

[DependencyInjection] Allow using expressions as service factories #45512

merged 1 commit into from
Mar 26, 2022

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 6.1
Bug fix? no
New feature? yes
Deprecations? yes
Tickets -
License MIT
Doc PR -

Replaces #45447

This PR allows using expressions as service factories:

  • in YAML: factory: '@=service("foo").bar()'
  • in PHP: ->factory(expr('service("foo").bar()'))
  • in XML: <factory expression="service('foo').bar()" />

In addition, it allows the corresponding expressions to get access to the arguments of the service definition using the arg($index) function and args variable inside expressions:

services:
  foo:
    factory: '@=arg(0).baz()' # works also: @=args.get(0).baz()
    arguments: ['@bar']

Internally, instead of allowing Expression objects in Definition objects as in #45447, factory expressions are conveyed as a strings that starts with @=. This is chosen by taking inspiration from yaml and to not collide with any existing callable.

@carsonbot carsonbot added this to the 6.1 milestone Feb 22, 2022
@nicolas-grekas nicolas-grekas force-pushed the di-expr-arg branch 2 times, most recently from 1764295 to d4b53cd Compare February 23, 2022 08:04
@jvasseur
Copy link
Contributor

That's an interesting way to do it without having to change the typing of Definition.

I think this could still introduce subtle BC breaks since the PHPDoc of getFactory says:

The PHP function or an array containing a class/Reference and a method to call

people could expect to get a function name when the value is a string and probably doesn't bother doing additional checks before passing it to something like the ReflectionFunction constructor. But this is mostly theoretical, not sure if it would cause real BC breaks.

BTW good jobs adding support for passing arguments to the expression, I didn't bother doing it because the gain wasn't important but it's nice to have expressions be more in line with other types of factories.

@nicolas-grekas
Copy link
Member Author

people could expect to get a function name when the value is a string and probably doesn't bother doing additional checks before passing it to something like the ReflectionFunction constructor. But this is mostly theoretical, not sure if it would cause real BC breaks.

I agree about that, especially the last sentence :)

jvasseur
jvasseur approved these changes Mar 1, 2022
View reviewed changes
Copy link
Contributor

@jvasseur jvasseur left a comment

Choose a reason for hiding this comment

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

Well apart from that theoretical BC break (but every change can break someone's workflow) this looks good to me.

@nicolas-grekas nicolas-grekas force-pushed the di-expr-arg branch 4 times, most recently from b1ae0fe to 913e1ba Compare March 8, 2022 14:39
@jvasseur jvasseur mentioned this pull request Mar 21, 2022
Merged
@fabpot
Copy link
Member

fabpot commented Mar 26, 2022

Thank you @nicolas-grekas.

@fabpot fabpot mentioned this pull request Mar 26, 2022
Closed
@fabpot fabpot merged commit c66bb29 into symfony:6.1 Mar 26, 2022
@fabpot fabpot deleted the di-expr-arg branch March 26, 2022 17:11
@fabpot fabpot mentioned this pull request Apr 15, 2022
Merged
wouterj added a commit to symfony/symfony-docs that referenced this pull request May 1, 2022
@wouterj
feature #16516 Add documentation for using expressions as service fac…
2b58adb 
…tories (jvasseur)

This PR was merged into the 6.1 branch.

Discussion
----------

Add documentation for using expressions as service factories

Doc PR for symfony/symfony#45512

Commits
-------

f8fc3d6 Add documentation for using expressions as service factories
@nicolas-grekas nicolas-grekas mentioned this pull request May 11, 2022
Closed
@VincentLanglet VincentLanglet mentioned this pull request Jun 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stof stof stof left review comments

@fabpot fabpot fabpot approved these changes

@jvasseur jvasseur jvasseur approved these changes

Assignees
No one assigned
Labels
DependencyInjection Deprecation Feature Ready Status: Reviewed
Projects
None yet
Milestone
6.1
Development

Successfully merging this pull request may close these issues.
5 participants
@nicolas-grekas @jvasseur @fabpot @stof @carsonbot