Skip to content

[DI] Deprecate case insentivity of service identifiers #21193

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

Closed
nicolas-grekas opened this issue Jan 7, 2017 · 7 comments
Closed

[DI] Deprecate case insentivity of service identifiers #21193

nicolas-grekas opened this issue Jan 7, 2017 · 7 comments
Labels
DependencyInjection RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Milestone

Comments

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 7, 2017

Q A
RFC? yes
Symfony version 3.3

Case insensitivity blocks some optimizations in the dumped container, slows down getting some services (especially now that we're going to use class names as ids) and adds code complexity.
We'd better make them case sensitive in 4.0, thus deprecate before.

@nicolas-grekas nicolas-grekas added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Jan 7, 2017
@nicolas-grekas nicolas-grekas added this to the 3.3 milestone Jan 7, 2017
@fabpot
Copy link
Member

fabpot commented Jan 7, 2017

I agree

@mvrhov
Copy link

mvrhov commented Jan 7, 2017

So the recommended way of getting service by name will from 4.00 be ClassName::class as the IDE will help greatly there will be less typing however overriding/decorating the services will be annoying as the name won't represent the actual class anymore but the name itself will suggest that. ATM the service name could be seen as some kind of alias.

P.S. also service names will be longer I'm wondering how much more RAM will the cached DIC require and also if those strings go into the interned string cache the cache might become to small the default which is 4M might be to small even today

@stof
Copy link
Member

stof commented Jan 9, 2017

@mvrhov we don't enforce using class names as id (it would break many use cases, as being able to replace an implementation is a great use case).
But we are making the id optional, using the class name in this case, to keep things simpler for newcomers.

@mvrhov
Copy link

mvrhov commented Jan 9, 2017

Then the above doesn't make much sense (ok, it does, if the service name is class name). AFAIR the we switched once before to CI ones.

@stof
Copy link
Member

stof commented Jan 9, 2017

I don't understand your answer

fabpot added a commit that referenced this issue Jan 11, 2017
… (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Deprecate case insentivity of service identifiers

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | minor (see UPGRADE note)
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #21193
| License       | MIT
| Doc PR        | -

As discussed in linked RFC.

Commits
-------

d08f110 [DI] Deprecate case insentivity of service identifiers
@fabpot fabpot closed this as completed Jan 11, 2017
@chalasr
Copy link
Member

chalasr commented Jan 12, 2017

Would doing the same for parameters be relevant?

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Jan 12, 2017

@chalasr I don't think so. I mean, deprecating for ids has first technical reason, which is doing more inlining, thus perf. For params, unless you can envision one, there is no such gain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DependencyInjection RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests

6 participants