Skip to content

[New Rule] Do not use setTemplate in Block classes #31

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

Open
lenaorobei opened this issue Feb 1, 2019 · 5 comments
Open

[New Rule] Do not use setTemplate in Block classes #31

lenaorobei opened this issue Feb 1, 2019 · 5 comments
Labels
need to discuss Rule requires discussion new rule New feature implementation proposal New rule proposal

Comments

@lenaorobei
Copy link
Contributor

Background

When creating a Block class, a Block class could set its PHTML template in multiple ways: Through XML layout, through a call to $this->setTemplate() and through a variable $_template. The new design of Block classes is to configure them at constructor time, meaning that configuration options (like the template) are added using constructor arguments. This allows for the XML layout to change the template. The template in the Block class is then only defined as a default value, if the XML layout is not overriding the template: This default value is best defined via a protected variable $_template.

Reason

If $this->setTemplate() is used instead, this could lead to potential issues: First of all, setters are deprecated in Block classes (because constructor arguments should be preferred instead). Second, if $this->setTemplate() is added to the constructor after calling upon the parent constructor, it would undo the configuration via XML layout. Simply put: It is outdated and leads to issues quickly.

Implementation

ExtDn SetTemplateInBlockSniff.

@lenaorobei lenaorobei added proposal New rule proposal need to discuss Rule requires discussion new rule New feature implementation labels Feb 1, 2019
@lenaorobei lenaorobei added the waiting for feedback Additional explanation needed label Feb 13, 2019
@lenaorobei
Copy link
Contributor Author

Need to investigate and find Magento core use cases where this rule is not applicable.

@ldusan84
Copy link
Contributor

Hi @lenaorobei

I investigated a bit, there might be 3 scenarios in core where the rule is not applicable.

  1. Overriding setTemplate method.
  2. setTemplate in tests.
  3. setTemplate used in constructor.

All the other occurrences are rule violation and should be refactored.

Let me know what you think.

@lenaorobei lenaorobei removed the waiting for feedback Additional explanation needed label May 2, 2019
@lenaorobei
Copy link
Contributor Author

Hi @ldusan84

I can raise this question during weekly architecture discussion magento/architecture#153
Would you be interested in attending? So we can cover this topic and move forward.

@ldusan84
Copy link
Contributor

Hi @lenaorobei

Sorry for the late reply.

Yeah let's do it if it's still possible.

Thanks.

@lenaorobei
Copy link
Contributor Author

magento-devops-reposync-svc pushed a commit that referenced this issue Aug 30, 2021
MTS-2096: PHP Code Sniffer: additional functions to block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need to discuss Rule requires discussion new rule New feature implementation proposal New rule proposal
Projects
None yet
Development

No branches or pull requests

2 participants