1

I have two Javascript modules, one for my navigation, and one for route changes.

Inside of nav.js is simple if to check if the menu can be opened

const initNav = () => {
  const openMenu = () => {
    if (!menuIsOpen && isMobile) {
      navItems.classList.add("is-menu-open");
      menuIsOpen = true;
    } else {
      navItems.classList.remove("is-menu-open");
      menuIsOpen = false;
   }
 }
}

initNav();

export default initNav;

Then, at the top of the other module I import

import initNav from './nav.js';

The question is, inside of my other module route.js, I need to be able to check if the menu is opened, and, if it is then close it, so I was going to use:

 beforeLeave: function (data) {
    if (menuIsOpen) {
      navItems.classList.remove("is-menu-open");
      menuIsOpen = false;
    }
 }

The console, however, says menuIsOpen is not defined.

I can't then this way check to see the status of this variable.

Am I able to do this another way, rather than combining the two modules into one very large js module?

cmp
  • 420
  • 5
  • 22

3 Answers3

3

The problem is that you're trying to reassign a variable that exists in multiple modules, but imported variables are read-only references - they can only be reassigned in their original module. So you can't just also export the menuIsOpen from nav.js because reassigning it in route.js would not be allowed.

You could have nav.js export a function that reassigns its menuIsOpen while also exporting menuIsOpen, but reassignable exports in general are unintuitive - nowhere else in Javascript does what looks like a reference to a primitive value seemingly reassign itself due to a different module reassigning the binding. Linters often forbid them.

A better alternative is for the main user of menuIsOpen to export setter and getter functions to interface with menuIsOpen. For example, you could do something like the following:

// nav.js
export const getMenuIsOpen = () => menuIsOpen;
export const setMenuIsOpen = (newVal) => menuIsOpen = newVal;
// ...
// route.js
import initNav, { getMenuIsOpen, setMenuIsOpen } from './nav.js';
// ...
beforeLeave: function (data) {
  if (getMenuIsOpen()) {
    navItems.classList.remove("is-menu-open");
    setMenuIsOpen(false);
  }
}

For anything more complicated than a single variable that needs to be accessed and changeable in multiple places, you can consider exporting an object instead, eg:

// nav.js
export const navState = {
  menuIsOpen: false,
  // other properties
};
// ...
// route.js
import initNav, { navState } from './nav.js';
// ...
beforeLeave: function (data) {
  if (navState.menuIsOpen) {
    navItems.classList.remove("is-menu-open");
    navState.menuIsOpen = false;
  }
}

Having to handle mutable state across multiple modules makes code a bit ugly. I prefer to avoid it when I can. Given the code in the question, since you already have a reference to navItems in both modules, you might be able to avoid having a persistent menuIsOpen variable at all if you simply check if the navItems class list has is-menu-open:

// nav.js
// ...
const openMenu = () => {
  // defining menuIsOpen as a standalone variable isn't necessary,
  // but you might find it makes the code more readable
  const menuIsOpen = navItems.classList.contains('is-menu-open');
  if (!menuIsOpen && isMobile) {
    navItems.classList.add("is-menu-open");
  } else {
    navItems.classList.remove("is-menu-open");
 }
}
// ...
// route.js
// ...
beforeLeave: function (data) {
  const menuIsOpen = navItems.classList.contains('is-menu-open');
  if (menuIsOpen) {
    navItems.classList.remove("is-menu-open");
  }
}

If you do need a standalone menuIsOpen variable in nav.js, you could have nav.js export a function which can close the menu and set menuIsOpen to false:

// nav.js
// ...
export const closeMenu = () => {
  navItems.classList.remove("is-menu-open");
  menuIsOpen = false;
};
// ...
// route.js
import initNav, { closeMenu } from './nav.js';
// ...
beforeLeave: closeMenu

(if beforeLeave really isn't doing anything else in your actual code other than checking if the class exists and removing it if so, you don't need to check if the class exists first - you can just remove it from the class list unconditionally - this can apply to all the other snippets in the answer too)

CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
0

try getter/setter or export the variable menuIsOpen

-1

Try like this

 beforeLeave: (data) => {
    if (menuIsOpen) {
      navItems.classList.remove("is-menu-open");
      menuIsOpen = false;
    }
 } 
Yaseer
  • 506
  • 2
  • 14
  • What did you change and why? – Bergi Jan 30 '20 at 08:18
  • beforeLeave: (data) => {} I changed the normal function to arrow function and **menuIsOpen** is outside of the function (Which mean it used as global variable), So the normal function it wan't recognize. – Yaseer Jan 30 '20 at 10:42
  • Changing to an arrow function doesn't affect anything. And the `menuIsOpen` variable already was global (not declared in that function) in the OPs code? – Bergi Jan 30 '20 at 10:47