Simplify lazy-define: idle-disconnect observer, isolate callback errors, add size budget

- Disconnect the MutationObserver once all lazy definitions resolve so it
  stops reacting to unrelated DOM mutations for the rest of the page's life
- Run each callback independently and report errors via reportError instead
  of surfacing unhandled promise rejections
- Restore single-frame scan batching (revert per-element rAF timers)
- Add a size-limit budget for lazyDefine (768B / 0.9kb)
- Add regression tests for re-observation and error isolation

Author: mattcosta7

package.json

@@ -57,6 +57,11 @@
       "import": "{controller, attr, target, targets}",
       "limit": "2.6kb"
     },
+    {
+      "path": "lib/index.js",
+      "import": "{lazyDefine}",
+      "limit": "0.9kb"
+    },
     {
       "path": "lib/abilities.js",
       "import": "{providable}",

src/lazy-define.ts

@@ -1,7 +1,6 @@
 type Strategy = (tagName: string) => Promise<void>
 
-const pending = new Map<string, Set<() => void>>()
-const triggered = new Set<string>()
+const dynamicElements = new Map<string, Set<() => void>>()
 
 const ready = new Promise<void>(resolve => {
   if (document.readyState !== 'loading') {
@@ -24,67 +23,31 @@ const firstInteraction = new Promise<void>(resolve => {
   document.addEventListener('pointerdown', handler, listenerOptions)
 })
 
-const visible = async (tagName: string): Promise<void> => {
-  const observeIntersection = (elements: Element[]) => {
-    return new Promise<void>(resolve => {
-      const observer = new IntersectionObserver(
-        entries => {
-          for (const entry of entries) {
-            if (entry.isIntersecting) {
-              resolve()
-              observer.disconnect()
-              return
-            }
+const visible = (tagName: string): Promise<void> =>
+  new Promise<void>(resolve => {
+    const observer = new IntersectionObserver(
+      entries => {
+        for (const entry of entries) {
+          if (entry.isIntersecting) {
+            resolve()
+            observer.disconnect()
+            return
           }
-        },
-        {
-          // Currently the threshold is set to 256px from the bottom of the viewport
-          // with a threshold of 0.1. This means the element will not load until about
-          // 2 keyboard-down-arrow presses away from being visible in the viewport,
-          // giving us some time to fetch it before the contents are made visible
-          rootMargin: '0px 0px 256px 0px',
-          threshold: 0.01
         }
-      )
-      for (const element of elements) {
-        observer.observe(element)
+      },
+      {
+        // Currently the threshold is set to 256px from the bottom of the viewport
+        // with a threshold of 0.1. This means the element will not load until about
+        // 2 keyboard-down-arrow presses away from being visible in the viewport,
+        // giving us some time to fetch it before the contents are made visible
+        rootMargin: '0px 0px 256px 0px',
+        threshold: 0.01
       }
-    })
-  }
-
-  const waitForElement = () => {
-    return new Promise<Element[]>(resolve => {
-      const observer = new MutationObserver(mutations => {
-        for (const mutation of mutations) {
-          const addedNodes = Array.from(mutation.addedNodes)
-          for (const node of addedNodes) {
-            if (!(node instanceof Element)) continue
-
-            const isMatch = node.matches(tagName)
-            const descendant = node.querySelector(tagName)
-
-            if (isMatch || descendant) {
-              observer.disconnect()
-              resolve(Array.from(document.querySelectorAll(tagName)))
-              return
-            }
-          }
-        }
-      })
-
-      observer.observe(document.documentElement, {childList: true, subtree: true})
-    })
-  }
-
-  const existingElements = Array.from(document.querySelectorAll(tagName))
-
-  if (existingElements.length > 0) {
-    return observeIntersection(existingElements)
-  }
-
-  const foundElements = await waitForElement()
-  return observeIntersection(foundElements)
-}
+    )
+    for (const el of document.querySelectorAll(tagName)) {
+      observer.observe(el)
+    }
+  })
 
 const strategies: Record<string, Strategy> = {
   ready: () => ready,
@@ -94,121 +57,71 @@ const strategies: Record<string, Strategy> = {
 
 type ElementLike = Element | Document | ShadowRoot
 
-const observedTargets = new WeakSet<ElementLike>()
-const timers = new WeakMap<ElementLike, number>()
-
-function cleanupObserver() {
-  if (pending.size === 0 && elementLoader) {
-    elementLoader.disconnect()
-    elementLoader = undefined
-  }
-}
+const pendingElements = new Set<ElementLike>()
+let scanTimer: number | null = null
+let elementLoader: MutationObserver | undefined
 
 function scan(element: ElementLike) {
-  const currentTimer = timers.get(element)
-  if (currentTimer) cancelAnimationFrame(currentTimer)
-
-  const newTimer = requestAnimationFrame(() => {
-    // FIX 7: Early return optimization
-    if (pending.size === 0) return
-
-    // FIX 7: Create snapshot to prevent modification-during-iteration issues
-    // (concurrent scans may delete tags from pending)
-    const tagList = Array.from(pending.keys())
-
-    for (const tagName of tagList) {
-      const child: Element | null =
-        element instanceof Element && element.matches(tagName) ? element : element.querySelector(tagName)
-      if (customElements.get(tagName) || child) {
-        // Skip if already processed and not re-registered
-        if (triggered.has(tagName) && !pending.has(tagName)) continue
-
-        triggered.add(tagName)
-
-        const callbackSet = pending.get(tagName)
-        pending.delete(tagName)
-
-        const strategyName = (child?.getAttribute('data-load-on') || 'ready') as keyof typeof strategies
-        const strategy = strategyName in strategies ? strategies[strategyName] : strategies.ready
-
-        // FIX 5: Wrap callback execution in try-catch and handle rejections
-        const callbackList = Array.from(callbackSet || [])
-        for (const callback of callbackList) {
-          strategy(tagName)
-            // eslint-disable-next-line github/no-then
-            .then(() => {
-              try {
-                callback()
-              } catch (err) {
-                reportError(err)
-              }
-            })
+  pendingElements.add(element)
+  if (scanTimer != null) return
+  scanTimer = requestAnimationFrame(() => {
+    scanTimer = null
+    const elements = new Set(pendingElements)
+    pendingElements.clear()
+    if (!dynamicElements.size) return
+    outer: for (const el of elements) {
+      for (const tagName of dynamicElements.keys()) {
+        const child: Element | null = el instanceof Element && el.matches(tagName) ? el : el.querySelector(tagName)
+        if (customElements.get(tagName) || child) {
+          const strategyName = (child?.getAttribute('data-load-on') || 'ready') as keyof typeof strategies
+          const strategy = strategyName in strategies ? strategies[strategyName] : strategies.ready
+          const callbacks = dynamicElements.get(tagName) || []
+          dynamicElements.delete(tagName)
+          for (const callback of callbacks) {
+            // Run each callback independently so one failure cannot prevent the
+            // others, and surface errors through reportError instead of as
+            // unhandled promise rejections.
             // eslint-disable-next-line github/no-then
-            .catch(reportError)
+            strategy(tagName).then(callback).catch(reportError)
+          }
+          if (!dynamicElements.size) break outer
         }
-
-        timers.delete(element)
       }
     }
-
-    // FIX 4: Disconnect observer when all pending tags are processed
-    cleanupObserver()
+    // Once every registered element has been found there is nothing left to look
+    // for, so disconnect the observer rather than keep reacting to unrelated DOM
+    // mutations for the rest of the page's lifetime.
+    if (!dynamicElements.size && elementLoader) {
+      elementLoader.disconnect()
+      elementLoader = undefined
+    }
   })
-
-  timers.set(element, newTimer)
 }
 
-let elementLoader: MutationObserver | undefined
-
 export function lazyDefine(object: Record<string, () => void>): void
 export function lazyDefine(tagName: string, callback: () => void): void
 export function lazyDefine(tagNameOrObj: string | Record<string, () => void>, singleCallback?: () => void) {
   if (typeof tagNameOrObj === 'string' && singleCallback) {
     tagNameOrObj = {[tagNameOrObj]: singleCallback}
   }
-
   for (const [tagName, callback] of Object.entries(tagNameOrObj)) {
-    // FIX 6: Late registration - execute immediately if already triggered
-    // Check both triggered state and element existence to avoid executing for removed elements
-    if (triggered.has(tagName) && document.querySelector(tagName)) {
-      // eslint-disable-next-line github/no-then
-      Promise.resolve().then(() => {
-        try {
-          callback()
-        } catch (err) {
-          reportError(err)
-        }
-      })
-    } else {
-      if (!pending.has(tagName)) {
-        pending.set(tagName, new Set<() => void>())
-      }
-      pending.get(tagName)!.add(callback)
-    }
+    if (!dynamicElements.has(tagName)) dynamicElements.set(tagName, new Set<() => void>())
+    dynamicElements.get(tagName)!.add(callback)
   }
   observe(document)
 }
 
 export function observe(target: ElementLike): void {
-  if (!elementLoader) {
-    elementLoader = new MutationObserver(mutations => {
-      if (!pending.size) return
-      for (const mutation of mutations) {
-        const nodes = mutation.addedNodes
-        for (const node of nodes) {
-          if (node instanceof Element) {
-            scan(node)
-          }
-        }
+  elementLoader ||= new MutationObserver(mutations => {
+    if (!dynamicElements.size) return
+    for (const mutation of mutations) {
+      for (const node of mutation.addedNodes) {
+        if (node instanceof Element) scan(node)
       }
-    })
-  }
+    }
+  })
 
   scan(target)
 
-  // FIX 3: Check observedTargets to avoid redundant observe() calls
-  if (!observedTargets.has(target)) {
-    observedTargets.add(target)
-    elementLoader.observe(target, {subtree: true, childList: true})
-  }
+  elementLoader.observe(target, {subtree: true, childList: true})
 }

test/lazy-define.ts

@@ -149,6 +149,25 @@ describe('lazyDefine', () => {
     })
   })
 
+  describe('observer lifecycle', () => {
+    it('re-observes for definitions registered after everything has resolved', async () => {
+      const onFirst = spy()
+      lazyDefine('idle-first-element', onFirst)
+      await fixture(html`<idle-first-element></idle-first-element>`)
+      await animationFrame()
+      // All pending definitions have resolved, so the observer disconnects here.
+      expect(onFirst).to.be.callCount(1)
+
+      // A later registration must re-establish observation of newly added nodes.
+      const onSecond = spy()
+      lazyDefine('idle-second-element', onSecond)
+      await fixture(html`<idle-second-element></idle-second-element>`)
+      await animationFrame()
+
+      expect(onSecond).to.be.callCount(1)
+    })
+  })
+
   describe('race condition prevention', () => {
     it('does not fire callbacks multiple times from concurrent scans', async () => {
       const onDefine = spy()
@@ -167,7 +186,7 @@ describe('lazyDefine', () => {
   })
 
   describe('late registration', () => {
-    it('executes callback immediately for already-triggered tags', async () => {
+    it('runs a callback registered for a tag that already resolved', async () => {
       const onDefine1 = spy()
       const onDefine2 = spy()
 
@@ -177,11 +196,11 @@ describe('lazyDefine', () => {
       await animationFrame()
       expect(onDefine1).to.be.callCount(1)
 
-      // Register second callback after element is already triggered
+      // Register a second callback after the element already exists in the DOM
       lazyDefine('late-reg-element', onDefine2)
       await animationFrame()
 
-      // Second callback should be executed immediately
+      // The late callback should still run
       expect(onDefine2).to.be.callCount(1)
     })
   })