✨ (grapher) null values unset a property when merging

owid · Aug 8, 2024 · 3ecbe66 · 3ecbe66
1 parent f78facc
commit 3ecbe66
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 4 deletions.
diff --git a/packages/@ourworldindata/utils/src/Util.ts b/packages/@ourworldindata/utils/src/Util.ts
@@ -194,6 +194,10 @@ export type NoUndefinedValues<T> = {
     [P in keyof T]: Required<NonNullable<T[P]>>
 }
 
+export type DeepNullable<T> = {
+    [P in keyof T]: DeepNullable<T[P]> | null
+}
+
 type OptionalKeysOf<T> = Exclude<
     {
         [K in keyof T]: T extends Record<K, T[K]> ? never : K
@@ -1145,6 +1149,24 @@ export function omitUndefinedValuesRecursive<T extends Record<string, any>>(
     return result
 }
 
+export function omitNullValuesRecursive<T extends Record<string, any>>(
+    obj: T
+): NoUndefinedValues<T> {
+    const result: any = {}
+    for (const key in obj) {
+        if (isPlainObject(obj[key])) {
+            // re-apply the function if we encounter a non-empty object
+            result[key] = omitNullValuesRecursive(obj[key])
+        } else if (obj[key] === null) {
+            // omit null values
+        } else {
+            // otherwise, keep the value
+            result[key] = obj[key]
+        }
+    }
+    return result
+}
+
 export function omitEmptyObjectsRecursive<T extends Record<string, any>>(
     obj: T
 ): Partial<T> {

diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts
@@ -198,13 +198,70 @@ describe(mergeGrapherConfigs, () => {
         })
     })
 
-    it("overwrites values with an empty string if requested", () => {
+    it("removes a property that is set to null", () => {
+        expect(mergeGrapherConfigs({ selectedEntityNames: null })).toEqual({})
+        expect(mergeGrapherConfigs({}, { selectedEntityNames: null })).toEqual(
+            {}
+        )
+        expect(
+            mergeGrapherConfigs(
+                {
+                    title: "Parent title",
+                    selectedEntityNames: ["France"],
+                    hasMapTab: false,
+                    map: {
+                        time: 2000,
+                        hideTimeline: true,
+                    },
+                    hideAnnotationFieldsInTitle: {
+                        time: true,
+                    },
+                },
+                {
+                    title: "Child title",
+                    selectedEntityNames: null,
+                    hasMapTab: null,
+                    map: {
+                        time: 2000,
+                        hideTimeline: null,
+                    },
+                    hideAnnotationFieldsInTitle: null,
+                    tab: null,
+                }
+            )
+        ).toEqual({ title: "Child title", map: { time: 2000 } })
+        expect(
+            mergeGrapherConfigs(
+                {
+                    title: "Parent title",
+                    selectedEntityNames: ["France"],
+                },
+                {
+                    title: "Child title",
+                    // intermediate nulls are ignored
+                    selectedEntityNames: null,
+                },
+                { title: "Grandchild title", selectedEntityNames: ["Germany"] }
+            )
+        ).toEqual({
+            title: "Grandchild title",
+            selectedEntityNames: ["Germany"],
+        })
+    })
+
+    it("overwrites values with an empty string or empty array if requested", () => {
         expect(
             mergeGrapherConfigs(
                 { title: "Parent title", subtitle: "Parent subtitle" },
                 { subtitle: "" }
             )
         ).toEqual({ title: "Parent title", subtitle: "" })
+        expect(
+            mergeGrapherConfigs(
+                { title: "Parent title", selectedEntityNames: ["France"] },
+                { title: "Child title", selectedEntityNames: [] }
+            )
+        ).toEqual({ title: "Child title", selectedEntityNames: [] })
     })
 
     it("is associative", () => {

diff --git a/packages/@ourworldindata/utils/src/grapherConfigUtils.ts b/packages/@ourworldindata/utils/src/grapherConfigUtils.ts
@@ -10,6 +10,8 @@ import {
     omitEmptyObjectsRecursive,
     traverseObjects,
     isEmpty,
+    omitNullValuesRecursive,
+    type DeepNullable,
 } from "./Util"
 
 const REQUIRED_KEYS = ["$schema", "dimensions"]
@@ -23,13 +25,14 @@ const KEYS_EXCLUDED_FROM_INHERITANCE = [
 ]
 
 export function mergeGrapherConfigs(
-    ...grapherConfigs: GrapherInterface[]
+    ...grapherConfigs: DeepNullable<GrapherInterface>[]
 ): GrapherInterface {
     const configsToMerge = grapherConfigs.filter((c) => !isEmpty(c))
 
     // return early if there are no configs to merge
     if (configsToMerge.length === 0) return {}
-    if (configsToMerge.length === 1) return configsToMerge[0]
+    if (configsToMerge.length === 1)
+        return omitNullValuesRecursive(configsToMerge[0]) as GrapherInterface
 
     // warn if one of the configs is missing a schema version
     const configsWithoutSchema = configsToMerge.filter(
@@ -60,7 +63,8 @@ export function mergeGrapherConfigs(
         return omit(config, KEYS_EXCLUDED_FROM_INHERITANCE)
     })
 
-    return mergeWith(
+    // merge all configs
+    const mergedConfig = mergeWith(
         {}, // mergeWith mutates the first argument
         ...cleanedConfigs,
         (_: unknown, childValue: unknown): any => {
@@ -70,6 +74,9 @@ export function mergeGrapherConfigs(
             }
         }
     )
+
+    // null values unset a property, so omit it from the final config
+    return omitNullValuesRecursive(mergedConfig)
 }
 
 export function diffGrapherConfigs(