MAGE-6 Fix: recommend.js get null from script when script loading bef…

[sgeleon]

Hi.

I have seen your solution for when the configuration doesn't loading.

I think it’s not good because we can get other bugs if we use this module in other modules because we don’t have any guarantee that we will get object not null, for example:

require('Algolia_AlgoliaSearch/js/recommend.js',

function(recommend) {

    // recommend -> this is object or null ???

})

I think you should add some checker for it.

define([
    'jquery',
    'algoliaBundle',
    'recommend',
    'recommendJs',
    'recommendProductsHtml',
    'domReady!'
],function ($, algoliaBundle, recommend, recommendJs, recommendProductsHtml) {
    'use strict';

    return function (config, element) {

        function checkerConfigLoading() {
            if (typeof algoliaConfig === 'undefined')
                setTimeout(checkerConfigLoading, '500')
            else
                exec()
        }

        function exec() {

           ....

      }
});

[damcou]

Hello @sgeleon , thanks for bringing this to our attention.
I wanted to review the changes, but the PR targeted the develop branch which we don't use anymore, would it be possible for you to provide the same changes targeting the algolia:release/3.14.0-beta branch since retargetting this one leads to some conflicts? Thanks in advance.

[cammonro]

@sgeleon This seems like potential for an endless loop. I'd be more interested in why algoliaConfig object doesn't load when expected so we can be sure to properly load the dependency. Can you please provide the steps to recreate this scenario?

FWIW the underlying code is a bit old. The legacy use of the global window object for dependencies that we are trying to ensure load through RequireJS creates some challenges for us. It is not ideal and may be refactored in the future but we want to do it right. The more input you guys give us, the more it helps us to nail this stuff down! Feedback is welcome. 🙂

[sgeleon]

Hello @cammonro.

This seems like potential for an endless loop.

No, because RequireJS executes this script only once when loading require.js. The loop stops when the value is defined.

I'd be more interested in why algoliaConfig object doesn't load when expected so we can be sure to properly load the dependency. Can you please provide the steps to recreate this scenario?

In your setup, we don't see algoliaConfig. This value is not managed by RequireJS.

define([
    'jquery',
    'algoliaBundle',
    'recommend',
    'recommendJs',
    'recommendProductsHtml',
    'domReady!'
],function ($, algoliaBundle, recommend, recommendJs, recommendProductsHtml) {

In this case, we check the value of algoliaConfig, which is not controlled by RequireJS. This value might not be defined during script execution.

    if (typeof algoliaConfig === 'undefined') {
        return;
    }

[sgeleon]

Hello @sgeleon , thanks for bringing this to our attention.
I wanted to review the changes, but the PR targeted the develop branch which we don't use anymore, would it be possible for you to provide the same changes targeting the algolia:release/3.14.0-beta branch since retargetting this one leads to some conflicts? Thanks in advance.

Hello @damcou.

I created a new request.

#1624