search

fetchify-dev / magento2

Fetchify Magento 2 Integration
https://fetchify.com
3 stars 6 forks source link

- removed setInterval as there is no clearInterval #9

Closed valguss closed 5 years ago

valguss commented 6 years ago

This keeps running even after the module has initiated. It also caused double initialisation

GaborSuranyi commented 6 years ago

The function called here first checks if the form in question already has the functionality applied or not. Do let me know if you can reproduce any actual errors, or double initialisation; it should not happen at all.

As a general rule of thumb with our extensions, we try to avoid having any interval checks in. While it does seem like that the current version of Magento does not require this kind of loop to keep the integration active, this line is part of the extension's code since Magento 2 beta. I'd need to verify whether or not all versions of Magento 2 keep the forms intact, when going back and forth on the checkout. What the code is supposed to catch, is cases where Magento rebuilds the form from scratch within the same page.

I'll try to go over all major supported versions to confirm whether or not we can merge this change in. The fact that on M2.2 this code is definitely not required gives me a bit of hope.

GaborSuranyi commented 6 years ago

I confirmed last week that the removal of setInterval creates a race condition against Magento2 loading the checkout page. If you have applied this code to your own shop, I would highly suggest reverting it, as it will make the extension fail if your shop is under heavy load.

I also double checked, and there are no double initialisation, and the interval will not cause any issues. But it does allow Magento & many checkout extensions to dynamically reload the address forms, also to load shipping & billing forms at separate times.

The callback function in the interval is called with a significant delay (200ms) and exits swiftly if there are no address forms where the code needs to be applied. 200ms is an ideal midpoint between reacting to a re-appearing address form before the user interacts with the form, and not packing too many things to the JS message queue. This should result in the code having no visible performance effects at all, even on mobile devices.