From 695be06c6118bd60018d7d1ba7ddab21b4d9a914 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sun, 10 Dec 2023 16:37:01 -0800
Subject: [PATCH] refactor: simplify map() impl

---
 .../components/Picker/PickerTemplate.js       | 18 +++++-----
 src/picker/components/Picker/framework.js     | 35 +++++++++----------
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/src/picker/components/Picker/PickerTemplate.js b/src/picker/components/Picker/PickerTemplate.js
index f320e12..40e16b6 100644
--- a/src/picker/components/Picker/PickerTemplate.js
+++ b/src/picker/components/Picker/PickerTemplate.js
@@ -4,7 +4,7 @@ export function render (state, helpers, events, container, firstRender) {
   const { labelWithSkin, titleForEmoji, unicodeWithSkin } = helpers
   const { html, map } = createFramework(state)
 
-  function emojiList (emojis, searchMode, prefix, uniqueId) {
+  function emojiList (emojis, searchMode, prefix) {
     return map(emojis, (emoji, i) => {
       return html`
       <button role="${searchMode ? 'option' : 'menuitem'}"
@@ -12,7 +12,7 @@ export function render (state, helpers, events, container, firstRender) {
               aria-label="${labelWithSkin(emoji, state.currentSkinTone)}"
               title="${titleForEmoji(emoji)}"
               class="emoji ${searchMode && i === state.activeSearchItem ? 'active' : ''}"
-              id="${prefix + '-' + emoji.id}">
+              id=${`${prefix}-${emoji.id}`}>
         ${
         emoji.unicode
           ? unicodeWithSkin(emoji, state.currentSkinTone)
@@ -20,7 +20,9 @@ export function render (state, helpers, events, container, firstRender) {
       }
       </button>
     `
-    }, emoji => emoji.id, uniqueId)
+      // It's important for the cache key to be unique based on the prefix, because the framework caches based on the
+      // unique tokens + cache key, and the same emoji may be used in the tab as well as in the fav bar
+    }, emoji => `${prefix}-${emoji.id}`)
   }
 
   const section = () => {
@@ -103,7 +105,7 @@ export function render (state, helpers, events, container, firstRender) {
           ${skinTone}
         </div>
       `
-    }, skinTone => skinTone, 'skintones')
+    }, skinTone => skinTone)
         }
       </div>
       </div>
@@ -131,7 +133,7 @@ export function render (state, helpers, events, container, firstRender) {
           </div>
         </button>
       `
-            }, group => group.id, 'nav')
+            }, group => group.id)
           }
         </div>
         <div class="indicator-wrapper">
@@ -189,12 +191,12 @@ export function render (state, helpers, events, container, firstRender) {
                aria-labelledby="menu-label-${i}"
                id=${state.searchMode ? 'search-results' : ''}>
             ${
-              emojiList(emojiWithCategory.emojis, state.searchMode, /* prefix */ 'emo', /* uniqueId */ `emo-${emojiWithCategory.category}`)
+              emojiList(emojiWithCategory.emojis, state.searchMode, /* prefix */ 'emo')
             }
           </div>
         </div>
       `
-              }, emojiWithCategory => emojiWithCategory.category, 'emojisWithCategories')
+              }, emojiWithCategory => emojiWithCategory.category)
             }
           </div>
         </div>
@@ -205,7 +207,7 @@ export function render (state, helpers, events, container, firstRender) {
              style="padding-inline-end: ${`${state.scrollbarWidth}px`}"
              data-on-click="onEmojiClick">
           ${
-            emojiList(state.currentFavorites, /* searchMode */ false, /* prefix */ 'fav', /* uniqueId */ 'fav')
+            emojiList(state.currentFavorites, /* searchMode */ false, /* prefix */ 'fav')
           }
         </div>
         <!-- This serves as a baseline emoji for measuring against and determining emoji support -->
diff --git a/src/picker/components/Picker/framework.js b/src/picker/components/Picker/framework.js
index b38b7da..6d1fda2 100644
--- a/src/picker/components/Picker/framework.js
+++ b/src/picker/components/Picker/framework.js
@@ -1,7 +1,7 @@
 import { getFromMap, parseTemplate, toString } from './utils.js'
 
 const parseCache = new WeakMap()
-const updatersCache = new WeakMap()
+const domInstancesCache = new WeakMap()
 const unkeyedSymbol = Symbol('un-keyed')
 
 // Not supported in Safari <=13
@@ -263,29 +263,28 @@ function parseHtml (tokens) {
 }
 
 export function createFramework (state) {
-  let updaters = getFromMap(updatersCache, state, () => new Map())
-  let iteratorKey = unkeyedSymbol
+  const domInstances = getFromMap(domInstancesCache, state, () => new Map())
+  let domInstanceCacheKey = unkeyedSymbol
 
   function html (tokens, ...expressions) {
-    const updatersForKey = getFromMap(updaters, iteratorKey, () => new WeakMap())
-    const updater = getFromMap(updatersForKey, tokens, () => parseHtml(tokens))
+    // Each unique lexical usage of map() is considered unique due to the html`` tagged template call it makes,
+    // which has lexically unique tokens. The unkeyed symbol is just used for html`` usage outside of a map().
+    const domInstancesForTokens = getFromMap(domInstances, tokens, () => new Map())
+    const domInstance = getFromMap(domInstancesForTokens, domInstanceCacheKey, () => parseHtml(tokens))
 
-    return updater(expressions)
+    return domInstance(expressions) // update with expressions
   }
 
-  function map (array, callback, keyFunction, mapKey) {
-    const originalCacheKey = iteratorKey
-    const originalUpdaters = updaters
-    updaters = getFromMap(updaters, mapKey, () => new Map())
-    try {
-      return array.map((item, index) => {
-        iteratorKey = keyFunction(item)
+  function map (array, callback, keyFunction) {
+    return array.map((item, index) => {
+      const originalCacheKey = domInstanceCacheKey
+      domInstanceCacheKey = keyFunction(item)
+      try {
         return callback(item, index)
-      })
-    } finally {
-      iteratorKey = originalCacheKey
-      updaters = originalUpdaters
-    }
+      } finally {
+        domInstanceCacheKey = originalCacheKey
+      }
+    })
   }
 
   return { map, html }