Search code examples
javascriptwebpackscopebabeljsecmascript-5

My function not working outside of another function scope even though I have imported it


I'm trying to figure out whats going on with my code down below, I have imported my function using import { getTodos } from './todos' into another file called views.js. I'm trying to call getTodos in the top level of views.js, but I get an error that it's not a function (it's undefined). But once I try using getTodos inside a function in the same module, it suddenly works!? Why is it suddenly no longer undefined, and how can I fix the code so that it's defined where I want it to be?

My todos.js file which includes defining getTodos function:

import uuidv4 from 'uuid/v4'
import { renderTodos } from './views'

let todos = []

const loadTodos = () => {
    const todosJSON = localStorage.getItem('todos')
    try {
        return todosJSON ? JSON.parse(todosJSON) : []
    } catch (e) {
        return []
    }
}


todos = loadTodos()

const getTodos = () => todos


const saveTodos = () => {
    localStorage.setItem('todos', JSON.stringify(todos))
}


const createTodo = (text) => {

    if (text.length > 0) {
        todos.push({
            id: uuidv4(),
            text,
            completed: false
        })
        saveTodos()
        renderTodos()
    }

}

const removeTodo = (id) => {
    const todoIndex = todos.findIndex((todo) => todo.id === id)

    if (todoIndex > -1) {
        todos.splice(todoIndex, 1)
        saveTodos()
    }
}

const toggleTodo = (id) => {
    const todo = todos.find((todo) => todo.id === id)

    if (todo) {
        todo.completed = !todo.completed
        saveTodos()
    }
}



export { loadTodos, getTodos, createTodo, removeTodo, toggleTodo }


My views.js file which Im trying to load getTodos in it then use it:

import { getTodos, saveTodos, toggleTodo, removeTodo  } from './todos'
import { getFilters } from './filters'

const todos = getTodos()
const renderTodos = () => {

    const filters = getFilters()
    const todosEl = document.querySelector('#todos')

    const filteredTodos = todos.filter((todo) => {
        const searchTextMatch = todo.text.toLowerCase().includes(filters.searchText.toLowerCase())
        const hideCompletedMatch = !filters.hideCompleted || !todo.completed

        return searchTextMatch && hideCompletedMatch
    })

    const incompleteTodos = filteredTodos.filter((todo) => !todo.completed)

    todosEl.innerHTML = ''
    todosEl.appendChild(generateSummaryDOM(incompleteTodos))

    if (todos.length > 0) {
        filteredTodos.forEach((todo) => {
            todosEl.appendChild(generateTodoDOM(todo))
        })
    } else {
        const emptyMessage = document.createElement('p')
        emptyMessage.textContent = 'No to-dos to show, go ahead and add some!'
        emptyMessage.classList.add('empty-message')
        todosEl.appendChild(emptyMessage)
    }
}


const generateTodoDOM = (todo) => {

    const todoEl = document.createElement('label')
    const containerEl = document.createElement('div')
    const checkbox = document.createElement('input')
    const removeButton = document.createElement('button')
    const span = document.createElement('span')


    // Setup Container
    todoEl.classList.add('list-item')
    containerEl.classList.add('list-item__container')
    todoEl.appendChild(containerEl)

    // Setup Remove Button
    removeButton.textContent = 'remove'
    removeButton.classList.add('button', 'button--text')
    removeButton.addEventListener('click', () => {
        removeTodo(todo.id)
        renderTodos()
    })
    span.textContent = todo.text


    // Setup Checkbox
    checkbox.setAttribute('type', 'checkbox')
    checkbox.checked = todo.completed
    checkbox.addEventListener('change', (e) => {
        toggleTodo(todo.id)
        renderTodos()
    })
    containerEl.appendChild(checkbox)


    containerEl.appendChild(span)
    todoEl.appendChild(removeButton)

    return todoEl
}

const generateSummaryDOM = (incompleteTodos) => {
    const summary = document.createElement('h2')
    summary.classList.add('list-title')
    const plural =  incompleteTodos.length === 1 ? '' : 's'
    summary.textContent = `You have ${incompleteTodos.length} todo${plural} left`
    return summary
}

// Make sure to set up the exports

export { generateSummaryDOM, renderTodos, generateTodoDOM }

index.js file:

import { createTodo } from './todos'
import { renderTodos } from './views'
import { setFilters } from './filters'

// Render initial todos

renderTodos()

// Set up search text handler

document.querySelector('#search-todos').addEventListener('input', (e) => {
    setFilters({
        searchText: e.target.value
    })
    renderTodos()
})

// Set up checkbox handler

document.querySelector('#hide-completed').addEventListener('change', (e) => {
    setFilters({
        hideCompleted: e.target.checked
    })
    renderTodos()
})

// Set up form submission handler

document.querySelector('#add-todo').addEventListener('submit', (e) => {
    e.preventDefault()
    const text = e.target.elements.addTodoText.value.trim()
    createTodo(text)
    e.target.elements.addTodoText.value = ''
})

The errors in the chrome debugger console im getting are:

views.js:4 Uncaught TypeError: (0 , _todos.getTodos) is not a function
    at Object../src/views.js (views.js:4)
    at __webpack_require__ (bootstrap:19)
    at Object../src/todos.js (todos.js:2)
    at __webpack_require__ (bootstrap:19)
    at Object../src/index.js (index.js:1)
    at __webpack_require__ (bootstrap:19)
    at Object.0 (bundle.js:20254)
    at __webpack_require__ (bootstrap:19)
    at bootstrap:68
    at bootstrap:68

I'm just trying to understand why this strange behavior is happening, because once I use getTodos inside renderTodos it actually works and I'm not getting any errors, like this:

const renderTodos = () => {
    const todos = getTodos()
    // Other stuffs
}

I'm using Babel and Webpack.


Solution

  • The main problem is that you have a circular dependency. For a more minimal example which can illustrate the problem, consider:

    // foo.js
    import bar from './bar';
    const x = bar();
    export default () => x;
    
    // bar.js
    import foo from './foo';
    const y = foo();
    export default () => y;
    

    In the above code, just like in your code, you have modules which are using an import, where said import also depends on importing something from the current module. If A imports from B, and B also imports from A, then you need to make sure that neither A nor B use anything from each other before both of their top-level code has finished completely - otherwise, you can't depend on the other import being defined at that time.

    It would be best to refactor the code to remove the circular dependency somehow.

    A pattern I like to use is to make sure top-level code in a module does not initiate anything - rather, it's nice when everything can ultimately kicked off from the entry point (which, here, looks to be index.js). A quick fix would be for each module to export a function which runs the initialization code, rather than putting that code on the top level. For example:

    // views.js
    
    import { getTodos, saveTodos, toggleTodo, removeTodo  } from './todos'
    import { getFilters } from './filters'
    
    let todos;
    const init = () => {
      todos = getTodos();
    };
    // ... everything else
    export { generateSummaryDOM, renderTodos, generateTodoDOM, init }
    

    Then have index.js import and call init at the very beginning, right after importing everything.

    Luckily, since todos.js only calls (the cross-module, imported) renderTodos in its createTodo function, and createTodo is exported but not called in todos.js, it doesn't need to be changed.

    Another (probably better) option would be to remove todos.js's dependancy on renderTodos. In todos.js, you're currently only using renderTodos in its createTodo function. Consider changing thigns so that createTodo creates and saves the todo, but does not render it with renderTodos- instead, look for where createTodo is used (which looks to be only in index.js), and have index.js call renderTodos instead:

    // todos.js
    
    // do not import renderTodos here
    
    // ...
    const createTodo = (text) => {
        if (text.length > 0) {
            todos.push({
                id: uuidv4(),
                text,
                completed: false
            })
            saveTodos()
        }
    }
    

    and

    // index.js
    
    // ...
    document.querySelector('#add-todo').addEventListener('submit', (e) => {
        e.preventDefault()
        const text = e.target.elements.addTodoText.value.trim()
        createTodo(text)
        renderTodos();
        e.target.elements.addTodoText.value = ''
    })