Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

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

Sign up
Appearance settings

[DI] Remove synthetic services from methodMap + generated methods#21244

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromnicolas-grekas:di-synthetic
Jan 15, 2017

Conversation

@nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedJan 12, 2017
edited
Loading

QA
Branch?master
Bug fix?no
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets-
LicenseMIT
Doc PR-

For synthetic services, the generated methods are just dead code to fill opcache ;)
And having them in "methodMap" prevents using the property for checking if a service comes from a (non-synthetic) definition or not.
This prepares some changes that we'd like to do in 4.0, see#19668.

ro0NL and linaori reacted with thumbs up emoji
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage You have requested asynthetic service("request"). The DIC does not know how to construct this service.
* @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
Copy link
Contributor

Choose a reason for hiding this comment

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

We go from\RuntimeException to\InvalidArgumentException now.. minor BC break :)

👍 for master, as it's needed to keep things simple.

@stof
Copy link
Member

The generated method (and it existence in the method map) has another purpose: it allowshas() to know about the service, and means thatget('kernel') gives you a meaningful message if it is not set yet (same for other synthetic services).

@nicolas-grekas
Copy link
MemberAuthor

@stof, yes, and my point is that we don't care, because these are erroneous situations anyway. The fact the current behavior prevents meaningful progress largely wins over the small&rare DX+ that it provides.
For "has", I don't expect anyone to rely on a faulty behavior (userland side) anyway :)

@fabpot
Copy link
Member

👍

@nicolas-grekas
Copy link
MemberAuthor

@stof no blocker for you? I'd like to merge to unlock progress in#19668.

@ro0NL
Copy link
Contributor

It makes sense right? Ie. we donthavekernel till it's set.has() returning true, whileget() throws is weird..

Imo. the exception is even better now, service not found.

@ro0NL
Copy link
Contributor

ro0NL commentedJan 14, 2017
edited
Loading

What if an alias refers a pre-defined service? I thinkhas() needs an update so that$id is set first, and continue checks based on the aliased id (basically whatget() does).

@nicolas-grekas
Copy link
MemberAuthor

@ro0NL updated, was that what you meant?

@ro0NL
Copy link
Contributor

Yep 👍

@lyrixx
Copy link
Member

👍

@fabpot
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commitc1e1e99 intosymfony:masterJan 15, 2017
fabpot added a commit that referenced this pull requestJan 15, 2017
…ted methods (nicolas-grekas)This PR was merged into the 3.3-dev branch.Discussion----------[DI] Remove synthetic services from methodMap + generated methods| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -For synthetic services, the generated methods are just dead code to fill opcache ;)And having them in "methodMap" prevents using the property for checking if a service comes from a (non-synthetic) definition or not.This prepares some changes that we'd like to do in 4.0, see#19668.Commits-------c1e1e99 [DI] Remove synthetic services from methodMap + generated methods
@nicolas-grekasnicolas-grekas deleted the di-synthetic branchJanuary 22, 2017 09:19
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@ro0NLro0NLro0NL left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

6 participants

@nicolas-grekas@stof@fabpot@ro0NL@lyrixx@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp