perf: optimize setting attributes

src/picker/components/Picker/PickerTemplate.js

@@ -8,7 +8,7 @@ export function render (container, state, helpers, events, actions, refs, abortS
     return map(emojis, (emoji, i) => {
       return html`
       <button role="${searchMode ? 'option' : 'menuitem'}"
-              aria-selected="${searchMode ? i === state.activeSearchItem : ''}"
+              aria-selected="${searchMode ? i === state.activeSearchItem : undefined}"
               aria-label="${labelWithSkin(emoji, state.currentSkinTone)}"
               title="${titleForEmoji(emoji)}"
               class="${
@@ -17,7 +17,7 @@ export function render (container, state, helpers, events, actions, refs, abortS
                 (emoji.unicode ? '' : ' custom-emoji')
               }"
               id=${`${prefix}-${emoji.id}`}
-              style="${emoji.unicode ? '' : `--custom-emoji-background: url(${JSON.stringify(emoji.url)})`}"
+              style=${emoji.unicode ? undefined : `--custom-emoji-background: url(${JSON.stringify(emoji.url)})`}
       >
         ${
         emoji.unicode
@@ -59,7 +59,7 @@ export function render (container, state, helpers, events, actions, refs, abortS
             aria-controls="search-results"
             aria-describedby="search-description"
             aria-autocomplete="list"
-            aria-activedescendant="${state.activeSearchItemId ? `emo-${state.activeSearchItemId}` : ''}"
+            aria-activedescendant="${state.activeSearchItemId ? `emo-${state.activeSearchItemId}` : undefined}"
             data-ref="searchElement"
             data-on-input="onSearchInput"
             data-on-keydown="onSearchKeydown"
@@ -162,7 +162,7 @@ export function render (container, state, helpers, events, actions, refs, abortS
              class="tabpanel ${(!state.databaseLoaded || state.message) ? 'gone' : ''}"
              role="${state.searchMode ? 'region' : 'tabpanel'}"
              aria-label="${state.searchMode ? state.i18n.searchResultsLabel : state.i18n.categories[state.currentGroup.name]}"
-             id="${state.searchMode ? '' : `tab-${state.currentGroup.id}`}"
+             id=${state.searchMode ? undefined : `tab-${state.currentGroup.id}`}
              tabIndex="0"
              data-on-click="onEmojiClick"
         >
@@ -211,7 +211,7 @@ export function render (container, state, helpers, events, actions, refs, abortS
                data-action="updateOnIntersection"
                role="${state.searchMode ? 'listbox' : 'menu'}"
                aria-labelledby="menu-label-${i}"
-               id=${state.searchMode ? 'search-results' : ''}
+               id=${state.searchMode ? 'search-results' : undefined}
           >
             ${
               emojiList(emojiWithCategory.emojis, state.searchMode, /* prefix */ 'emo')

src/picker/components/Picker/framework.js

@@ -87,7 +87,14 @@ function patch (expressions, instanceBindings) {
     instanceBinding.currentExpression = expression
 
     if (attributeName) { // attribute replacement
-      targetNode.setAttribute(attributeName, attributeValuePre + toString(expression) + attributeValuePost)
+      if (!attributeValuePre && !attributeValuePost && expression === undefined) {
+        // undefined is treated as a special case by the framework - we don't render an attribute at all in this case
+        targetNode.removeAttribute(attributeName)
+      } else {
+        // attribute value is not undefined; set a new attribute
+        const newValue = attributeValuePre + toString(expression) + attributeValuePost
+        targetNode.setAttribute(attributeName, newValue)
+      }
     } else { // text node / child element / children replacement
       let newNode
       if (Array.isArray(expression)) { // array of DOM elements produced by tag template literals
@@ -123,9 +130,10 @@ function parse (tokens) {
   const elementsToBindings = new Map()
   const elementIndexes = []
 
+  let skipTokenChars = 0
   for (let i = 0, len = tokens.length; i < len; i++) {
     const token = tokens[i]
-    htmlString += token
+    htmlString += token.slice(skipTokenChars)
 
     if (i === len - 1) {
       break // no need to process characters - no more expressions to be found
@@ -177,10 +185,17 @@ function parse (tokens) {
     let attributeValuePost
     if (withinAttribute) {
       // I never use single-quotes for attribute values in HTML, so just support double-quotes or no-quotes
-      const match = /(\S+)="?([^"=]*)$/.exec(token)
-      attributeName = match[1]
-      attributeValuePre = match[2]
-      attributeValuePost = /^[^">]*/.exec(tokens[i + 1])[0]
+      const attributePreMatch = /(\S+)="?([^"=]*)$/.exec(token)
+      attributeName = attributePreMatch[1]
+      attributeValuePre = attributePreMatch[2]
+      const attributePostMatch = /^([^">]*)("?)/.exec(tokens[i + 1])
+      attributeValuePost = attributePostMatch[1]
+      // Optimization: remove the attribute itself, so we don't create a default attribute which is either empty or just
+      // the "pre" text, e.g. `<div foo>` or `<div foo="prefix">`. It will be replaced by the expression anyway.
+      htmlString = htmlString.slice(0, -1 * attributePreMatch[0].length)
+      skipTokenChars = attributePostMatch[0].length
+    } else {
+      skipTokenChars = 0
     }
 
     const binding = {

test/spec/picker/framework.test.js

@@ -76,6 +76,70 @@ describe('framework', () => {
     expect(node.outerHTML).toBe('<div>baz</div>')
   })
 
+  test('set to undefined then something then back to undefined', () => {
+    const state = { value: undefined }
+    const { html } = createFramework(state)
+
+    let node
+    const render = () => {
+      node = html`<div aria-selected=${state.value}></div>`
+    }
+
+    render()
+    expect(node.getAttribute('aria-selected')).toBe(null)
+
+    state.value = true
+    render()
+    expect(node.getAttribute('aria-selected')).toBe('true')
+
+    state.value = undefined
+    render()
+    expect(node.getAttribute('aria-selected')).toBe(null)
+  })
+
+  test('set to undefined then something then back to undefined - with quotes', () => {
+    const state = { value: undefined }
+    const { html } = createFramework(state)
+
+    let node
+    const render = () => {
+      node = html`<div aria-selected="${state.value}"></div>`
+    }
+
+    render()
+    expect(node.getAttribute('aria-selected')).toBe(null)
+
+    state.value = true
+    render()
+    expect(node.getAttribute('aria-selected')).toBe('true')
+
+    state.value = undefined
+    render()
+    expect(node.getAttribute('aria-selected')).toBe(null)
+  })
+
+  test('set to undefined then something then back to undefined - with pre/post text', () => {
+    const state = { value: undefined }
+    const { html } = createFramework(state)
+
+    let node
+    const render = () => {
+      node = html`<div class="foo ${state.value} bar"></div>`
+    }
+
+    render()
+    // expect(node.getAttribute('class')).toBe('foo undefined bar')
+    expect(node.getAttribute('class')).toBe(null) // we don't handle the initial undefined case correctly
+
+    state.value = true
+    render()
+    expect(node.getAttribute('class')).toBe('foo true bar')
+
+    state.value = undefined
+    render()
+    expect(node.getAttribute('class')).toBe('foo undefined bar')
+  })
+
   // Framework no longer supports this since we switched from HTML comments to text nodes
   test.skip('render two dynamic expressions inside the same element', () => {
     const state = { name1: 'foo', name2: 'bar' }