fix: simplify

config/concatenateAdjacentExpressionsRollupPlugin.js

@@ -1,83 +0,0 @@
-import MagicString from 'magic-string'
-
-// Rollup plugin to concatenate adjacent expressions inside of html`` template literals
-// for text nodes
-// E.g. html`<div>1${a}2{b}3</div>` becomes html`<div>${'1' + a + '2' + b + '3'}</div>`
-
-function concatenateAdjacentExpressions (string) {
-  let count = 0
-  const tokens = []
-  let currentString = ''
-  let i = 0
-
-  const consumeStringLiteral = () => {
-    if (currentString) {
-      tokens.push(JSON.stringify(currentString))
-      currentString = ''
-    }
-  }
-
-  while (i < string.length) {
-    count++
-    if (count > 1000) {
-      debugger
-      throw new Error('stack overflow here')
-    }
-    const char = string.charAt(i)
-    if (char === '$' && string.charAt(i + 1) === '{') {
-      // any pre-existing chars are string literals
-      consumeStringLiteral()
-      // consume expression
-      i += 2
-      let expression = ''
-      while (string.charAt(i) !== '}') {
-        count++
-        if (count > 1000) {
-          debugger
-          throw new Error('stack overflow here')
-        }
-        expression += string.charAt(i)
-        i++
-      }
-      tokens.push(expression)
-    } else {
-      currentString += char
-    }
-    i++
-  }
-
-  consumeStringLiteral()
-
-  return '>${' + tokens.join(' + ') + '}<'
-}
-
-export function concatenateAdjacentExpressionsRollupPlugin () {
-  return {
-    id: 'concatenate-adjacent-expressions',
-    transform (content) {
-      if (content.includes('html`')) {
-        const magicString = new MagicString(content)
-
-        const matchingHtmlTagTemplateLiterals = content.matchAll(/html`.*?`/sg)
-
-        for (const match of matchingHtmlTagTemplateLiterals) {
-          const substring = match[0]
-          const { index } = match
-          for (const subMatch of substring.matchAll(/>(.+?)</gs)) {
-            const [ whole, arg1] = subMatch
-            const { index: subIndex } = subMatch
-
-            const replacement = concatenateAdjacentExpressions(arg1)
-
-            magicString.update(index + subIndex, index + subIndex + whole.length, replacement)
-          }
-        }
-
-        return {
-          code: magicString.toString(),
-          map: magicString.generateMap()
-        }
-      }
-    }
-  }
-}

rollup.config.js

@@ -5,7 +5,6 @@ import strip from '@rollup/plugin-strip'
 import analyze from 'rollup-plugin-analyzer'
 import { minifyHtmlLiteralsRollupPlugin } from './config/minifyHtmlLiteralsRollupPlugin.js'
 import { buildStylesRollupPlugin } from './config/buildStylesRollupPlugin.js'
-import { concatenateAdjacentExpressionsRollupPlugin } from './config/concatenateAdjacentExpressionsRollupPlugin.js'
 
 const { NODE_ENV, DEBUG, PERF } = process.env
 const dev = NODE_ENV !== 'production'
@@ -27,7 +26,6 @@ const baseConfig = {
       preventAssignment: true
     }),
     minifyHtmlLiteralsRollupPlugin(),
-    concatenateAdjacentExpressionsRollupPlugin(),
     buildStylesRollupPlugin(),
     strip({
       include: ['**/*.js'],

src/picker/components/Picker/framework.js

@@ -199,12 +199,12 @@ function parse (tokens) {
       Object.freeze(binding)
     }
 
-    bindings.push(binding)
-
     if (!withinTag && !withinAttribute) {
-      // add a text node that we can find later
-      htmlString += `{${bindings.length - 1}}`
+      // add the index as a text node, so we can find it later
+      htmlString += bindings.length
     }
+
+    bindings.push(binding)
   }
 
   const template = parseTemplate(htmlString)
@@ -216,7 +216,9 @@ function parse (tokens) {
 }
 
 function findPlaceholderTextNode (element, bindingId) {
-  const bindingIdAsNodeValue = `{${bindingId}}`
+  // This can technically conflict with user markup, but we just don't support numeric literals in tags, e.g.
+  // `<div>123</div>`
+  const bindingIdAsNodeValue = '' + bindingId
   // If we had a lot of placeholder nodes to find, it would make more sense to build up a map once
   // rather than search the DOM every time. But it turns out that we always only have one child,
   // and it's the placeholder node, so searching every time is actually faster.

test.mjs

@@ -1,4 +0,0 @@
-import { concatenateAdjacentExpressionsRollupPlugin } from './config/concatenateAdjacentExpressionsRollupPlugin.js'
-
-console.log(concatenateAdjacentExpressionsRollupPlugin().transform('html`<div>yolo${foo}</div>`'))
-

test/spec/picker/framework.test.js

@@ -1,7 +1,4 @@
 import { createFramework } from '../../../src/picker/components/Picker/framework.js'
-import {
-  concatenateAdjacentExpressionsRollupPlugin
-} from '../../../config/concatenateAdjacentExpressionsRollupPlugin.js'
 
 describe('framework', () => {
   test('patches a node', () => {
@@ -61,7 +58,8 @@ describe('framework', () => {
     expect(node.outerHTML).toBe('<div><span>foo</span></div>')
   })
 
-  test('render two dynamic expressions inside the same element', () => {
+  // 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' }
 
     const { html } = createFramework(state)
@@ -80,7 +78,8 @@ describe('framework', () => {
     expect(node.outerHTML).toBe('<div>bazquux</div>')
   })
 
-  test('render a mix of dynamic and static text nodes in the same element', () => {
+  // Framework no longer supports this since we switched from HTML comments to text nodes
+  test.skip('render a mix of dynamic and static text nodes in the same element', () => {
     const state = { name1: 'foo', name2: 'bar' }
 
     const { html } = createFramework(state)
@@ -127,18 +126,4 @@ describe('framework', () => {
     expectRender(() => html`<div class="a${state.name}z"></div>`, '<div class="afooz"></div>', '<div class="abarz"></div>')
     expectRender(() => html`<div class=a${state.name}z></div>`, '<div class="afooz"></div>', '<div class="abarz"></div>')
   })
-
-  test('transform-adjacent-expressions', () => {
-    /* eslint-disable no-template-curly-in-string */
-    const HTML = 'html' // avoid plugin activating on this very file
-    const plugin = concatenateAdjacentExpressionsRollupPlugin()
-    expect(plugin.transform(HTML+'`<div>yolo</div>`').code).toBe(HTML+'`<div>${"yolo"}</div>`')
-    expect(plugin.transform(HTML+'`<div>${a}</div>`').code).toBe(HTML+'`<div>${a}</div>`')
-    expect(plugin.transform(HTML+'`<div>1${a}</div>`').code).toBe(HTML+'`<div>${"1" + a}</div>`')
-    expect(plugin.transform(HTML+'`<div>1${a}2</div>`').code).toBe(HTML+'`<div>${"1" + a + "2"}</div>`')
-    expect(plugin.transform(HTML+'`<div>123${a}456</div>`').code).toBe(HTML+'`<div>${"123" + a + "456"}</div>`')
-    expect(plugin.transform(HTML+'`<div>123${a}456${b}</div>`').code).toBe(HTML+'`<div>${"123" + a + "456" + b}</div>`')
-    expect(plugin.transform(HTML+'`<div>123${a}456${b}789</div>`').code).toBe(HTML+'`<div>${"123" + a + "456" + b + "789"}</div>`')
-    /* eslint-enable no-template-curly-in-string */
-  })
 })

vitest.config.js

@@ -1,12 +1,10 @@
 import { defineConfig } from 'vitest/config'
 import { minifyHtmlLiteralsRollupPlugin } from './config/minifyHtmlLiteralsRollupPlugin.js'
 import { buildStylesRollupPlugin } from './config/buildStylesRollupPlugin.js'
-import { concatenateAdjacentExpressionsRollupPlugin } from './config/concatenateAdjacentExpressionsRollupPlugin.js'
 
 export default defineConfig({
   plugins: [
     minifyHtmlLiteralsRollupPlugin(),
-    concatenateAdjacentExpressionsRollupPlugin(),
     buildStylesRollupPlugin()
   ],
   test: {