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)