Skip to content

Commit

Permalink
fix: only save package-lock when truly finished
Browse files Browse the repository at this point in the history
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.