Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: only save package-lock when truly finished
When we update the trees, the edges are refreshed to correct their
specs.  If we have written the package-lock before this point, it will
be incorrect.

No new tests are needed as existing tests were snapshotted w/ the broken
behavior, and are fixed now showing this bugfix in action.
  • Loading branch information
wraithgar authored and fritzy committed Feb 1, 2023
1 parent 5ec35b7 commit 72a7a59
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 52 deletions.
86 changes: 37 additions & 49 deletions workspaces/arborist/lib/arborist/reify.js
Expand Up @@ -82,7 +82,6 @@ const _rollbackRetireShallowNodes = Symbol.for('rollbackRetireShallowNodes')
const _rollbackCreateSparseTree = Symbol.for('rollbackCreateSparseTree')
const _rollbackMoveBackRetiredUnchanged = Symbol.for('rollbackMoveBackRetiredUnchanged')
const _saveIdealTree = Symbol.for('saveIdealTree')
const _saveLockFile = Symbol('saveLockFile')
const _copyIdealToActual = Symbol('copyIdealToActual')
const _addOmitsToTrashList = Symbol('addOmitsToTrashList')
const _packageLockOnly = Symbol('packageLockOnly')
Expand Down Expand Up @@ -1404,64 +1403,53 @@ module.exports = cls => class Reifier extends cls {
}
}

// preserve indentation, if possible
const {
[Symbol.for('indent')]: indent,
} = this.idealTree.package
const format = indent === undefined ? ' ' : indent

const saveOpt = {
format: (this[_formatPackageLock] && format) ? format
: this[_formatPackageLock],
}

const promises = [this[_saveLockFile](saveOpt)]

const updatePackageJson = async (tree) => {
const pkgJson = await PackageJson.load(tree.path)
.catch(() => new PackageJson(tree.path))
const {
dependencies = {},
devDependencies = {},
optionalDependencies = {},
peerDependencies = {},
// bundleDependencies is not required by PackageJson like the other fields here
// PackageJson also doesn't omit an empty array for this field so defaulting this
// to an empty array would add that field to every package.json file.
bundleDependencies,
} = tree.package

pkgJson.update({
dependencies,
devDependencies,
optionalDependencies,
peerDependencies,
bundleDependencies,
})
await pkgJson.save()
}

if (save) {
for (const tree of updatedTrees) {
// refresh the edges so they have the correct specs
tree.package = tree.package
promises.push(updatePackageJson(tree))
const pkgJson = await PackageJson.load(tree.path)
.catch(() => new PackageJson(tree.path))
const {
dependencies = {},
devDependencies = {},
optionalDependencies = {},
peerDependencies = {},
// bundleDependencies is not required by PackageJson like the other
// fields here PackageJson also doesn't omit an empty array for this
// field so defaulting this to an empty array would add that field to
// every package.json file.
bundleDependencies,
} = tree.package

pkgJson.update({
dependencies,
devDependencies,
optionalDependencies,
peerDependencies,
bundleDependencies,
})
await pkgJson.save()
}
}

await Promise.all(promises)
process.emit('timeEnd', 'reify:save')
return true
}
// before now edge specs could be changing, affecting the `requires` field
// in the package lock, so we hold off saving to the very last action
if (this[_usePackageLock]) {
// preserve indentation, if possible
let format = this.idealTree.package[Symbol.for('indent')]
if (format === undefined) {
format = ' '
}

async [_saveLockFile] (saveOpt) {
if (!this[_usePackageLock]) {
return
// TODO this ignores options.save
await this.idealTree.meta.save({
format: (this[_formatPackageLock] && format) ? format
: this[_formatPackageLock],
})
}

const { meta } = this.idealTree

return meta.save(saveOpt)
process.emit('timeEnd', 'reify:save')
return true
}

async [_copyIdealToActual] () {
Expand Down
Expand Up @@ -32675,7 +32675,7 @@ exports[`test/arborist/reify.js TAP save package.json on update should update na
"a": {
"version": "file:a",
"requires": {
"abbrev": "^1.0.4",
"abbrev": "^1.1.1",
"once": "^1.3.2"
}
},
Expand All @@ -32687,7 +32687,7 @@ exports[`test/arborist/reify.js TAP save package.json on update should update na
"b": {
"version": "file:b",
"requires": {
"abbrev": "^1.0.4"
"abbrev": "^1.1.1"
}
},
"once": {
Expand Down Expand Up @@ -32766,7 +32766,7 @@ exports[`test/arborist/reify.js TAP save package.json on update should update si
"version": "file:a",
"requires": {
"abbrev": "^1.0.4",
"once": "^1.3.2"
"once": "^1.4.0"
}
},
"abbrev": {
Expand Down

0 comments on commit 72a7a59

Please sign in to comment.