6
\$\begingroup\$

I recently switched to modular JavaScript and really like the idea of having the state of your application in JavaScript and not in the DOM. I want to know if what I am doing is considered best practices or not and how I can avoid re-rendering the entire list after each change.

First, the code:

HTML

<form id="add-user-form">
  <input type="text" placeholder="username" name="username">
  <button>add User</button>
</form>
<hr>
<ul id="user-list"></ul>


<script id="user-template" type="text/html">
  <li data-key="${key}">${name}</li>
</script>

JavaScript

// lets you get templates from HTML ( as you can see in the HTML above ). 
// This is to avoid writing out HTML inside JavaScript
const Template = {
  cached: {},
  get(templateId, placeholders) {
   let templateHtml = ''
   if (!this.cached[templateId]) {
      templateHtml = document.getElementById(templateId).innerHTML.trim()
      this.cached[templateId] = templateHtml
   }
   else {
      templateHtml = this.cached[templateId]
   }
   
   for (key in placeholders) {
     templateHtml = templateHtml.replace(new RegExp("\\${\\s*" + key + "\\s*}", "g"), placeholders[key]);
   }
   return templateHtml
  }
}

const Users = {
  users: [],
  init() {
    this.cacheDom()
    this.bindEvents()
  },
  cacheDom() {
    this.addUserFormEl = document.getElementById('add-user-form')
    this.userListEl = document.getElementById('user-list')
  },
  bindEvents() {
    this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
  },
  render() {
    // gets the HTML from ALL users of our user list and replaces the current user list
    const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
    this.userListEl.innerHTML = userHtml
  },
  handleAddUser(e) {
    e.preventDefault()

    const username = e.currentTarget.elements['username'].value
    if (!username) return
    e.currentTarget.elements['username'].value = ''

    const key = this.users.length // I know I know this is bad practice, please ignore :)

    this.users.push({
      key: key,
      name: username      
    })

    this.render()
  }
}

Users.init()

The code has a form with an input field where you can type in a name that, once the form gets submitted, will be added to the user list below. It would probably be a good idea to separate the userlist and the userform into seperate concerns, but for the sake of this example I kept it simple.

I want to keep DOM modifications to a minimum so I put them inside the method Users.render. This however has one side-effect. Whenever I add an user, it reloads the entire list. It might work with this amount of data, but let's say I have thousands of records, maybe even with input fields. These would all be emptied again.

I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later I also want to delete users? I would have to delete the entry within Users.handleRemoveUser and would then have two methods that handle DOM modifications for the same thing.

I know this all can be easily and beautifully achieved with frameworks such as VueJs that use a vDOM but I am looking for a vanillaJs way of handling this. (Please note that the code is just an example. Yes Unit testing and typeScript are missing and I am well aware of the fact that the code will not run in browsers that do not support ES-6).

\$\endgroup\$
1
\$\begingroup\$

Your Concerns/Questions

Whenever I add an user, it reloads the entire list.

One alternate approach would be to have the handleAddUser() store the newly added user in a property (e.g. recentlyAdded) and then have the render() method look for that property - if it is set (to something other than null) then add the rendered template of the new user and clear that property.

When the page loads, is there an existing list of users that gets added to the list? If so, maybe those could be stored in a different property and the existing code in render() could look for that property for rendering the existing records.

I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later, I also want to delete users. I would have to delete the entry within Users.handleRemoveUser and would so already have two methods that handle DOM modifications for the same thing.

Going along with the alternate approach above, one could have the handleRemoveUser method also store the key of the recently removed user, and then the render() method can look for that property too and remove such an element associated with that key - perhaps by adding an id or other data attribute to the template.

You didn't include an implementation for handleRemoveUser so I can only guess as to what it would be: some way of removing the user from the list this.user. If this.users is still an array, then looking for the user to remove might require a loop. However if this.users is an associative-array (i.e. object with keys corresponding to the key of each user) (or a Map), then looking for the user to remove can be achieved without a loop.

See this demonstrated in the snippet below.


In revision 2 you had this question, altered in revision 5:

As you can see I am not getting the input by ID but by form.elements and then referencing the name.

It is fine to do that, but because the form data doesn't get sent to the server-side, you could also use the id attribute on the input instead of the name attribute, and then fetch that element by id.

Other Feedback

The Template.get() function loops over the keys in placeholders:

for (key in placeholders) {

For this loop, key is a global variable, which may or not be intentional. In general it is best to avoid global variables. One reason would be that if code in a separate function makes its own variable with the same name, there would be no chance of overwriting the value if it isn't a global variable. Use const to declare key as local to that loop.

for (const key in placeholders) {

I recently saw this SO answer that offers a solution to interpolating a string literal similar to how a template literal would be interpolated. I compared it with the regular expression approach in your code and found it to be much slower (see this jsPerf test). I was hoping I could offer that suggestion but unfortunately it can be much slower.

// lets you get templates from HTML ( as you can see in the HTML above ). 
// This is to avoid writing out HTML inside JavaScript
const Template = {
  cached: {},
  get(templateId, placeholders) {
   let templateHtml = ''
   if (!this.cached[templateId]) {
      templateHtml = document.getElementById(templateId).innerHTML.trim()
      this.cached[templateId] = templateHtml
   }
   else {
      templateHtml = this.cached[templateId]
   }
   
   for (let key in placeholders) {
     templateHtml = templateHtml.replace(new RegExp("\\${\\s*" + key + "\\s*}", "g"), placeholders[key]);
   }
   return templateHtml
  }
}

const Users = {
  users: {},
  recentlyAdded: null,
  recentlyRemovedKey: null,
  nextKey: 1,
  init() {
    this.cacheDom()
    this.bindEvents()
  },
  cacheDom() {
    this.addUserFormEl = document.getElementById('add-user-form')
    this.userListEl = document.getElementById('user-list')
  },
  bindEvents() {
    this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
    this.userListEl.addEventListener('click', this.handleListClick.bind(this));
  },
  render() {
    // gets the HTML from ALL users of our user list and replaces the current user list
    /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
    this.userListEl.innerHTML = userHtml*/
    if (this.recentlyAdded) {
    	this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
    	this.recentlyAdded = null;
    }
    if (this.recentlyRemovedKey) {
    	const element = document.getElementById(this.recentlyRemovedKey);
      if (element) {
      	element.remove();
      }
    	this.recentlyRemoved = null;
    }
    console.log('userlist after render: ',this.users);
  },
  handleAddUser(e) {
    e.preventDefault()

    const username = e.currentTarget.elements['username'].value
    if (!username) return
    e.currentTarget.elements['username'].value = ''

    const key = this.nextKey++;

    this.users[key] = {
      key: key,
      name: username      
    };
    this.recentlyAdded = this.users[key];

    this.render();
  },
  handleListClick(e) {
    const target = e.target;
  	if (target.classList.contains('remove')) {
    	this.handleRemoveUser(target.parentNode.id);
    }
  },
  handleRemoveUser(key) {
  	delete this.users[key];
    this.recentlyRemovedKey = key;
    this.render();
  }
}

Users.init()
.remove:before {
  content: 'X';
}
<form id="add-user-form">
  <input type="text" placeholder="username" name="username">
  <button>add User</button>
</form>
<hr>
<ul id="user-list"></ul>


<script id="user-template" type="text/html">
  <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
</script>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.