♻️ [PANA-5948] Make some small improvements to ItemId management (#4164)

Author: sethfowler-datadog

packages/rum/src/domain/record/itemIds.spec.ts

@@ -51,6 +51,33 @@ describe('ItemIds', () => {
         })
       })
 
+      describe('delete', () => {
+        it('allows an item to be re-inserted and receive a new id', () => {
+          const item = createItem()
+          expect(itemIds.getOrInsert(item)).toBe(firstId)
+          expect(itemIds.getOrInsert(item)).toBe(firstId)
+
+          itemIds.delete(item)
+
+          expect(itemIds.getOrInsert(item)).toBe((firstId + 1) as ItemId)
+          expect(itemIds.getOrInsert(item)).toBe((firstId + 1) as ItemId)
+        })
+
+        it('does not change the next assigned id', () => {
+          const item = createItem()
+          expect(itemIds.getOrInsert(item)).toBe(firstId)
+          expect(itemIds.getOrInsert(item)).toBe(firstId)
+          expect(itemIds.nextId).toBe((firstId + 1) as ItemId)
+
+          itemIds.delete(item)
+          expect(itemIds.nextId).toBe((firstId + 1) as ItemId)
+
+          const newItem = createItem()
+          expect(itemIds.getOrInsert(newItem)).toBe((firstId + 1) as ItemId)
+          expect(itemIds.getOrInsert(newItem)).toBe((firstId + 1) as ItemId)
+        })
+      })
+
       describe('get', () => {
         it('returns undefined for items that have not been assigned an id', () => {
           expect(itemIds.get(createItem())).toBe(undefined)
@@ -82,6 +109,21 @@ describe('ItemIds', () => {
         })
       })
 
+      describe('nextId getter', () => {
+        it('initially returns the first id', () => {
+          expect(itemIds.nextId).toBe(firstId)
+        })
+
+        it('increments after each insertion', () => {
+          for (let id = firstId; id < firstId + 3; id++) {
+            const item = createItem()
+            expect(itemIds.getOrInsert(item)).toBe(id)
+            expect(itemIds.getOrInsert(item)).toBe(id)
+            expect(itemIds.nextId).toBe((id + 1) as ItemId)
+          }
+        })
+      })
+
       describe('size', () => {
         it('increments when an id is assigned', () => {
           expect(itemIds.size).toBe(0)

packages/rum/src/domain/record/itemIds.ts

@@ -36,8 +36,10 @@ export function createStyleSheetIds(): StyleSheetIds {
 
 export interface ItemIds<ItemType, ItemId extends number> {
   clear(this: void): void
+  delete(this: void, item: ItemType): void
   get(this: void, item: ItemType): ItemId | undefined
   getOrInsert(this: void, item: ItemType): ItemId
+  get nextId(): ItemId
   get size(): number
 }
 
@@ -50,6 +52,7 @@ function createWeakIdMap<ItemType extends object, ItemId extends number>(firstId
 }
 
 interface MapLike<Key, Value> {
+  delete(key: Key): void
   get(key: Key): Value | undefined
   set(key: Key, value: Value): void
 }
@@ -65,9 +68,15 @@ function createItemIds<ItemType, ItemId extends number>(
 
   return {
     clear(): void {
+      if (nextId === firstId) {
+        return
+      }
       map = createMap()
       nextId = firstId
     },
+    delete(object: ItemType): void {
+      map.delete(object)
+    },
     get,
     getOrInsert(object: ItemType): ItemId {
       // Try to reuse any existing id.
@@ -78,6 +87,9 @@ function createItemIds<ItemType, ItemId extends number>(
       }
       return id
     },
+    get nextId(): ItemId {
+      return nextId
+    },
     get size(): number {
       return nextId - firstId
     },

packages/rum/src/domain/record/recordingScope.ts

@@ -32,12 +32,13 @@ export function createRecordingScope(
   const nodeIds = createNodeIds()
   const stringIds = createStringIds()
   const styleSheetIds = createStyleSheetIds()
-  return {
+
+  const scope: RecordingScope = {
     resetIds(): void {
-      eventIds.clear()
-      nodeIds.clear()
-      stringIds.clear()
-      styleSheetIds.clear()
+      scope.eventIds.clear()
+      scope.nodeIds.clear()
+      scope.stringIds.clear()
+      scope.styleSheetIds.clear()
     },
 
     configuration,
@@ -48,4 +49,6 @@ export function createRecordingScope(
     stringIds,
     styleSheetIds,
   }
+
+  return scope
 }

packages/rum/src/domain/record/startFullSnapshots.ts

@@ -69,9 +69,7 @@ export function takeFullSnapshot(
   })
 
   if (isExperimentalFeatureEnabled(ExperimentalFeature.USE_CHANGE_RECORDS)) {
-    if (kind === SerializationKind.SUBSEQUENT_FULL_SNAPSHOT) {
-      scope.resetIds()
-    }
+    scope.resetIds()
     serializeChangesInTransaction(
       kind,
       emitRecord,