Native JavaScript to-do list app using Jade

For practice and to explore my vanilla JavaScript (no-library) weaknesses, yesterday I made a Todo app, similar to what you see floating around for AngularJS blog demos.

How can I optimize this?

// VanillaJS Todo App with full CRUD

(function crudApp() {
  "use strict";
  // Look ma... no library!
  var itemData = {};
  var getSelector = function(targSelector) {
    for (var i = 0; i ' + data + 'CompletedEdit';
    }

  };

  // This should handle both the edit and delete functionality and also click event on dynamically added elms (both come from same elm type so I use the attribute value to decide which function to implement)

  getSelector(selectors.outputZone).onclick = function(event) {

    if (methods.hasAttribute(event.target, 'data-app-delete-item')) {
      var deleteItemAction = new methods.deleteItem(event.target.getAttribute('data-app-delete-item'));
    } else if (methods.hasAttribute(event.target, 'data-app-edit-item')) {
      var editItemAction = new methods.editItem(event, 'data-app-edit-item');
    }

  }

  // Bind DOM events directly to functionality

  getSelector(selectors.deleteAllItems).onclick = function() {
    var deleteAllItemsAction = new methods.deleteAllItems();
    methods.render(itemData);
  };

  getSelector(selectors.addItem).onclick = function() {
    var addItemAction = new methods.addItem(getSelector(selectors.textInput).value);
  };

  getSelector('['+selectors.deleteItem+']').onclick = function() {
    var deleteItemAction = new methods.deleteItem(getSelector(selectors.textInput).value);
    methods.render(itemData);
  };

})();

And this is the Jade markup:

```
#app
.user-entry
input(type="text", placeholder="Enter an item to do", data-app-todo-input)
input(type="submit", value="Add item", data-app-add-item)
input(type="button", value="Delete all items", data-app-delete-all)
input(type="button", value="Delete specific item", data-app-delete-item)
.app-output
h1 Pure javascript todo tracker

I'll dissect your JS from top to bottom. But first, the overarching stuff: you tagged this question with OOP, but it's not taking advantage of OOP. It would be very natural to have Task and TodoList classes that you could build upon instead of having some giant methods object.

// VanillaJS Todo App with full CRUD

Just a comment that CRUD usually implies persistent storage.

(function crudApp() {
  "use strict";

Upvote for strict mode :-)

// Look ma... no library!
  var itemData = {};
  var getSelector = function(targSelector) {
    for (var i = 0; i < document.querySelectorAll(targSelector).length; i++) {
      return document.querySelectorAll(targSelector)[i];
    }
  }

You're missing a semicolon—this is a function expression and not a function statement. More importantly, however, this function doesn't make much sense as written.

It returns a single element or undefined if it can't find one. That is almost exactly the behaviour of document.querySelector(), which will return null instead if it doesn't match an element. I would just straight up replace this with document.querySelector.

I'd also like to point out an efficiency issue, even though I'm asking you to get rid of it altogether. The loop condition gets executed every iteration. This means that you're actually performing two querySelectorAll calls to return a single element. This is bad, and it'll get even worse if you try to do something as you iterate through them. Instead, assign the result of document.querySelectorAll() to a variable, to avoid calling it multiple times.

var selectors = {
    'textInput': '[data-app-todo-input]',
    'addItem': '[data-app-add-item]',
    'outputZone': '[data-app-list-output]',
    'deleteAllItems': '[data-app-delete-all]',
    'deleteItem': 'data-app-delete-item', // These are used on dynamic click events, so they don't get backets
    'editItem': 'data-app-edit-item'
  };

  var methods = {

    'hasAttribute': function(target, attribute) {
      return target.getAttribute(attribute);
    },

    'addItem': function(item, properties) {
      var normalizedItem = item.toLowerCase();

You edited out normalizedItem; you should've taken this line out too.

if (!itemData[item] && properties) {
        itemData[item] = properties;
      } else if (!itemData[item] && !properties) {
        itemData[item] = true;
      }
      // No overwrites for duplicate items
      else if (itemData[item] === true) {
        alert('Duplicate entry prevented');
      }

I just want to highlight your if-else if chain. You should stick to one style. I understand that you want to put in a comment, but this should not cause you to uncuddle the else. You can put the comment on the line after the { instead.

Also, this function needs to be documented. I've read it over twice and I can't understand why an item has to be === true to count as an item. It doesn't help that you didn't actually use the addItem(item, properties) form ever.

Anyway, I'd rewrite it a bit still:

if (itemData[item]) {
  alert('...');
} else {
  itemData[item] = properties || true;
  /* or
  if (properties === undefined) {
    itemData[item] = true;
  } else {
    itemData[item] = false;
  }
  */
}

You'll also have to think about whether you need to distinguish between properties being undefined, i.e. defaulting to true, and properties being falsey, like 0 or false or ''.

getSelector(selectors.textInput).value = '';
      methods.render(itemData);

Ideally, you would separate these to be in the event handler, since they deal with UI logic instead, while the rest deal with model logic. This also causes you an issue later on.

},

    'deleteAllItems': function() {
      itemData = {};
      methods.render(itemData);
    },

    'deleteItem': function(itemName) {
      if (itemData[itemName]) {
        delete itemData[itemName];
        methods.render(itemData);
        methods.clearVal(selectors.textInput);
      }
    },

    'readData': function(dataset, property) {
      if (property) return dataset[property];
      else return dataset;

Prime time to use the conditional operator. Consider rewriting as return property ? dataset[property] : dataset;. (Although, I would still question why you need to return both.

},

    'editItem': function(event, attr) {
      var itemName = event.target.getAttribute(attr);

      if (itemData[itemName]) {
        var newItemName = prompt('New todo item?');
        if (newItemName) {
          methods.addItem(newItemName);
          methods.deleteItem(itemName); // Reversed from natural order for UX (no item disappear just from pressing edit)

This is a little bit messy; this is the issue I was referring to earlier. It's good that you added a comment, but even with the comment it seems a little bit bleh. This is mostly an issue because your addItem and deleteItem methods both call render, even though in

StackExchange Code Review Q#96156, answer score: 3