ensure vitals are consistent and have sane fallbacks and baselines

Author: metal-messiah

src/common/v2/mfe-vitals.js

@@ -72,6 +72,7 @@ const isObservable = (node) => {
  * @returns {boolean}
  */
 const isInMFE = (node, id) => {
+  if (!node || !id) return false
   try {
     let curr = node.nodeType === 1 ? node : node.parentElement
     while (curr?.tagName) {
@@ -85,16 +86,18 @@ const isInMFE = (node, id) => {
 /**
  * Create mutation observer for MFE nodes
  * @param {string} id - MFE ID to track
- * @param {Function} onMatch - Callback when matching node is added
+ * @param {Function} onMatch - Callback when matching node is *added*
  * @returns {MutationObserver}
  */
 const observeMutations = (id, onMatch) => {
   const obs = new globalScope.MutationObserver(mutations => {
-    mutations.forEach(m => m.addedNodes.forEach(node => {
-      if (isObservable(node) && isInMFE(node, id)) {
-        onMatch(node, obs)
-      }
-    }))
+    mutations.forEach(m => {
+      m.addedNodes.forEach(node => {
+        if (isObservable(node) && isInMFE(node, id)) {
+          onMatch(node, obs)
+        }
+      })
+    })
   })
   obs.observe(globalScope.document, { childList: true, subtree: true })
   return obs
@@ -126,47 +129,63 @@ const observePerformance = (observers, config, onEntry) => {
  * @returns {{fcp: object|null, lcp: object|null, cls: object|null, inp: object|null, disconnect: Function}}
  */
 export function trackMFEVitals (id, timings) {
+  let fcpObservedAt = null
+  let lcpObservedAt = null
+
+  const getTimeRelativeToScriptStart = (capturedAt) => {
+    if (capturedAt === null) return null
+    return capturedAt - (timings?.scriptStart || timings?.registeredAt || 0)
+  }
+
   const vitals = {
-    fcp: null,
-    lcp: null,
-    cls: null,
-    inp: null,
+    fcp: {
+      get value () { return getTimeRelativeToScriptStart(fcpObservedAt) }
+    },
+    lcp: {
+      get value () { return getTimeRelativeToScriptStart(lcpObservedAt) }
+    },
+    cls: {
+      value: null
+    },
+    inp: {
+      value: null
+    },
     disconnect: () => {}
   }
 
   if (!id || !isBrowserScope || !globalScope.MutationObserver || !globalScope.PerformanceObserver) return vitals
 
   const observers = []
 
-  const getNowRelativeToScriptStart = (capturedAt) => {
-    return capturedAt - (timings?.scriptStart || timings?.registeredAt || 0)
+  const populateVitalMinimums = () => {
+    fcpObservedAt ??= now()
+    lcpObservedAt ??= now()
+    vitals.cls.value ??= 0
   }
 
+  // if the MFE has already rendered something on the page before we could set up listeners, just populate vital minimums immediately
+  if (globalScope.document?.querySelector(`[data-nr-mfe-id="${id}"]`)) populateVitalMinimums()
+
   // Track FCP - first contentful paint
-  observeMutations(id, (node, obs) => {
-    if (vitals.fcp === null) {
-      const capturedAt = now()
-      vitals.fcp = {
-        get value () { return getNowRelativeToScriptStart(capturedAt) }
-      }
-      obs.disconnect()
-    }
+  observeMutations(id, (_, obs) => {
+    // An observed "FCP" means _something_ rendered, so at minimum we can populate all the baseline values for the vitals
+    populateVitalMinimums()
+    obs.disconnect()
   })
 
   // Track LCP - largest contentful paint
   let largestSize = 0
   const lcpObs = observeMutations(id, (node) => {
+    // an observed "LCP" means _something_ rendered, so at minimum we can make sure all the baseline values are populated for the vitals
+    populateVitalMinimums()
     try {
       const elem = node.nodeType === 1 ? node : node.parentElement
       if (!elem) return
       const rect = elem.getBoundingClientRect()
       const size = rect.width * rect.height
       if (size > largestSize) {
         largestSize = size
-        const capturedAt = now()
-        vitals.lcp = {
-          get value () { return getNowRelativeToScriptStart(capturedAt) }
-        }
+        lcpObservedAt = now()
       }
     } catch (e) {
       // Element may be detached from DOM
@@ -177,10 +196,11 @@ export function trackMFEVitals (id, timings) {
   // Track CLS - cumulative layout shift
   // Initialize CLS to 0 if browser supports it
   observePerformance(observers, { type: 'layout-shift', buffered: true }, (entry) => {
-    vitals.cls ??= { value: 0 }
     if (entry.hadRecentInput) return
     ;(entry.sources || []).some(source => {
       if (isInMFE(source.node, id)) {
+        // an observed "CLS" means _something_ rendered for the MFE, so at minimum we can make sure all the baseline values are populated for the vitals
+        populateVitalMinimums()
         vitals.cls.value += entry.value
         return true
       }
@@ -190,11 +210,11 @@ export function trackMFEVitals (id, timings) {
 
   // Track INP - interaction to next paint
   observePerformance(observers, { type: 'event', buffered: true, durationThreshold: 16 }, (entry) => {
-    if (!entry.interactionId || !entry.target || !isInMFE(entry.target, id)) return
-    if (vitals.inp === null || entry.duration > vitals.inp.value) {
-      vitals.inp = {
-        value: entry.duration
-      }
+    if (!entry.interactionId || !isInMFE(entry.target, id)) return
+    if (vitals.inp.value === null || entry.duration > vitals.inp.value) {
+      // an observed "INP" means _something_ rendered for the MFE, so at minimum we can make sure all the baseline values are populated for the vitals
+      populateVitalMinimums()
+      vitals.inp.value = entry.duration
     }
   })
 

tests/unit/common/v2/mfe-vitals.test.js

@@ -12,6 +12,7 @@ describe('trackMFEVitals', () => {
   let mutationCallbacks
   let performanceCallbacks
   let timings
+  const clockBaseline = 11
 
   beforeEach(async () => {
     jest.resetModules()
@@ -62,7 +63,9 @@ describe('trackMFEVitals', () => {
       isBrowserScope: true
     }))
 
-    // Mock now() to return incrementing values
+    // Mock now() to return incrementing values.
+    // The first relative LCP tick lands on `clockBaseline + 1` because the collector
+    // consumes one clock tick when it observes the node and another when it wins the size check.
     let timeCounter = 100
     jest.doMock('../../../../src/common/timing/now', () => ({
       now: jest.fn(() => timeCounter++)
@@ -81,7 +84,8 @@ describe('trackMFEVitals', () => {
   describe('FCP tracking', () => {
     it('should track FCP when contentful element is added to MFE', () => {
       const vitals = trackMFEVitals('test-mfe', timings)
-      expect(vitals.fcp).toBeNull()
+      // The collector seeds FCP as soon as it sees the MFE marker on the page.
+      expect(vitals.fcp.value).toBe(clockBaseline - 1)
 
       // Create a mock element with text content inside MFE container
       const mockContainer = {
@@ -107,7 +111,7 @@ describe('trackMFEVitals', () => {
       }]))
 
       expect(vitals.fcp).toBeDefined()
-      expect(vitals.fcp.value).toBe(10)
+      expect(vitals.fcp.value).toBe(clockBaseline - 1)
     })
 
     it('should use scriptStart as the timestamp anchor for FCP', () => {
@@ -132,7 +136,7 @@ describe('trackMFEVitals', () => {
 
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [mockElement] }]))
 
-      expect(vitals.fcp.value).toBe(10)
+      expect(vitals.fcp.value).toBe(clockBaseline - 1)
     })
   })
 
@@ -163,7 +167,7 @@ describe('trackMFEVitals', () => {
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [mockElement] }]))
 
       expect(vitals.lcp).toBeDefined()
-      expect(vitals.lcp.value).toBe(11)
+      expect(vitals.lcp.value).toBe(clockBaseline + 1)
     })
 
     it('should update LCP when larger element is added', () => {
@@ -204,8 +208,8 @@ describe('trackMFEVitals', () => {
 
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [largeElement] }]))
 
-      expect(firstLcp).toBe(11)
-      expect(vitals.lcp.value).toBe(12)
+      expect(firstLcp).toBe(clockBaseline + 1)
+      expect(vitals.lcp.value).toBe(clockBaseline + 2)
     })
 
     it('should extract URL from element background image', () => {
@@ -237,7 +241,7 @@ describe('trackMFEVitals', () => {
 
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [mockElement] }]))
 
-      expect(vitals.lcp.value).toBe(11)
+      expect(vitals.lcp.value).toBe(clockBaseline + 1)
     })
   })
 
@@ -352,7 +356,7 @@ describe('trackMFEVitals', () => {
 
       const vitals = trackMFEVitals('missing-mfe', timings)
 
-      expect(vitals.cls).toBeNull()
+      expect(vitals.cls.value).toBeNull()
     })
   })
 
@@ -491,12 +495,15 @@ describe('trackMFEVitals', () => {
         ]
       })
 
-      expect(vitals.inp).toBeNull()
+      expect(vitals.inp.value).toBeNull()
     })
   })
 
   describe('Element scope validation', () => {
     it('should only track elements within the specified MFE', () => {
+      const mockGlobalScope = require('../../../../src/common/constants/runtime').globalScope
+      mockGlobalScope.document.querySelector = jest.fn(() => null)
+
       const vitals = trackMFEVitals('mfe-a', timings)
 
       const mfeBContainer = {
@@ -518,7 +525,7 @@ describe('trackMFEVitals', () => {
 
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [mfeBElement] }]))
 
-      expect(vitals.fcp).toBeNull()
+      expect(vitals.fcp.value).toBeNull()
     })
 
     it('should track nested elements within MFE', () => {
@@ -545,9 +552,9 @@ describe('trackMFEVitals', () => {
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [childElement] }]))
 
       expect(vitals.fcp).toBeDefined()
-      expect(vitals.fcp.value).toBe(10)
+      expect(vitals.fcp.value).toBe(clockBaseline - 1)
       expect(vitals.lcp).toBeDefined()
-      expect(vitals.lcp.value).toBe(11)
+      expect(vitals.lcp.value).toBe(clockBaseline + 1)
     })
   })
 
@@ -593,10 +600,11 @@ describe('trackMFEVitals', () => {
     it('should return empty vitals when id is missing', () => {
       const vitals = trackMFEVitals('', timings)
 
-      expect(vitals.fcp).toBeNull()
-      expect(vitals.lcp).toBeNull()
-      expect(vitals.cls).toBeNull()
-      expect(vitals.inp).toBeNull()
+      // The collector still returns the vitals shell, but the value getters are never populated.
+      expect(Object.getOwnPropertyDescriptor(vitals.fcp, 'value')?.get).toEqual(expect.any(Function))
+      expect(Object.getOwnPropertyDescriptor(vitals.lcp, 'value')?.get).toEqual(expect.any(Function))
+      expect(vitals.cls.value).toBeNull()
+      expect(vitals.inp.value).toBeNull()
     })
 
     it('should handle elements without id or className', () => {
@@ -622,7 +630,7 @@ describe('trackMFEVitals', () => {
 
       mutationCallbacks.forEach(cb => cb([{ addedNodes: [mockElement] }]))
 
-      expect(vitals.lcp.value).toBe(11)
+      expect(vitals.lcp.value).toBe(clockBaseline + 1)
     })
 
     it('should handle elements with className as object', () => {