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

[FrameworkBundle] Deprecate renderView() in favor of renderTemplate()#36184

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:masterfromjaviereguiluz:deprecate_view
May 5, 2020

Conversation

javiereguiluz
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?no
Deprecations?yes
Tickets-
LicenseMIT
Doc PR-

jvasseur reacted with thumbs up emoji
@stofstof modified the milestones:5.0,nextMar 24, 2020
@stof
Copy link
Member

@nicolas-grekas I reverted the milestone. We cannot introduce a new deprecation in 5.0.x

nicolas-grekas reacted with thumbs up emoji

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Do we consider this is worth the migration cost?

@@ -241,20 +241,30 @@ protected function denyAccessUnlessGranted($attributes, $subject = null, string
* Returns a rendered view.
*/
protected function renderView(string $view, array $parameters = []): string
{
@trigger_error('Since symfony/framework-bundle 5.1: "renderView()" method is deprecated. Use "renderTemplate()" instead.', E_USER_DEPRECATED);

Choose a reason for hiding this comment

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

trigger_deprecation() + missing annotation

Choose a reason for hiding this comment

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

oh also: changelog+upgrade files should be updated

@javiereguiluz
Copy link
MemberAuthor

About the migration cost ... I would never change therender() method becauseeverybody uses it ... butrenderView() is much less popular (in my experience and from what I've seen out there: render() is used more than 100 times more often that renderView() ... but this is only my experience).

@dmaicher
Copy link
Contributor

About the migration cost ... I would never change therender() method becauseeverybody uses it ... butrenderView() is much less popular (in my experience and from what I've seen out there: render() is used more than 100 times more often that renderView() ... but this is only my experience).

Seems about right. Not exactly a factor of 100 but here some numbers from my biggest Symfony 4.4 work project:

$ grep -r --include *Controller.php  'this->render(' src/ vendor/ | wc -l180$ grep -r --include *Controller.php  'this->renderView(' src/ vendor/ | wc -l14

@michaljusiega
Copy link
Contributor

Another point. MVC division: Model, view, controller. Most programmers associate with such an approach that the controller generates a view, not a template.

@@ -242,7 +242,7 @@ protected function denyAccessUnlessGranted($attributes, $subject = null, string
*/
protected function renderView(string $view, array $parameters = []): string
{
@trigger_error('Since symfony/framework-bundle 5.1: "renderView()" method is deprecated. Use "renderTemplate()" instead.', E_USER_DEPRECATED);
@trigger_deprecation('Since symfony/framework-bundle 5.1: "renderView()" method is deprecated. Use "renderTemplate()" instead.', E_USER_DEPRECATED);

Choose a reason for hiding this comment

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

s/@//

Choose a reason for hiding this comment

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

well actually that's broken, you'll figure out :)

Choose a reason for hiding this comment

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

(don't miss the annotation also)

@@ -241,20 +241,30 @@ protected function denyAccessUnlessGranted($attributes, $subject = null, string
* Returns a rendered view.
*/
protected function renderView(string $view, array $parameters = []): string
{
trigger_deprecation('Since symfony/framework-bundle 5.1: "renderView()" method is deprecated. Use "renderTemplate()" instead.', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

The "renderView()" method [...]?

@@ -241,20 +241,30 @@ protected function denyAccessUnlessGranted($attributes, $subject = null, string
* Returns a rendered view.
*/
protected function renderView(string $view, array $parameters = []): string
{
trigger_deprecation('Since symfony/framework-bundle 5.1: "renderView()" method is deprecated. Use "renderTemplate()" instead.', E_USER_DEPRECATED);

Choose a reason for hiding this comment

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

that's not the signature of the function, and the annotation is missing

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(just to confirm this needs a few more hops)

@fabpot
Copy link
Member

Thank you@javiereguiluz.

@fabpot
Copy link
Member

Actually, that's a hard BC break. What if one already has arenderTemplate() method in their controllers? ... well, it happens that EasyAdmin does this, and it is broken now. Shall we revert?

@nicolas-grekas
Copy link
Member

Sure, let's revert if it needs more thoughts.

@javiereguiluz
Copy link
MemberAuthor

@fabpot I hadn't thought about that ... but then, we'll never be able to add any new method to AbstractController ... because someone may have already defined it 🤔

@fabpot
Copy link
Member

@javiereguiluz That's correct :(

@fabpot
Copy link
Member

reverted

fabpot added a commit that referenced this pull requestMay 5, 2020
…vor of renderTemplate() (javiereguiluz)"This reverts commitb494beb, reversingchanges made tob9d4149.
@javiereguiluz
Copy link
MemberAuthor

javiereguiluz commentedMay 5, 2020
edited
Loading

A possible solution would be to deprecate defining customrenderTemplate() methods and adding it in Symfony 6.0:

publicfunctionrenderView(...){if (method_exists($this,'renderTemplate')) {        @trigger_deprecation('            You define a custom renderTemplate() method in your controllers,            but Symfony will add that method in Symfony 6.0, so you should            rename your method.');    }// ...}

But then we could only deprecaterenderView() in Symfony 6 and remove it in Symfony 7 (to be released in November 2023). Maybe the simplest solution is to keep therenderView() and that's it.

@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stofstofstof left review comments

@ro0NLro0NLro0NL left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas requested changes

@fabpotfabpotfabpot approved these changes

@xabbuhxabbuhxabbuh approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

9 participants
@javiereguiluz@stof@dmaicher@michaljusiega@fabpot@nicolas-grekas@ro0NL@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp