Skip to content

Symfony 3.0 proposed backward incompatible changes #11742

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
javiereguiluz opened this issue Aug 23, 2014 · 28 comments
Closed

Symfony 3.0 proposed backward incompatible changes #11742

javiereguiluz opened this issue Aug 23, 2014 · 28 comments

Comments

@javiereguiluz
Copy link
Member

This is a meta-issue that groups all the proposed backward incompatible changes for the upcoming Symfony 3.0 version. It includes more information than the 3.0 milestone because sometimes people propose ideas in the comments of the issues and pull requests.

Commands

  • Make assets:install even smarter and create symlinks by default (without providing the --symlinks option) (ref)
  • ...

Console

  • As one may want to rely on the asset helper to generate URIs outside from a request (i.e., in a command-line, for sending e-mails), it should not depend on the request service, but rather on request_stack, in order to be available in a wider scope. (ref, related)
  • InputInterface operates only with arguments passed to the Application. I suggest to add a method that would allow to operate with an input stream (STDIN in case of CLI application). (ref)
  • Symfony\Component\Console\Output\OutputInteface should include the following methods: isQuiet, isVerbose, isVeryVerbose and isDebug. (ref, ref, [Console] Define isVerbose(), etc. methods in OutputInterface #13086)
  • remove the shell feature (not that useful and bad side-effects see Shell and Doctrine #11750)
  • ...

DependencyInjection

DomCrawler

  • Add support for fallback hidden fields. (ref)
  • Cleaning the Crawler API. (ref)
  • ...

Forms

  • IntegerType should be used for whole integers as the name suggests and not for floats. (ref)
  • NumberType should be used for floats (rendering as an input[type="number"]), and have a new option (for instance support_locale_strings) so that it renders as input[type="text"] and has the relevant attached NumberToLocalizedStringTransformer. (ref)
  • Remove duplicated HTML attributes. Right now, the attributes passed in the attr option are rendered both in the <form> tag and in the <div>/<table> tag of the form. (ref)
  • ...

FrameworkBundle

HttpFoundation

  • Remove the request service (ref)
  • Lazy initialization of Request headers. This can be a perfomance improvement for all requests which does not operate with request headers (almost all?) (ref)
  • Request->getRequestFormat() should only rely on the request attributes. (ref)
  • Deprecate session as a service (ref)
  • ...

HttpKernel

  • Remove isClassInActiveBundle() method from Kernel and KernelInterface (ref)

Security

  • supportsClass and supportAttribute are actually never used in Symfony except by voters themselves. They should be removed from the VoterInterface. (ref)
  • A clear exception should be thrown if no voters voted on an attribute. So if you did a typo (e.g. CREETE instead of CREATE), you'd see a clear message, rather than nobody voting and access being granted/denied 100% of the time based on your voting strategy. (ref)
  • Update UserInterface and UserProviderInterface to change the name of the methods getUsername() and loadUserByUsername() because they are confusing. You can identify your users bu email for example, so hardcoding username doesn't look right (ref, ref)
  • Deprecate SecurityContext and move its code to the AuthorizationChecker and TokenStorage. (ref)
  • Remove the RoleInterface and the Role class in favor of using strings only for roles: [Security] RoleSecurityIdentity checks for instances of RoleInterface to allow custom Role implementation #8313 (comment)
  • ...

Translation

  • Replace custom and hackish solutions (PR 4884, PR 5547) by the standard MessageFormatter PHP class. (ref)
  • ...

Web Profiler

@stof
Copy link
Member

stof commented Aug 23, 2014

It's not easy to generate Uris, asset Uris, etc. due to the infamous request scope

the issue is only about generating asset uris. there is no request scope involved when using the router

@javiereguiluz
Copy link
Member Author

@stof updated the description of the issue. Thanks!

@javiereguiluz
Copy link
Member Author

@lyrixx you are right! But this is a WIP issue, so we still have to add tens (hundreds?) of issues. Anyway, I'm adding right now the issues that you proposed. Thanks!

@weaverryan
Copy link
Member

@javiereguiluz Can you change the title to "Symfony 3.0 proposed backward incompatible changes"? That will help me understand immediately that we're not re-documenting the UPGRADE-3.0 file, but are keeping track of things we want to do for 3.0.

Thanks!

@javiereguiluz javiereguiluz changed the title Symfony 3.0 backward incompatible changes Symfony 3.0 proposed backward incompatible changes Aug 23, 2014
@javiereguiluz
Copy link
Member Author

@weaverryan done! Excuse me for the confusion.

@Koc
Copy link
Contributor

Koc commented Aug 25, 2014

supportsClass and supportAttribute are actually never used in Symfony except by voters themselves. They should be removed from the VoterInterface.

What about moving checks to the Symfony side from voters?

@stof
Copy link
Member

stof commented Aug 25, 2014

@Koc supportsClass and supportAttribute are not easy to use for a good implementation:

  • you are not always able to call supportsClass given that $object can be null (when voting for roles for instance)
  • supporting inheritance properly for a supportsClass implementation is tricky. It requires 2 conditions. See https://github.com/FriendsOfSymfony/FOSUserBundle/blob/094bea6f318fbb067db3ddf6d26a62af0bf13442/Security/UserProvider.php#L81 for instance. Doing the check in isGranted with the object itself allows to use an instanceof operator and being done with it.
  • supportAttribute is limited. Some voters may be able to support different attributes based on the object they receive
  • there can be more reasons to abstain.

We don't need these methods in the interface, and enforcing their usage would make implementing voters more complex IMO

@linaori
Copy link
Contributor

linaori commented Aug 26, 2014

As I made this PR, I find this name confusing: "Separate SecurityContext from the Security component. (ref)". Mainly because I have marked the SecurityContext as deprecated due to logic moving to the AuthorizationChecker and TokenStorage. It's not moving away from the component.

@javiereguiluz
Copy link
Member Author

@iltar fixed! Thanks for noticing this error.

@hacfi
Copy link
Contributor

hacfi commented Sep 6, 2014

I created a deprecation PR #11869 for removal of Kernel/KernelInterface::isClassInActiveBundle (HttpKernel).

@Tobion
Copy link
Contributor

Tobion commented Oct 5, 2014

I removed #10645 from the list.

@stof
Copy link
Member

stof commented Oct 8, 2014

I added the proposal of removing the Role and RoleInterface from the Security component which was suggested in #8313 (comment)

@sebastianblum
Copy link

I would suggest that we rename all event subscriber classes from Listener to Subscriber with 3.0

@fabpot
Copy link
Member

fabpot commented Oct 8, 2014

@sebastianblum No, a subscriber class can also be used as a regular listener one. That's why they are named like this and that should not be changed.

@linaori
Copy link
Contributor

linaori commented Oct 8, 2014

Classes that connect to the event dispatcher should be suffixed with Listener.

I think this part of documentation should be updated in that case: http://symfony.com/doc/current/cookbook/bundles/best_practices.html#classes

Proposal:

Classes that connect to the event dispatcher should be suffixed with Listener or Subscriber.

@Koc
Copy link
Contributor

Koc commented Oct 18, 2014

ref #10832, #9071

@Koc
Copy link
Contributor

Koc commented Apr 22, 2015

ref #9233, #6036 (comment)

@Tobion
Copy link
Contributor

Tobion commented Sep 7, 2015

I updated the description with current state.

@fabpot
Copy link
Member

fabpot commented Sep 7, 2015

#15709 deprecates the profiler:import/export commands

@Tobion
Copy link
Contributor

Tobion commented Sep 7, 2015

@fabpot there is still a "remove prototype scope (#12585 (comment))" todo item which would mean removing the "shared" flag on the DI again which was just added. Is it really wanted?

@wouterj
Copy link
Member

wouterj commented Sep 7, 2015

@Tobion this was discussed shortly during last meeting: https://gist.github.com/Nicofuma/2f75343c73aa75577b52#file-symfony-metting-L34-L81

@Tobion
Copy link
Contributor

Tobion commented Sep 7, 2015

I see. It seems people agreed that removing shared=false is not needed as it already exists and is documented (via prototype) and doesn't cause overhead. Factory use-cases also exist. So I'm removing that point from the list.

@fabpot
Copy link
Member

fabpot commented Sep 9, 2015

shared should be kept.

fabpot added a commit that referenced this issue Sep 9, 2015
…(fabpot)

This PR was merged into the 2.8 branch.

Discussion
----------

[WebProfilerBundle] deprecated import/export commands

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | part of #11742
| License       | MIT
| Doc PR        | n/a

Commits
-------

943fec9 [HtppKernel] deprecated Profiler::import/export
17e00b9 [WebProfilerBundle] deprecated import/export commands
fabpot added a commit that referenced this issue Sep 15, 2015
…commands (jakzal)

This PR was merged into the 3.0-dev branch.

Discussion
----------

[WebProfilerBundle][HttpKernel] removed import/export commands

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | part of #11742
| License       | MIT
| Doc PR        | n/a

Follow up for #15709.

Commits
-------

c1d028e [HttpKernel] removed Profiler::import/export
1672a83 [WebProfilerBundle] removed import/export commands
@stof
Copy link
Member

stof commented Sep 20, 2015

I added #15849 in the list. I already thought about this in the past (always in cases when I was reviewing PRs related to optimizing the component btw), but it looks like I never opened an issue to discuss it.

fabpot added a commit that referenced this issue Sep 25, 2015
…ass methods (WouterJ)

This PR was squashed before being merged into the 2.8 branch (closes #15151).

Discussion
----------

[Security] Deprecated supportsAttribute and supportsClass methods

These methods aren't used at all in a Symfony application and don't make sense to use in the application. They are only used internally in the voters. This means the voter interface can be made much easier.

I'm not sure how we do these deprecations, should we remove the methods from the interface now already? Also, I don't think it's possible to trigger deprecation notices for the voter methods?

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | one of #11742
| License       | MIT
| Doc PR        | -

Abstract Voter
---

There is one remaining question about the abstract voter. This currently has abstract `getSupportedAttributes()` and `getSupportedClass()` methods. One of the reasons to remove the methods for the interface was that these methods are not flexible. Does it make sense to deprecate these methods as well and replace them by an abstract `protected vote(array $attributes, $class)` method in the `AbstractVoter` (which is called from `AbstractVoter#vote()`) ?

Commits
-------

6588708 [Security] Deprecated supportsAttribute and supportsClass methods
fabpot added a commit that referenced this issue Sep 26, 2015
This PR was merged into the 2.8 branch.

Discussion
----------

deprecated the Shell Console class

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | one of #11742
| License       | MIT
| Doc PR        | n/a

Commits
-------

1c17928 deprecated the Shell Console class
@javiereguiluz
Copy link
Member Author

Closing this issue because we're so close to Symfony 3 launch that it no longer makes sense to have this meta-issue.

@dosten
Copy link
Contributor

dosten commented Dec 20, 2015

@javiereguiluz Maybe we can create a new issue with the pending issues or rename this one to target 4.0

@javiereguiluz
Copy link
Member Author

@dosten I think it would be better to create a new issue.

xabbuh added a commit that referenced this issue Sep 12, 2017
…formatter (aitboudad)

This PR was merged into the 3.4 branch.

Discussion
----------

[Translation] added support for adding custom message formatter

| Q | A |
| --- | --- |
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | #6009, #10152, one item in #11742, #11948 |
| License | MIT |
| Doc PR | ~ |

Commits
-------

42183b0 [Translation] Support adding custom message formatter
ostrolucky pushed a commit to ostrolucky/symfony that referenced this issue Mar 25, 2018
…mmands (fabpot)

This PR was merged into the 2.8 branch.

Discussion
----------

[WebProfilerBundle] deprecated import/export commands

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | part of symfony#11742
| License       | MIT
| Doc PR        | n/a

Commits
-------

943fec9 [HtppKernel] deprecated Profiler::import/export
17e00b9 [WebProfilerBundle] deprecated import/export commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests