algoliasearch-magento-2 icon indicating copy to clipboard operation
algoliasearch-magento-2 copied to clipboard

Fix potential RequireJS race condition

Open michaellehmkuhl opened this issue 1 year ago • 0 comments

Summary

In some cases, RequireJS may execute the recommend.js callback before window.objectId is set in algolia/algoliasearch-magento-2/view/frontend/templates/recommend/products.phtml. When this happens, an Uncaught ReferenceError is thrown and short circuits RequireJS from firing the rest of the dependent scripts. In our case, that meant that the callback in autocomplete.js never fired, which caused the search box to not render. The error condition is also more likely when browser caching is enabled, since the recommend.js script is then more likely to be loaded from the cache and execute sooner.

To Reproduce

Since race conditions are tricky to reproduce, the assignment of the window.objectId variable can be artificially slowed down to force the error condition. One way to do this is to add wrap that assignement ina setTimeout() in view/frontend/templates/recommend/products.phtml, like so:

<script>
setTimeout(() => {
    window.objectId = "<?php echo $product->getId() ?>";
}, 5000);
</script>

Make sure to clear the Magento block and full page caches for that change to be effective.

Result

The fix wraps the recommend.js callback in a jQuery "ready" callback so it does not execute until after pageload when we can be sure the window.objectId variable exists and is set.

Console error for reference:

recommend.js:16 Uncaught ReferenceError: objectId is not defined
    at recommend.js:16:58
    at Object.execCb (require.js:1696:33)
    at Module.check (require.js:883:51)
    at Module.<anonymous> (require.js:1139:34)
    at require.js:134:23
    at require.js:1189:21
    at each (require.js:59:31)
    at Module.emit (require.js:1188:17)
    at Module.check (require.js:938:30)
    at Module.enable (require.js:1176:22)

Additional Notes

It may be wise to choose a more unique variable name than objectId to avoid potential collisions with other third party code down the road.

This PR purposely avoided reformatting the indentation for clarity of the actual change.

michaellehmkuhl avatar Sep 29 '22 15:09 michaellehmkuhl