From 7a1f485ad9a64491fefcbc788a6701d0198a2aed Mon Sep 17 00:00:00 2001
From: Alfonso Noriega <alfonso.noriega@shopify.com>
Date: Mon, 13 Apr 2026 16:00:28 +0200
Subject: [PATCH] refactor: replace JSON round-trip with explicit
 deepStripUndefined utility
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The analytics payload builder used JSON.parse(JSON.stringify(payload))
to strip undefined fields. While functionally correct, the intent is
non-obvious — it relies on a side-effect of JSON serialization.

Added a dedicated deepStripUndefined() utility to cli-kit's object
module with full test coverage, and replaced the JSON round-trip in
analytics.ts. This makes the code self-documenting.

Note: this is a readability improvement, not a performance one — the
operation runs once per CLI command and takes ~2µs either way.
---
 .../cli-kit/src/public/common/object.test.ts  | 39 +++++++++++++++++++
 packages/cli-kit/src/public/common/object.ts  | 23 +++++++++++
 packages/cli-kit/src/public/node/analytics.ts |  3 +-
 3 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/packages/cli-kit/src/public/common/object.test.ts b/packages/cli-kit/src/public/common/object.test.ts
index 93a116639c0..08c72914ae8 100644
--- a/packages/cli-kit/src/public/common/object.test.ts
+++ b/packages/cli-kit/src/public/common/object.test.ts
@@ -3,6 +3,7 @@ import {
   deepCompare,
   deepDifference,
   deepMergeObjects,
+  deepStripUndefined,
   getPathValue,
   mapValues,
   pickBy,
@@ -440,3 +441,41 @@ describe('unsetPathValue', () => {
     expect(obj).toEqual({regular: 'value2'})
   })
 })
+
+describe('deepStripUndefined', () => {
+  test('removes top-level undefined fields', () => {
+    const obj = {a: 1, b: undefined, c: 'hello'}
+    expect(deepStripUndefined(obj)).toEqual({a: 1, c: 'hello'})
+  })
+
+  test('removes nested undefined fields', () => {
+    const obj = {outer: {a: 1, b: undefined}, c: 'hello'}
+    expect(deepStripUndefined(obj)).toEqual({outer: {a: 1}, c: 'hello'})
+  })
+
+  test('preserves arrays and recurses into them', () => {
+    const obj = {list: [{a: 1, b: undefined}, {c: undefined, d: 2}]}
+    expect(deepStripUndefined(obj)).toEqual({list: [{a: 1}, {d: 2}]})
+  })
+
+  test('returns primitives as-is', () => {
+    expect(deepStripUndefined('hello')).toBe('hello')
+    expect(deepStripUndefined(42)).toBe(42)
+    expect(deepStripUndefined(null)).toBe(null)
+    expect(deepStripUndefined(true)).toBe(true)
+  })
+
+  test('preserves null values (only strips undefined)', () => {
+    const obj = {a: null, b: undefined, c: 0, d: false, e: ''}
+    expect(deepStripUndefined(obj)).toEqual({a: null, c: 0, d: false, e: ''})
+  })
+
+  test('handles empty objects', () => {
+    expect(deepStripUndefined({})).toEqual({})
+  })
+
+  test('handles objects where all values are undefined', () => {
+    const obj = {a: undefined, b: undefined}
+    expect(deepStripUndefined(obj)).toEqual({})
+  })
+})
diff --git a/packages/cli-kit/src/public/common/object.ts b/packages/cli-kit/src/public/common/object.ts
index 23e6e122df8..0ef0f01bb49 100644
--- a/packages/cli-kit/src/public/common/object.ts
+++ b/packages/cli-kit/src/public/common/object.ts
@@ -134,3 +134,26 @@ export function isEmpty(object: object): boolean {
 export function compact(object: object): object {
   return Object.fromEntries(Object.entries(object).filter(([_, value]) => value != null))
 }
+
+/**
+ * Recursively removes properties with `undefined` values from an object.
+ * Arrays are traversed but not filtered. Non-object values are returned as-is.
+ *
+ * @param value - The value to strip undefined fields from.
+ * @returns A deep copy of the value with all undefined-valued keys removed.
+ */
+export function deepStripUndefined<T>(value: T): T {
+  if (Array.isArray(value)) {
+    return value.map(deepStripUndefined) as T
+  }
+  if (value !== null && typeof value === 'object') {
+    const result: Record<string, unknown> = {}
+    for (const [key, val] of Object.entries(value)) {
+      if (val !== undefined) {
+        result[key] = deepStripUndefined(val)
+      }
+    }
+    return result as T
+  }
+  return value
+}
diff --git a/packages/cli-kit/src/public/node/analytics.ts b/packages/cli-kit/src/public/node/analytics.ts
index 5578e3af328..1fe1bf4056b 100644
--- a/packages/cli-kit/src/public/node/analytics.ts
+++ b/packages/cli-kit/src/public/node/analytics.ts
@@ -1,4 +1,5 @@
 import {alwaysLogAnalytics, alwaysLogMetrics, analyticsDisabled, isShopify} from './context/local.js'
+import {deepStripUndefined} from '../common/object.js'
 import * as metadata from './metadata.js'
 import {publishMonorailEvent, MONORAIL_COMMAND_TOPIC} from './monorail.js'
 import {fanoutHooks} from './plugins.js'
@@ -193,7 +194,7 @@ async function buildPayload({config, errorMessage, exitMode}: ReportAnalyticsEve
   })
 
   // strip undefined fields -- they make up the majority of payloads due to wide metadata structure.
-  payload = JSON.parse(JSON.stringify(payload))
+  payload = deepStripUndefined(payload)
 
   return sanitizePayload(payload)
 }