yireo / Yireo_GoogleTagManager2

Open Software License 3.0
148 stars 76 forks source link

Add return to TriggerViewSearchResultDataLayerEvent plugin #214

Closed TommyChausson closed 9 months ago

TommyChausson commented 9 months ago

This plugin should return a result instead of null, can cause issues on ajax layered navigations

ssx commented 9 months ago

I've had to debug and fix this today and it was a right pain in the arse.

This is how I've patched it: https://gist.github.com/ssx/e055ef75a4ba39aada3463c955a944b6

Do you want a PR with this approach?

jissereitsma commented 9 months ago

First of all, thank you all for chiming in. This class was added by @gaeldelmer months ago and I never needed it myself, so didn't spot any issue. Indeed, the return value is missing in the plugin, which is causing things.

Unfortunately, this PR can't be accepted either because mixed is not supported in PHP 7.4 (and yes, OMG, this extension still needs to support 7.4).

Another thing to add here is that adding PHP typing here is what I would love to do, but the original class Magento\CatalogSearch\Controller\Result\Index that is intercepted and the DI plugin interceptor architecture itself is (still) not supporting PHP typing either, so it seems easier and quicker to just add the return without any type hinting. Making the type hints more generic (from Index to Action parent) or renaming variables doesn't add much. I'll keep it simple: https://github.com/yireo/Yireo_GoogleTagManager2/commit/7a166eb002b108ee893cd4e5e1f5e586e1eeadbd