3 ms·

... what's so bad about it?

guntars 6y ago
It's a minefield of potential bugs: this.$todoList = qs('.todo-list'); This bit will only work if there's only one `.todo-list` on the page and it'll break if you try to use more than one component. const listItem = target.parentElement.parentElement; This will break the second you change the template and have to add a wrapper for any reason. input.value = target.innerText; Using the DOM to store your data is just wrong. Browsers can and will change then contents of your nodes if they feel like it. They can strip whitespace, for example. This would be a problem if you're trying to use some kind of change detection. this.$todoList.innerHTML = this.template.itemList(items); Better hope that whoever wrote itemList knows to escape the data properly. Have you noticed how XSS used to be a much more common issue until the advent of view frameworks? Experience has taught us that at least for security you simply cannot depend on the developers to always do the right thing, especially since their experience level might be all over the place. const elem = qs(`[data-id="${id}"]`); if (elem) { this.$todoList.removeChild(elem); } Global selector again, it also depends on the elem being the child of $todoList which is not guaranteed if there's any change to the template. listItem.className = completed ? 'completed' : ''; This will clear any other classes that the element might have on it. You might add some styling to it and then wonder why it breaks after you click a button. target.dataset.iscanceled = true; Storing data on elements is also risky, since there's a lot of other code that updates innerHTML wholesale so your element might not be there anymore. This is just one file. The point I'm trying to make is that this direct DOM manipulation makes for brittle code and only works for very simple cases. Once you have even a small amount of behavior, any sane developer will want to abstract it out a bit and you'll end up with a framework anyway. Better to choose a low overhead one from the beginning.

It's a minefield of potential bugs: this.$todoList = qs('.todo-list'); This bit will only work if there's only one `.todo-list` on the page and it'll break if you try to use more than one component. const listItem = target.parentElement.parentElement; This will break the second you change the template and have to add a wrapper for any reason. input.value = target.innerText; Using the DOM to store your data is just wrong. Browsers can and will change then contents of your nodes if they feel like it. They can strip whitespace, for example. This would be a problem if you're trying to use some kind of change detection. this.$todoList.innerHTML = this.template.itemList(items); Better hope that whoever wrote itemList knows to escape the data properly. Have you noticed how XSS used to be a much more common issue until the advent of view frameworks? Experience has taught us that at least for security you simply cannot depend on the developers to always do the right thing, especially since their experience level might be all over the place. const elem = qs(`[data-id="${id}"]`); if (elem) { this.$todoList.removeChild(elem); } Global selector again, it also depends on the elem being the child of $todoList which is not guaranteed if there's any change to the template. listItem.className = completed ? 'completed' : ''; This will clear any other classes that the element might have on it. You might add some styling to it and then wonder why it breaks after you click a button. target.dataset.iscanceled = true; Storing data on elements is also risky, since there's a lot of other code that updates innerHTML wholesale so your element might not be there anymore. This is just one file. The point I'm trying to make is that this direct DOM manipulation makes for brittle code and only works for very simple cases. Once you have even a small amount of behavior, any sane developer will want to abstract it out a bit and you'll end up with a framework anyway. Better to choose a low overhead one from the beginning.