Skip to content

Commit

Permalink
✨ (grapher) ignore empty objects when merging configs
Browse files Browse the repository at this point in the history
  • Loading branch information
sophiamersmann committed Aug 8, 2024
1 parent 9f6b00e commit d9f616a
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 8 deletions.
34 changes: 32 additions & 2 deletions packages/@ourworldindata/utils/src/grapherConfigUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ describe(mergeGrapherConfigs, () => {
).toThrowError()
})

it("excludes $schema, id, slug, version and isPublished from inheritance", () => {
it("excludes id, slug, version and isPublished from inheritance", () => {
expect(
mergeGrapherConfigs(
{
Expand All @@ -165,7 +165,37 @@ describe(mergeGrapherConfigs, () => {
},
{ slug: "child-slug", version: 1, title: "Title B" }
)
).toEqual({ slug: "child-slug", version: 1, title: "Title B" })
).toEqual({
slug: "child-slug",
version: 1,
title: "Title B",
})
})

it("ignores empty objects", () => {
expect(
mergeGrapherConfigs(
{
title: "Parent title",
subtitle: "Parent subtitle",
},
{
$schema: "004",
id: 1,
slug: "parent-slug",
version: 1,
title: "Title A",
},
{}
)
).toEqual({
$schema: "004",
id: 1,
slug: "parent-slug",
version: 1,
title: "Title A",
subtitle: "Parent subtitle",
})
})

it("overwrites values with an empty string if requested", () => {
Expand Down
19 changes: 13 additions & 6 deletions packages/@ourworldindata/utils/src/grapherConfigUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
omitUndefinedValuesRecursive,
omitEmptyObjectsRecursive,
traverseObjects,
isEmpty,
} from "./Util"

const REQUIRED_KEYS = ["$schema", "dimensions"]
Expand All @@ -24,20 +25,26 @@ const KEYS_EXCLUDED_FROM_INHERITANCE = [
export function mergeGrapherConfigs(
...grapherConfigs: 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]

// warn if one of the configs is missing a schema version
const configsWithoutSchema = grapherConfigs.filter(
const configsWithoutSchema = configsToMerge.filter(
(c) => c["$schema"] === undefined
)
if (configsWithoutSchema.length > 0) {
const ids = configsWithoutSchema.map((c) => c.id)
const configsJson = JSON.stringify(configsWithoutSchema, null, 2)
console.warn(
`About to merge Grapher configs with missing schema information. Charts with missing schema information: ${ids}`
`About to merge Grapher configs with missing schema information: ${configsJson}`
)
}

// abort if the grapher configs have different schema versions
const uniqueSchemas = uniq(
excludeUndefined(grapherConfigs.map((c) => c["$schema"]))
excludeUndefined(configsToMerge.map((c) => c["$schema"]))
)
if (uniqueSchemas.length > 1) {
throw new Error(
Expand All @@ -48,8 +55,8 @@ export function mergeGrapherConfigs(
}

// keys that should not be inherited are removed from all but the last config
const cleanedConfigs = grapherConfigs.map((config, index) => {
if (index === grapherConfigs.length - 1) return config
const cleanedConfigs = configsToMerge.map((config, index) => {
if (index === configsToMerge.length - 1) return config
return omit(config, KEYS_EXCLUDED_FROM_INHERITANCE)
})

Expand Down

0 comments on commit d9f616a

Please sign in to comment.