Skip to content

Commit

Permalink
🐛 Fix poor merge behavior of Config + overrides (#1519)
Browse files Browse the repository at this point in the history
  • Loading branch information
TheAfroOfDoom authored Jul 30, 2024
1 parent db02249 commit 1a469d7
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 9 deletions.
131 changes: 131 additions & 0 deletions __snapshots__/packages/core/test-out/service/Config.spec.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 2 additions & 9 deletions packages/core/src/service/Config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import rfdc from 'rfdc'
import type { ExternalEventEmitter } from '../common/index.js'
import { Arrayable, bufferToString, TypePredicates } from '../common/index.js'
import { Arrayable, bufferToString, merge, TypePredicates } from '../common/index.js'
import { ErrorSeverity } from '../source/index.js'
import { FileCategories, RegistryCategories } from '../symbol/index.js'
import type { Project } from './Project.js'
Expand Down Expand Up @@ -504,13 +504,6 @@ export class ConfigService implements ExternalEventEmitter {
}

public static merge(base: Config, ...overrides: any[]): Config {
// FIXME
const ans = rfdc()(base)
for (const override of overrides) {
for (const key of ['env', 'format', 'lint', 'snippet'] as const) {
ans[key] = { ...ans[key], ...override[key] } as any
}
}
return ans
return overrides.reduce(merge, rfdc()(base))
}
}
56 changes: 56 additions & 0 deletions packages/core/test/service/Config.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import assert from 'assert'
import { describe, it } from 'mocha'
import snapshot from 'snap-shot-it'
import type { Config } from '../../lib/index.js'
import { ConfigService } from '../../lib/index.js'

describe('ConfigService', () => {
describe('merge()', () => {
const base = {
env: {
dataSource: 'GitHub',
dependencies: [
'@vanilla-mcdoc',
],
feature: {
a: true,
b: false,
},
},
} as unknown as Config

it('Should create a clone of the base object', async () => {
const merged = ConfigService.merge(base)
merged.env.dataSource = 'new string'
assert.notEqual(base.env.dataSource, merged.env.dataSource)
})

it('Should merge empty overrides correctly', async () => {
snapshot(ConfigService.merge(base))
snapshot(ConfigService.merge(base, {}))
snapshot(ConfigService.merge(base, {}, {}))
})

it('Should merge top-level overrides correctly', async () => {
snapshot(ConfigService.merge(base, { test: true }))
})

it('Should merge nested overrides correctly', async () => {
snapshot(ConfigService.merge(base, { env: { dataSource: 'TEST' } }))
snapshot(ConfigService.merge(base, { env: { feature: {} } }))
snapshot(ConfigService.merge(base, { env: { feature: { b: true } } }))
snapshot(ConfigService.merge(base, { env: { feature: { b: true, c: 9 } } }))
snapshot(ConfigService.merge(base, { env: { dependencies: [] } }))
})

it('Should merge multiple overrides correctly', async () => {
snapshot(
ConfigService.merge(
base,
{ env: { dataSource: 'TEST', foo: 'bar' } },
{ env: { foo: 'qux', erm: 3 } },
),
)
})
})
})

0 comments on commit 1a469d7

Please sign in to comment.