magento-coding-standard icon indicating copy to clipboard operation
magento-coding-standard copied to clipboard

Replacing `$this` with `$block` in templates is not universally safe / correct

Open fredden opened this issue 2 years ago • 5 comments

Preconditions

  1. In a template (.phtml file), make use of $this->helper(SomeClass:class);. For example, https://github.com/magento/magento2/blob/2.4.4/app/code/Magento/Catalog/view/frontend/templates/product/listing.phtml#L20
  2. Magento Coding Standard version 25

Steps to reproduce

  1. Run phpcbf --standard=Magento2 on template

Expected result

  1. Template works the same as before auto-fixes are applied.

Actual result

  1. Exception is throw: Invalid method Vendor\Module\Block\SomeBlockClass\Interceptor::helper

fredden avatar Jun 28 '22 12:06 fredden

Hi @fredden. Thank you for your report. To speed up processing of this issue, make sure that you provided sufficient information.

Add a comment to assign the issue: @magento I am working on this


m2-assistant[bot] avatar Jun 28 '22 12:06 m2-assistant[bot]

@fredden have a look at https://github.com/magento/magento-coding-standard/issues/156

Nuranto avatar Jun 29 '22 07:06 Nuranto

@Nuranto thanks for your feedback.

I have no problem with the warning / message that says that helpers are deprecated / should not be used. I have no problem with the warning / message that says that one should use $block->getThing() not $this->getThing() in templates. I have no problem with the auto-fix of $this->getThing() to $block->getThing().

This bug is for when $this->helper() gets replaced automatically with $block->helper(), which results in a broken template. (The automatic replacement of $this->getThing() with $block->getThing() is fine and the template works just fine with this change.)

fredden avatar Jun 29 '22 10:06 fredden

I understand, and agree but my point was that they probably won’t do anything about it since they consider this is a not a correct approach.

For my projects, I simply run the fixer with that rule excluded. But of course that means it won’t fix other $this cases..

Nuranto avatar Jun 29 '22 10:06 Nuranto

That's what we've had to do as well (remove/exclude this rule from the fixer).

fredden avatar Jun 29 '22 10:06 fredden