Ensure plugins not reapplied (#691)

* Update to 0.7.2 * Fix indicator and remove auto ID generation * Setup * Data-* attributes hash their contents * Update release notes * Revert star.ts and remove test --------- Co-authored-by: Delaney Gillilan <[email protected]>
starfederation · Feb 22, 2025 · 17f2201 · 17f2201
1 parent 0c085c3
commit 17f2201
Show file tree

Hide file tree

Showing 12 changed files with 116 additions and 129 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,9 +15,10 @@ Each tagged version of Datastar is accompanied by a release note. Read the [rele
 - Updated Idiomorph to version [0.7.2](https://github.com/bigskysoftware/idiomorph/blob/main/CHANGELOG.md#072---2025-02-20).
 - When using `data-bind` on an element, the signal value now defaults to the element’s `value` attribute, provided the signal has not already been defined ([#685](https://github.com/starfederation/datastar/issues/685)).
 - Whitespace is now maintained in merged fragments ([#658](https://github.com/starfederation/datastar/issues/658)).
+- Attribute plugins now define a hash of their contents, preventing duplicate applies ([#691](https://github.com/starfederation/datastar/issues/691)).
+- Attribute plugins are now applied to the `html` element instead of the `body` element ([#691](https://github.com/starfederation/datastar/issues/691)).
 
 ### Fixed
 
 - Fixed a bug in which `datastar-remove-fragments` events were not having any effect ([#664](https://github.com/starfederation/datastar/issues/664)).
-- Fixed a bug in which `datastarNaN` could be used as an auto-generated element ID ([#679](https://github.com/starfederation/datastar/issues/679)).
-- Fixed a bug in which plugins were being applied to the DOM twice on page load.
+- Fixed a bug in which `datastarNaN` could be used as an auto-generated element ID ([#679](https://github.com/starfederation/datastar/issues/679)).
diff --git a/bundles/datastar-aliased.js b/bundles/datastar-aliased.js
diff --git a/bundles/datastar-aliased.js.map b/bundles/datastar-aliased.js.map
diff --git a/bundles/datastar-core.js b/bundles/datastar-core.js
diff --git a/bundles/datastar-core.js.map b/bundles/datastar-core.js.map
diff --git a/bundles/datastar.js b/bundles/datastar.js
diff --git a/bundles/datastar.js.map b/bundles/datastar.js.map
diff --git a/library/src/engine/engine.ts b/library/src/engine/engine.ts
@@ -1,6 +1,5 @@
-import { Hash, elUniqId, walkDOM } from '../utils/dom'
+import { Hash, attrHash, elUniqId, walkDOM } from '../utils/dom'
 import { camel } from '../utils/text'
-import { debounce } from '../utils/timing'
 import { effect } from '../vendored/preact-core'
 import { DSP, DSS } from './consts'
 import { initErr, runtimeErr } from './errors'
@@ -21,8 +20,6 @@ import {
   type WatcherPlugin,
 } from './types'
 
-const removalKey = (k: string, v: string) => `${k}${DSP}${v}`
-
 export class Engine {
   aliasPrefix = ''
   #signals: SignalsRoot = new SignalsRoot()
@@ -32,7 +29,7 @@ export class Engine {
   #mutationObserver: MutationObserver | null = null
 
   // Map of cleanup functions by element, keyed by the dataset key and value
-  #removals = new Map<Element, Map<string, OnRemovalFn>>()
+  #removals = new Map<Element, Map<number, OnRemovalFn>>()
 
   get signals() {
     return this.#signals
@@ -48,7 +45,7 @@ export class Engine {
         effect: (cb: () => void): OnRemovalFn => effect(cb),
         actions: this.#actions,
         plugin,
-        applyAttributePlugin: that.#applyAttributePlugin.bind(that),
+        applyPluginsTo: that.#apply.bind(that),
       }
 
       let globalInitializer: GlobalInitializer | undefined
@@ -85,31 +82,40 @@ export class Engine {
       return a.name.localeCompare(b.name)
     })
 
-    this.#debouncedApply()
-  }
-
-  // Add a debounce so that it is only applyied once, regardless of how many times the load function is called
-  #debouncedApply = debounce(() => {
-    this.#apply(document.body)
+    this.#apply(document.documentElement)
     this.#observe()
-  }, 1)
+  }
 
   // Apply all plugins to the element and its children
   #apply(rootElement: Element) {
     walkDOM(rootElement, (el) => {
-      // Cleanup any existing removal functions
-      const elRemovals = this.#removals.get(el)
-      if (elRemovals) {
-        for (const [, removalFn] of elRemovals) {
-          removalFn()
-        }
-        this.#removals.delete(el)
-      }
+      // Check if the element has any data attributes already
+      const toApply = new Array<string>()
+      const elCleanups = this.#removals.get(el) || new Map()
+      const toCleanup = new Map<number, OnRemovalFn>([...elCleanups])
+      const hashes = new Map<string, number>()
 
       // Apply the plugins to the element in order of application
       // since DOMStringMap is ordered, we can be deterministic
       for (const datasetKey of Object.keys(el.dataset)) {
-        this.#applyAttributePlugin(el, datasetKey)
+        const datasetValue = el.dataset[datasetKey] || ''
+        const currentHash = attrHash(datasetKey, datasetValue)
+        hashes.set(datasetKey, currentHash)
+
+        // If the hash hasn't changed, ignore
+        // otherwise keep the old cleanup and add new to applys
+        if (elCleanups.has(currentHash)) {
+          toCleanup.delete(currentHash)
+        } else {
+          toApply.push(datasetKey)
+        }
+      }
+
+      // Clean up any old plugins and apply the new ones
+      for (const [_, cleanup] of toCleanup) cleanup()
+      for (const key of toApply) {
+        const h = hashes.get(key)!
+        this.#applyAttributePlugin(el, key, h)
       }
     })
   }
@@ -121,31 +127,23 @@ export class Engine {
     }
 
     this.#mutationObserver = new MutationObserver((mutations) => {
+      const toRemove = new Set<HTMLorSVGElement>()
+      const toApply = new Set<HTMLorSVGElement>()
       for (const {
         target,
         type,
         attributeName,
-        oldValue,
         addedNodes,
         removedNodes,
       } of mutations) {
         switch (type) {
           case 'childList':
             {
               for (const node of removedNodes) {
-                const el = node as HTMLorSVGElement
-                const elRemovals = this.#removals.get(el)
-                if (!elRemovals) continue
-
-                for (const [_, removalFn] of elRemovals) {
-                  removalFn()
-                }
-                this.#removals.delete(el)
+                toRemove.add(node as HTMLorSVGElement)
               }
-
               for (const node of addedNodes) {
-                const el = node as HTMLorSVGElement
-                this.#apply(el)
+                toApply.add(node as HTMLorSVGElement)
               }
             }
             break
@@ -157,34 +155,25 @@ export class Engine {
               if (!attributeName?.startsWith(requiredPrefix)) {
                 break
               }
-
-              const el = target as HTMLorSVGElement
-              const datasetKey = camel(
-                attributeName.slice(datasetPrefix.length),
-              )
-
-              // If the value has changed, clean up the old value
-              if (oldValue !== null && el.dataset[datasetKey] !== oldValue) {
-                const elRemovals = this.#removals.get(el)
-                if (elRemovals) {
-                  const rk = removalKey(datasetKey, oldValue)
-                  const removalFn = elRemovals.get(rk)
-                  if (removalFn) {
-                    removalFn()
-                    elRemovals.delete(rk)
-                  }
-                }
-              }
-
-              // Apply the plugin only if the dataset key exists
-              if (datasetKey in el.dataset) {
-                this.#applyAttributePlugin(el, datasetKey)
-              }
+              toApply.add(target as HTMLorSVGElement)
             }
             break
           }
         }
       }
+      for (const el of toRemove) {
+        const elTracking = this.#removals.get(el)
+        if (elTracking) {
+          for (const [h, cleanup] of elTracking) {
+            cleanup()
+            elTracking.delete(h)
+          }
+          if (elTracking.size === 0) {
+            this.#removals.delete(el)
+          }
+        }
+      }
+      for (const el of toApply) this.#apply(el)
     })
 
     this.#mutationObserver.observe(document.body, {
@@ -195,7 +184,11 @@ export class Engine {
     })
   }
 
-  #applyAttributePlugin(el: HTMLorSVGElement, camelCasedKey: string) {
+  #applyAttributePlugin(
+    el: HTMLorSVGElement,
+    camelCasedKey: string,
+    hash: number,
+  ) {
     // Extract the raw key from the dataset
     const rawKey = camelCasedKey.slice(this.aliasPrefix.length)
 
@@ -205,16 +198,6 @@ export class Engine {
     // Skip if no plugin is found
     if (!plugin) return
 
-    const elAttr = this.#removals.get(el)
-    if (elAttr) {
-      for (const [k, removalFn] of elAttr) {
-        if (k.startsWith(camelCasedKey)) {
-          removalFn()
-          elAttr.delete(k)
-        }
-      }
-    }
-
     // Ensure the element has an id
     if (!el.id.length) el.id = elUniqId(el)
 
@@ -234,7 +217,7 @@ export class Engine {
       get signals() {
         return that.#signals
       },
-      applyAttributePlugin: that.#applyAttributePlugin.bind(that),
+      applyPluginsTo: that.#apply.bind(that),
       effect: (cb: () => void): OnRemovalFn => effect(cb),
       actions: this.#actions,
       genRX: () => this.#genRX(ctx, ...(plugin.argNames || [])),
@@ -281,20 +264,14 @@ export class Engine {
     }
 
     // Load the plugin and store any cleanup functions
-    const removalFn = plugin.onLoad(ctx)
-    if (removalFn) {
-      let elRemovals = this.#removals.get(el)
-      if (!elRemovals) {
-        elRemovals = new Map()
-        this.#removals.set(el, elRemovals)
+    const cleanup = plugin.onLoad(ctx)
+    if (cleanup) {
+      let elTracking = this.#removals.get(el)
+      if (!elTracking) {
+        elTracking = new Map()
+        this.#removals.set(el, elTracking)
       }
-      elRemovals.set(removalKey(camelCasedKey, value), removalFn)
-    }
-
-    // Remove the attribute if required
-    const removeOnLoad = plugin.removeOnLoad
-    if (removeOnLoad && removeOnLoad(rawKey) === true) {
-      delete el.dataset[camelCasedKey]
+      elTracking.set(hash, cleanup)
     }
   }
 
@@ -337,7 +314,7 @@ export class Engine {
     const escapeRe = new RegExp(`(?:${DSP})(.*?)(?:${DSS})`, 'gm')
     for (const match of userExpression.matchAll(escapeRe)) {
       const k = match[1]
-      const v = new Hash('dsEscaped').with(k).value
+      const v = new Hash('dsEscaped').with(k).string
       escaped.set(v, k)
       userExpression = userExpression.replace(DSP + k + DSS, v)
     }

diff --git a/library/src/engine/types.ts b/library/src/engine/types.ts
@@ -54,7 +54,6 @@ export interface AttributePlugin extends DatastarPlugin {
   type: PluginType.Attribute
   onGlobalInit?: (ctx: InitContext) => void // Called once on registration of the plugin
   onLoad: (ctx: RuntimeContext) => OnRemovalFn | void // Return a function to be called on removal
-  removeOnLoad?: (rawKey: string) => boolean // Return whether the attribute key should be removed after onLoad (useful for plugin attributes you don’t want reapplied)
   mods?: Set<string> // If not provided, all modifiers are allowed
   keyReq?: Requirement // The rules for the key requirements
   valReq?: Requirement // The rules for the value requirements
@@ -82,7 +81,7 @@ export type InitContext = {
   signals: SignalsRoot
   effect: (fn: EffectFn) => OnRemovalFn
   actions: Readonly<ActionPlugins>
-  applyAttributePlugin: (el: HTMLorSVGElement, datasetKey: string) => void
+  applyPluginsTo: (el: HTMLorSVGElement) => void
 }
 
 export type HTMLorSVGElement = Element & (HTMLElement | SVGElement)

diff --git a/library/src/plugins/official/dom/attributes/on.ts b/library/src/plugins/official/dom/attributes/on.ts
@@ -26,7 +26,6 @@ export const On: AttributePlugin = {
   keyReq: Requirement.Must,
   valReq: Requirement.Must,
   argNames: [EVT],
-  removeOnLoad: (rawKey: string) => rawKey.startsWith('onLoad'),
   onLoad: ({ el, key, mods, rawKey, signals, value, effect, genRX }) => {
     const rx = genRX()
     let target: Element | Window | Document = el

diff --git a/library/src/utils/dom.ts b/library/src/utils/dom.ts
@@ -9,17 +9,24 @@ export class Hash {
     this.#prefix = prefix
   }
 
-  with(x: number | string) {
+  with(x: number | string | boolean): Hash {
     if (typeof x === 'string') {
       for (const c of x.split('')) {
         this.with(c.charCodeAt(0))
       }
+    } else if (typeof x === 'boolean') {
+      this.with(1 << (x ? 7 : 3))
     } else {
       this.#value = (this.#value << 5) - this.#value + x
     }
+    return this
   }
 
   get value() {
+    return this.#value
+  }
+
+  get string() {
     return this.#prefix + Math.abs(this.#value).toString(36)
   }
 }
@@ -40,7 +47,11 @@ export function elUniqId(el: Element) {
 
     currentEl = p as Element
   }
-  return hash.value
+  return hash.string
+}
+
+export function attrHash(key: string, val: string) {
+  return new Hash().with(key).with(val).value
 }
 
 export function walkDOM(

diff --git a/sdk/go/consts.go b/sdk/go/consts.go