Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: properly check for diff with multiple files from overwrite:false #442

Merged
merged 1 commit into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/check/check-apply.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ const run = async (type, dir, files, options) => {
const { add: addFiles, rm: rmFiles } = files

const rm = await rmEach(dir, rmFiles, options, (f) => rel(f))
const parseOpts = { allowMultipleSources: false }
const [add, update] = partition(await parseEach(dir, addFiles, options, parseOpts, async (p) => {
const [add, update] = partition(await parseEach(dir, addFiles, options, {}, async (p) => {
const diff = await p.applyDiff()
const target = rel(p.target)
if (diff === null) {
Expand Down
27 changes: 13 additions & 14 deletions lib/util/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,28 @@ const mergeFiles = mergeWithCustomizers((value, srcValue, key, target, source, s
}
}, customizers.overwriteArrays)

const fileEntries = (dir, files, options, { allowMultipleSources = true } = {}) => {
const fileEntries = (dir, files, options) => {
const results = []

for (const [key, source] of Object.entries(files)) {
for (const [key, value] of Object.entries(files)) {
// remove any false values first since that means those targets are skipped
if (source === false) {
if (value === false) {
continue
}

// target paths need to be joined with dir and templated
const target = join(dir, template(key, options))

if (Array.isArray(source)) {
// When turning an object of files into all its entries, we allow
// multiples when applying changes, but not when checking for changes
// since earlier files would always return as needing an update. So we
// either allow multiples and return the array or only return the last
// source file in the array.
const sources = allowMultipleSources ? source : source.slice(-1)
results.push(...sources.map(s => [target, s]))
} else {
results.push([target, source])
}
// Allow an array of values to merge into a single source to be
// applied or diffed against the target. This is how overwrite:false
// works and they are merged.
const source = Array.isArray(value)
? value.reduce((acc, { file, ...rest }) => {
acc.file.push(file)
return Object.assign(acc, rest)
}, { file: [] }) : value

results.push([target, source])
}

return results
Expand Down
9 changes: 9 additions & 0 deletions lib/util/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,16 @@ class Base {
}

read (s) {
if (Array.isArray(s)) {
return Promise.all(s.map(f => this.read(f)))
}
return fs.readFile(s, { encoding: 'utf-8' })
}

template (s) {
if (Array.isArray(s)) {
return Promise.all(s.map(f => this.template(f)))
}
return template(s, this.options)
}

Expand Down Expand Up @@ -285,6 +291,9 @@ class Json extends Base {
}

parse (s) {
if (Array.isArray(s)) {
return s.map(f => this.parse(f)).reduce((a, f) => this.merge(a, f), {})
}
return jsonParse(s)
}

Expand Down
21 changes: 21 additions & 0 deletions test/apply/overwrite-false.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,31 @@ t.test('json merge', async (t) => {
})

await s.apply()
t.strictSame(await s.check(), [])

const pkg = await s.readJson('package.json')
t.equal(pkg.scripts.test, 'tap test/')
t.equal(pkg.scripts.snap, 'tap')

await s.writeJson('package.json', {
...pkg,
author: 'What?',
scripts: {
...pkg.scripts,
test: 'tap',
},
templateOSS: {
...pkg.templateOSS,
version: '1.0.0',
},
})

const checks = await s.check()
t.equal(checks.length, 1)
t.match(checks[0].title, 'package.json needs to be updated')
t.match(checks[0].body[0], `"author" is "What?", expected "GitHub Inc."`)
t.match(checks[0].body[0], `scripts.test" is "tap", expected "tap test/"`)

await s.apply()
t.strictSame(await s.check(), [])
})
Loading