irrelevant products variable outside of scope

[gitmrwick]

learning-area/javascript/apis/fetching-data/can-store/can-script.js

Line 2 in 71ed5ea

var products;

Hello, I am learning javascript via various places, one being MDN. They link here to the can-script.js file and there I see there is a bit of a scope issue which has thrown me off a bit.

When putting code like this in a file and running it, this script wide variable products is actually never assigned to outside of the fetch call, therefore it is completely unnecessary to declare it here. This leads to confusion because when trying to use this variable again after the initialise() function, it is undefined.

Either declare it inside the fetch and remove from the beginning of the file or show an example where fetch assigns to a variable outside its scope.

[chrisdavidmills]

Hi there @gitmrwick !

I pondered this one for a while, then I remembered a weird bit of JS behaviour that is probably what is causing the confusion here.

Quoting from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/var#Description:

Assigning a value to an undeclared variable implicitly creates it as a global variable (it becomes a property of the global object) when the assignment is executed.

So if you remove the declaration of products from the top of the code, the next time we meet it is on line 10. At that point, we are assigning a variable to it without declaring it, so it gets raised up to being a global variable. Which is why everything seeming still works.

But if you try properly declaring it on line, i.e. change line 10 to let products = json; , you'll see that it stops working.

Having undeclared variables is a bad idea for a number of reasons, so you should avoid it.

[gitmrwick]

Hello, thanks for the quick response.

This so-called global variable that is declared but not initialised is not available at, say, line 191, it continues to be undefined. Thus the only actual value that gets assigned to it is in the non-global scope of the anonymous function of the promise of the fetch. If there is an assignment during declaration then that initial value is what would be available on line 191.

For my purposes, and perhaps to reduce or eliminate this confusion for anyone else, I would prefer that the json is passed in a an argument to initialise() and this argument then gets assigned at line 38 to the local finalGroup variable. Eliminating the need for global variables in general is extremely good practice.

The scope of everything here is also obfuscated (at least for me) by the chained promises (a method I am coming to prefer as I learn more about fetch.) The scope of a declared variable inside the anonymous function that calls initialise() does not carry to the new function, since the initialise function is defined outside the anonymous function, OK - but a variable declared outside of everything that gets assigned to inside this anonymous function does carry its assignment through the call and then when exiting initialise() on line 190 this scope somehow is no longer valid.

A condensed version of what I prefer, if you agree, I am happy to make a pull request:

https://gist.github.com/gitmrwick/a19fee790f6af240db69f91555ab3300

[gitmrwick]

Just realised something while looking closer at my debugging: the debug statement of the global variable after the initialise function (thus outside of the local scope) comes to the console before the debug statements in the fetch() or the initialise() - so perhaps what is happening here has less to do with scope and more to do with interpreter order. The fetch is performed after the rest of the script.

[chrisdavidmills]

so perhaps what is happening here has less to do with scope and more to do with interpreter order.

Hrm, perhaps, but I still think your gist suggestion seems like better practice than what I am doing. I've not looked at this code for a while, but I suspect I was probably using a global here to try to keep things simpler for beginners ... but it isn't really.

I'd be happy to examine/test a PR if you are happy to spend the time submitting one. I do wonder whether it would also require some changes to the selectCategory() function — this also has products used in it.

Thanks for taking the time to explore this.

[gitmrwick]

OK, PR made and tested, it looks good to me. Hopefully this will make things more clear for future learners

#181

It is a simple change, but it helps me quite a bit to focus just on fetch!