10000 [TwigBridge] fix FormExtension::$renderer bc break by fbourigault · Pull Request #20710 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[TwigBridge] fix FormExtension::$renderer bc break #20710

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

Closed
wants to merge 1 commit into from
Closed

[TwigBridge] fix FormExtension::$renderer bc break #20710

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[TwigBridge] fix FormExtension::$renderer bc break 
    Before b515702, the FormExtension::$renderer property was not
declared but initialized in FormExtension::__construct(). This makes the
property visibility to public. In b515702, the property was declared but
with a private visibility. This broke the backward compatibility of the
FormExtension class.
  • Loading branch information
Fabien Bourigault committed Dec 1, 2016
commit 01120bfc439dd9ec094601fda85e6b37656fb17e
5 changes: 4 additions & 1 deletion src/Symfony/Bridge/Twig/Extension/FormExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
*/
class FormExtension extends \Twig_Extension implements \Twig_Extension_InitRuntimeInterface
{
private $renderer;
/**
* Make this property private in 4.0.
Copy link
Member

Choose a reason for hiding this comment

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

please mark it as @internal though, so that IDEs warn users

Copy link
Member

Choose a reason for hiding this comment

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

btw, no need to mark it as private in 4.0, as it will be removed.

*/
public $renderer;
Copy link
Contributor
@greg0ire greg0ire Dec 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe a better way might be to keep it private and use a magic getter to throw a deprecation notice? I know, magic is bad, but deprecation notices might outweight this?

Copy link
Contributor Author
@fbourigault fbourigault Dec 1, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I like the idea but how to avoid access to properties other than $renderer in a nice way? Throwing a RuntimeException? And what about __isset and __unset ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd indeed throw an exception. And yeah, __isset and __unset would have to be implemented, so… not sure it's worth the hassle.


public function __construct(TwigRendererInterface $renderer = null)
{
Expand Down
0