1

I have been trying to understand why my js code which was inside the HTML is not working now that it is in an external file. English is not my first language which makes me even harder to understand (and spanish results haven't been quite productive as I hoped).

I literally copy-paste the code to the other file and placed <script src="core.js"></script> to get the js loaded but both Firefox and Chrome keep telling me "TypeError: eng is undefined" when it is.

Before anyone tells me, it was already responded to in other questions, yes, I saw it was. But I don't understand anything so if anyone could explain me like "JS for dummies" would be REALLY appreciated.

The code is:

//SETS "LOCALE" "EN" TO REDIRECT TO ENGLISH
(function (){
  'use strict';

 var eng = document.querySelector('.eng')

  function setLocalStorage(){
     eng.addEventListener('click', () => {
      localStorage.setItem('locale','en')
    })
  }
  setLocalStorage()

}());

//SETS "LOCALE" "ES" TO REDIRECT TO SPANISH
(function (){
  'use strict';

  var esp = document.querySelector('.esp')

  function setLocalStorage(){
     esp.addEventListener('click', () => {
      localStorage.setItem('locale','es')
    })
  }
  setLocalStorage()

}());

//GETS THE "LOCALE" AND REDIRECT
(function () {
  'use strict';

  const locale = localStorage.getItem('locale');

  if (locale === 'en') {
    window.location = 'eng.html';

  } else if (locale === 'es') {
    window.location = 'esp.html';

  } else {
    // first visit:
    document
      .querySelector('.eng')
      .addEventListener('click', () => {
        localStorage.setItem('locale', 'en')
      });

    document
      .querySelector('.esp')
      .addEventListener('click', () => {
        localStorage.setItem('locale', 'es')
      });    
  }

}());
Gaurav Mall
  • 2,372
  • 1
  • 17
  • 33
itu
  • 15
  • 5
  • Where is this script in relation to the rest of the HTML? – Sebastian Simon May 28 '20 at 17:34
  • I think you should defer the loading of the script responsible for this js code if not already done so. – Lakshya Thakur May 28 '20 at 17:35
  • @user4642212 the this script basically identifies two CTAs and tells them to add to localStorage the values "locale", "en" or "es" so when one of those is clicked, the page from now on, whenever the index is loaded, it will take you straight to the chosen language. Is this what you asked? – itu May 28 '20 at 17:37
  • Most likely a duplicate of [Why does jQuery or a DOM method such as getElementById not find the element?](https://stackoverflow.com/q/14028959/4642212). I see your [previous question](https://stackoverflow.com/a/61982882/4642212) has a CodePen link in the answers and comments. Please be aware of this: [Why does this simple JSFiddle not work?](https://stackoverflow.com/q/7043649/4642212). – Sebastian Simon May 28 '20 at 17:37
  • Is your script runned before or after the body of html being loaded? – innis May 28 '20 at 17:38
  • @itu _“sorry, what”_ — [`defer`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-defer) your ` – Sebastian Simon May 28 '20 at 17:39
  • @innis uh I haven't set such thing – itu May 28 '20 at 17:40
  • @user4642212 ohhh ok, I'll test it out – itu May 28 '20 at 17:40
  • @itu _“I haven't set such thing”_ — Yes, you have, because you _“literally copy-paste the code to the other file”_. So _where_ did you put your ` – Sebastian Simon May 28 '20 at 17:42
  • @user4642212 ok, so now the script seems to be working, but the console still throws "TypeError: eng is undefined". Why is it working if the console shows an error? – itu May 28 '20 at 17:44
  • @user4642212 oh ok, I thought it had to do with something like preload. I placed it in the head, below the CSS and font links. – itu May 28 '20 at 17:44

2 Answers2

2

This line of JavaScript:

var eng = document.querySelector('.eng')

is trying to access an element on the HTML page, which probably looks like this:

<div class="eng">english text here</div>

In order for the JavaScript code to find that HTML element, the page must already be loaded before the JavaScript line of code runs. If not, the document.querySelector('.eng') expression will return null.

To improve the chances that the HTML will be loaded and available before the JavaScript runs, it is a good practice to put your scripts at the bottom of the <body> element. Something like this:

<body>
  <div class="eng">english text</div>
  <script src="myjavascript.js"></script>
<body>

But that's not a complete guarantee. To further improve your chances, you can add the defer attribute to the script tag, which prevents the script from running until after the page has loaded.

<script src="myjavascript.js" defer></script>

Issue #2 - Hoisting

There's a further problem with the JavaScript code that can cause an error. This has to do with "hoisting", where variables and function declarations are "hoisted" to the top of their blocks.

This JavaScript involves some hoisting:

function () {
  'use strict';

  var eng = document.querySelector('.eng');

  function setLocalStorage () {
    eng.addEventListener('click', () => {
      localStorage.setItem('locale', 'en');
    });
  }

  setLocalStorage();

}

Because of hoisting, it's actually doing this:

function () {

  // declare 'eng' as undefined
  var eng; 

  // declare setLocalStorage
  function setLocalStorage () {

    // JavaScript interpreter says during declaration:
    // "TypeError: I don't know what 'eng.addEventListener' is"

    eng.addEventListener('click', () => {
      localStorage.setItem('locale', 'en');
    });
  }

  // set 'eng'
  eng = document.querySelector('.eng');

  // now that 'eng' is set, call setLocalStorage
  setLocalStorage();

}

The fix - don't use an unset variable in a hoisted function. This code does the same thing, without any hoisting problems:

(function () {
  var eng = document.querySelector('.eng');

  eng.addEventListener('click', () => {
      localStorage.setItem('locale', 'en');
    });
})();
terrymorse
  • 6,771
  • 1
  • 21
  • 27
  • I have both added the script at the end of my body element and set defer, still I get the same issue... It works, but the console still tells me "TypeError: eng is undefined" – itu May 28 '20 at 17:47
  • @itu You're seeing that TypeError because of "hoisting". `var eng` is declared at the beginning of the function, before it is set, and since `setLocalStorage` accesses `eng.addEventListener`, it throws an error. I'll explain further in my answer above. – terrymorse May 28 '20 at 18:05
1

Actually according to your code you are accessing the DOM element and further you are calling them to do some task. But suppose you are trying to access some unavailable DOM content and then telling them to do something. In that case if you are trying to access some DOM but javascript couldn't find that it will return undefined. That's why you'll get undefined. (i'm trying to explain it easily)

Please make sure you have some elements with the class name eng and esp.

So, to overcome this case you need to check if you can access them and then give some task to do.

Suppose i'm checking here if eng is present there and then assigning some task to it.

eng && eng.addEventListener('click', () => {
    localStorage.setItem('locale','en')
})

Full Code is here.

//SETS "LOCALE" "EN" TO REDIRECT TO ENGLISH
(

    function (){
      'use strict';

     var eng = document.querySelector('.eng')

      function setLocalStorage(){
         eng && eng.addEventListener('click', () => {
          localStorage.setItem('locale','en')
        })
      }
      setLocalStorage()

    }());

    //SETS "LOCALE" "ES" TO REDIRECT TO SPANISH
    (function (){
      'use strict';

      var esp = document.querySelector('.esp')

      function setLocalStorage(){
         esp && esp.addEventListener('click', () => {
          localStorage.setItem('locale','es')
        })
      }
      setLocalStorage()

    }());

    //GETS THE "LOCALE" AND REDIRECT
    (function () {
      'use strict';

      const locale = localStorage.getItem('locale');

      if (locale === 'en') {
        window.location = 'eng.html';

      } else if (locale === 'es') {
        window.location = 'esp.html';

      } else {
        // first visit:
        document
          .querySelector('.eng')
          .addEventListener('click', () => {
            localStorage.setItem('locale', 'en')
          });

        document
          .querySelector('.esp')
          .addEventListener('click', () => {
            localStorage.setItem('locale', 'es')
          });    
      }

    }());
Sifat Haque
  • 5,357
  • 1
  • 16
  • 23
  • `eng && eng.addEventListener` isn’t really best practice. Use `if` instead and ensure that your DOM is loaded. – Sebastian Simon May 28 '20 at 17:48
  • This... Actually worked! No console errors and it works smooth! Awesome! Thank you very much! And everyone who came immediately to help! – itu May 28 '20 at 17:50