Variable callback scope problem?

[generalProf]

Hi everyone.

I’m currently sitting with this problem in my recursive method i’m creating that will go through an array structure to create a multidimensional object which will then be used in a Vue template once processed. What this array contains is a component object which Vue uses on the template and also some other information relevant to what i’m making…etc.

Now the things that i’m having a problem with i think is called a variable callback scope problem, I will explain what is the setup and then the problem.

In my process, what happens is an ajax call collects the array and data which will be going inside the array from the backend, In the success callback of the ajax call i have a method which is outside of the ajax call, which is being called to perform the recursive processes.

The problem i’m having is the last value from my data is being set on two of the vue components, this i think is due to the async method which is ajax, i read that because it’s async the for loops i’m using set the var’s inside the loop params in the global space and then the code get’s executed once the async method is done, or something along those lines.

Example of my code:

Array format
[
   [
      [
          {
             'component': object
          }
      ]
   ]
   [
      [
          {
             'component': object
          }
      ]
   ]
]

That may look a bit weird with all the nested arrays but there is a reason for this, the system i’m making is for creating bootStrap layouts and the array is the layout itself.

The method i have that’s doing the recursive processing:

function doAllTheTHings($data)
{
    $returnArray = [];

    for(var $index in $data)
    {
        if(some conditions)
        {
            $returnArray[$index] = componentObjectData;
        }
        else if(x is not there do the recursive part)
        {
            $retyrbArray[$index] = doAllTheThings($data[$index]);
        }
    }
    
    return $returnArray;
}

Please note thins is a rough draft to what it’s really life, and there is also some things in place to make sure the recursive process does not go on forever.

Do any of you know how i can solve this problem? i need to not have the for loop duplicate / use the last data in the array for both components’s data

[generalProf]

Can anyone help me with this problem?

[mawburn]

The psuedocode you posted does not match the description of what you said might be wrong. There is no async and no callbacks.

Please post relevant code trimmed to where you think the issue lies, instead of psuedocode.

[m_hutley]

@generalProf

$returnArray[$index] = componentObjectData;

This is a fixed variable. So yes, you’ll get the same thing twice. That’s what you’ve told the code to do.

[generalProf]

The code i posted is called within a ajax callback which in itself is an async function. Sorry i didn’t make this clear, so the code i have above is just called inside the the ajax success callback.

$.ajax({
   ...
   success: function(event){
     $results = doAllTheThings(event.data);
   }
});

That is the simplest form of what is happening, i make an ajax call to get the data, then in the success callback i call the function (which is the code in the original post) and because that function is a recursive it calls itself as you know.

[generalProf]

The variable is not fixed, it’s an array that is set in the beginning of the function and the for loop has the index which increments (that is the $index) so there is no fixed variable.

As i understand it, async makes the method run and once the async method is done then the for loop has incremented the $index value to the last one in the array, because the $index var is set in the global scope, that is why in ES6 you use let because it’s block scoped.

In this post you can see what the problem is but for some reason the solutions in there is not helping me and i was wondering if someone can help me understand why this is:

How to Fix JavaScript Callbacks Variable Scope Problems | Pluralsight |...

Thank you for any help

[m3g4p0p]

@generalProf

As i understand it, async makes the method run and once the async method is done then the for loop has incremented the $index value to the last one in the array, because the $index var is set in the global scope, that is why in ES6 you use let because it’s block scoped.

The AJAX success() method being called asynchronously has nothing to do with this, and the problem described in that link doesn’t apply here either as you don’t create a closure but use the index directly when calling the function… it would only be a problem if you have an asynchronous callback inside the loop. Consider:

var data = [1, 2, 3, 4, 5]

// -> 1, 2, 3, 4, 5
for (i in data) {
  console.log(data[i])
}

// -> 5, 5, 5, 5, 5
for (i in data) {
  window.setTimeout(function () {
    // Here a closure over `i` (and `data`) is getting created, 
    // and by the time it gets called, `i` will always be 5
    console.log(data[i])
  })
}

So as far as I can tell from your pseudo code, this shouldn’t be an issue. You’re always assigning

$returnArray[$index] = componentObjectData

though – where does componentObjectData come from? Is it supposed to change at some point? If not, you’re simply adding the same object for every index. But as @mawburn said, it’s a bit hard to help you debug pseudo code…

[m_hutley]

Well, no.

I’m not talking about the LEFT side of the assignment statement, i’m talking about the RIGHT side.

You can put index as many times on the left side as you want, but your original problem statement was “It’s all the same”, and we’re pointing out to you that the right side of your statement says “Make it this static thing.”

If componentObjectData contains the word “orange”, you’re going to get an array full of the word “orange”.

[mawburn]

Where are you trying to use the results variable? And how are you waiting for the async action to finish before using it?

[generalProf]

Thank you all for the help so far and my apologize for the pseudo code.

@m3g4p0p

The AJAX success() method being called asynchronously has nothing to do with this, and the problem described in that link doesn’t apply here either as you don’t create a closure but use the index directly when calling the function… it would only be a problem if you have an asynchronous callback inside the loop. Consider:

Thank you for clearing that up for me, i was having a hard time wrapping my head around the actual cause for that problem. To answer you with regards to componentObjectData, it’s another object that get’s passed through the method and it itself has widget id, index…etc to match up with before being assigned.

I asked permission from my boss to post the code so you can see what it does. Here you go.

function widgetObjectSetter($layoutArray, $data, $widgetObject, $widgetNames)
{
    for(var index in $layoutArray) {

        // If there is a widget in this section
        // of the array then set the widget
        // name and object
        if (
            $layoutArray[index]['details'] !== undefined &&
            $layoutArray[index]['details']['widgetId'] !== undefined
        ) {
            // Getting the current widget id
            // and index for later use
            var $widgetId = $layoutArray[index]['details']['widgetId'],
                $widgetIndex = $layoutArray[index]['details']['widgetIndex'],
                $returnData = {};


            // If there is a widget object
            // then set the object
            // and the name
            if ($widgetObject[$widgetId] !== undefined) {
                // Set name and object
                $layoutArray[index]['details']['widget'] = $widgetObject[$widgetId];
                $layoutArray[index]['details']['widgetName'] = $widgetNames[$widgetId];


                // Set widget data
                if (
                    $data['userData'][$widgetId] !== undefined &&
                    $data['userData'][$widgetId]['widgetData'] !== undefined &&
                    $data['userData'][$widgetId]['widgetData'][$widgetIndex] !== undefined
                ) {
                    $returnData = $data['userData'][$widgetId]['widgetData'][$widgetIndex]['processed'];

                    $returnData['buildData'] = $data['userData'][$widgetId]['widgetData'][$widgetIndex]['buildData'];

                    $returnData['metaData'] = $data['userData'][$widgetId]['metaData'];
                }
                else if (
                    $data['stock'][$widgetId] !== undefined &&
                    $data['stock'][$widgetId]['widgetData'] !== undefined &&
                    $data['stock'][$widgetId]['widgetData'][0] !== undefined
                ) {
                    $returnData = $data['stock'][$widgetId]['widgetData'][0]['processed'];

                    $returnData['buildData'] = $data['stock'][$widgetId]['widgetData'][0]['buildData'];

                    $returnData['metaData'] = $data['stock'][$widgetId]['metaData'];
                }


                $layoutArray[index]['details']['widget']['data'] = function () {
                    return $returnData;
                };
            }
        }
        else {
            $layoutArray[index] = widgetObjectSetter($layoutArray[index], $data, $widgetObject, $widgetNames);
        }
    }


    return $layoutArray;
}

To give some idea as to what each of those params mean:
$layoutArray = an array built for vue in the frontend to construct a bootstrap layout.
$data = data from the backend which ether a user created or stock data in case there is no user data.
$widgetObject = this will be a object passed to vue in the front end that will construct the widget.
$widgetNames = does what the tin says, list of names for the widgets.

Also the arrays are already setup in the backend to have the index’s match up with the layout of the page, that is why i can use the index within the for loop on the other objects.

@m_hutley

Well, no.

I’m not talking about the LEFT side of the assignment statement, i’m talking about the RIGHT side.

You can put index as many times on the left side as you want, but your original problem statement was “It’s all the same”, and we’re pointing out to you that the right side of your statement says “Make it this static thing.”

If componentObjectData contains the word “orange”, you’re going to get an array full of the word “orange”.

Sorry out that, i see what you mean now, the code i posted just now i think will clear up everything.

@mawburn

Where are you trying to use the results variable? And how are you waiting for the async action to finish before using it?

In the new code i posted, you will see the results var is being set inside a function which will be assign into the main layout array, that will then intern be used within vue as a component object. The async wait is what i was wondering about, i thought the article talks about it. but i could be wrong, i’m not the best at JS i’m more of a php guy and over the last while learning about JS has told me that there is A LOT of small things you need to know because you’re proficient in it

Hopefully this clears up all the question, ask anything and i will answer as best as i can.

[m_hutley]

Out of curiousity, when testing this, are you using 3 test cases, one of which has an undefined details or widgetId?

[generalProf]

I’m not running testing, just taking best case to make it work then will test call cases. If you are talking about the first if statement, I do this because that’s the only why i know of to do a !empty() from php within JS, is there a different solution?

[Paul_Wilkins]

JavaScript has an easier preferred way of doing that, by testing if the property has a truthy contents instead:

if ($layoutArray[index].details && $layoutArray[index].details.widgetId) {

[Paul_Wilkins]

And if you want to make it easier to read, use the forEach method instead:

$layoutArray.forEach(widget) {
  ...
  // replace $layoutArray[index] with widget
  if (widget.details && widget.details.widgetId) {
    ...
  }
  ...
});

[generalProf]

Oh thank you so much @Paul_Wilkins for the helpful info appreciated.

Do you know why it says i can’t post more then 255 characters when posting? i can’t quote anyone without going over.

[Paul_Wilkins]

@generalProf

Do you know why it says i can’t post more then 255 characters when posting?

There hasn’t been this limit before. I’ve flagged your post to allow admins to find out more about this.

[Paul_Wilkins]

@generalProf

Do you know why it says i can’t post more then 255 characters when posting? i can’t quote anyone without going over.

An update had recently been made to the website, that normally are seamless but had a strange side-effect this time. Staff have investigated and fixed the problem, which is now no longer an issue.

[generalProf]

Thank you

[m_hutley]

So a couple of thoughts:
1: The validity/existance of widgetIndex is unchecked.
2: Have you actually inspected the return from this function? Does it look correct? (IE: Is this a Vue/display issue, or is this a function issue?)
3: Have you inspected the input to this function to ensure IT doesn’t have duplicates?
4: Definitely agree with a forEach. for...in on a numeric array can cause indexes to be referenced out of order, which may cause issues if you have order-specific code later on.

[mawburn]

i’m more of a php guy

$lol['I']['can']['tell']

I think because of that, you might be vastly overusing Arrays and trying to treat all of your JS Objects as PHP Arrays. It’s actually very rare that I’ve used a multi-dimensional array in JS. I can probably count on one hand how many times I’ve used them in the last 4 or 5 years of doing mostly JS full time.

I feel like this is adding a ton of unnecessary complexity to your code, because you’re then having to convert this back into normal JS to fit in with the Vue way of doing things.

I had to rewrite this in JS to make it readable for me. This was just too far into PHP land for me to understand at a glance and follow what was going on. I’m pretty sure this is a 1 to 1 translation, except for the overuse of arrays which exist outside of this and I can’t fix.

 function widgetObjectSetter(layoutArray, data, widgetObject, widgetNames) {
  return layoutArray.map(layout => {
    if(layout.details && layout.details.widgetId) {
      const widgetId = layout.details.widget;
      const widgetIndex = layout.details.widgetIndex;
      let returnData = {};

      if(widgetObject[widgetId]) {
        layout.details.widget = widgetObject[widgetId];
        layout.details.widgetName = widgetObject[widgetIndex];

        if(data.userData[widgetId]
          && data.userData[widgetId].widgetData
          && data.userData.widgetData[widgetIndex]) {

          // copy by reference instead of value problem here, fixed
          returnData = Object.assign({}, data.userData[widgetId].widgetData[widgetIndex].processed);

          // is this an object? if so, also needs Object.assign
          returnData.buildData = Object.assign({}, data.userData[widgetId].widgetData[widgetIndex].buildData);

          // is this an object? if so, also needs Object.assign
          returnData.metaData = Object.assign({}, data.userData[widgetId].metaData);

          // note:
          // if you're using Babel you can use the spread operator
          // const x = {...y} instead of const x = Object.assign({}, y)

          // if these are arrays for some reason, use arrName.slice() to copy by value
          // or the spread operator in Babel [...arrName]
        } else if (data.stock[widgetId] && data.stock[widgetId].widgetData && data.stock[widgetId].widgetData[0]) {
          // same issues as above
          returnData = Object.assign({}, data.stock[widgetId].widgetData[0].processed);
          returnDat.buildData = Object.assign({}, data.stock[widgetId].widgetData[0].buildData);
          returnData.metaData = Object.assign({}. data.stock[widgetId].metaData);
        }

        // not sure why you had an anonymous function here.
        // I suspect it has something to do with the copying by reference?
        layout.details.widget.data = returnData; 
        // returnData is a new object so passing by reference is ok.
        // If you want to be extra safe you can always do = Object.assign({}, returnData)
        // I do this when using Babel just because it's easy to do {...returnData}
      }
    } else {
      layout = widgetObjectSetter(layout, data, widgetObject, widgetNames);
    }
  })
}

So the only issues I can see here, is you were doing a lot of copy by reference of objects (and arrays?), which might have been causing some unintended side effects elsewhere in your code. I fixed that and made comments in the code for what I did and other alternatives.

@Paul_Wilkins was right about the .forEach, but he probably had the same problem as me and this was too hard to think of looking at it as PHP in JS. I used a .map which functions similar, but returns a new array with mutated values.

I really think you should take a step back and de-Array your data. I feel like there is no good reason why this needs to be an Array. Arrays in JS are only for sequential data, which is why multidimensional arrays are rare. The only time you really need them is when you’re trying to build a matrix or grid or something along those lines.

I also feel very uneasy about doing this recursively like this, because it’s weird to me that nested arrays would look so identical that they would fit the same, fairly complicated, logic.

The reason why I hardly ever use multi-dimensional arrays is because it’s easier to build an array of JS objects, than have an array inside of that with extra data or a relevant key.

eg:

[
  {
     "someNameOrTitleOrSomething": "heyo",
     "someArrForSomething": [...],
     "anotherArrForTotallyDifferentData": [...]
  }
]