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.

Sign up for GitHub

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

Jump to bottom

fix: only save package-lock when truly finished #6095

Merged
merged 2 commits into from Feb 1, 2023
Merged

fix: only save package-lock when truly finished #6095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b6bea5
chore: clean snapshot before code change
wraithgar Jan 26, 2023
e05f890
fix: only save package-lock when truly finished
wraithgar Jan 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 37 additions & 49 deletions workspaces/arborist/lib/arborist/reify.js
View file
Open in desktop
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 @@ -1380,64 +1379,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
157 changes: 3 additions & 154 deletions workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs
View file
Open in desktop
Expand Up @@ -32352,157 +32352,6 @@ ArboristNode {
}
`

exports[`test/arborist/reify.js TAP rollbacks fail retiring node, but then rimraf fixes it > expect resolving Promise 1`] = `
ArboristNode {
"children": Map {
"@isaacs/testing-bundledeps-parent" => ArboristNode {
"children": Map {
"@isaacs/testing-bundledeps" => ArboristNode {
"bundleDependencies": Array [
"@isaacs/testing-bundledeps-a",
],
"children": Map {
"@isaacs/testing-bundledeps-a" => ArboristNode {
"bundled": true,
"bundler": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"name": "@isaacs/testing-bundledeps-a",
"spec": "*",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-bundledeps-b" => EdgeOut {
"name": "@isaacs/testing-bundledeps-b",
"spec": "*",
"to": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
"type": "prod",
},
},
"location": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
"name": "@isaacs/testing-bundledeps-a",
"path": "{CWD}/test/arborist/tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it/node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
"resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-a/-/testing-bundledeps-a-1.0.0.tgz",
"version": "1.0.0",
},
"@isaacs/testing-bundledeps-b" => ArboristNode {
"bundled": true,
"bundler": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
"name": "@isaacs/testing-bundledeps-b",
"spec": "*",
"type": "prod",
},
EdgeIn {
"from": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-c",
"name": "@isaacs/testing-bundledeps-b",
"spec": "*",
"type": "prod",
},
},
"location": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
"name": "@isaacs/testing-bundledeps-b",
"path": "{CWD}/test/arborist/tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it/node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
"resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-b/-/testing-bundledeps-b-1.0.0.tgz",
"version": "1.0.0",
},
"@isaacs/testing-bundledeps-c" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"name": "@isaacs/testing-bundledeps-c",
"spec": "*",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-bundledeps-b" => EdgeOut {
"name": "@isaacs/testing-bundledeps-b",
"spec": "*",
"to": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-b",
"type": "prod",
},
},
"location": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-c",
"name": "@isaacs/testing-bundledeps-c",
"path": "{CWD}/test/arborist/tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it/node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-c",
"resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-c/-/testing-bundledeps-c-2.0.0.tgz",
"version": "2.0.0",
},
},
"edgesIn": Set {
EdgeIn {
"from": "node_modules/@isaacs/testing-bundledeps-parent",
"name": "@isaacs/testing-bundledeps",
"spec": "^1.0.0",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-bundledeps-a" => EdgeOut {
"name": "@isaacs/testing-bundledeps-a",
"spec": "*",
"to": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-a",
"type": "prod",
},
"@isaacs/testing-bundledeps-c" => EdgeOut {
"name": "@isaacs/testing-bundledeps-c",
"spec": "*",
"to": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps/node_modules/@isaacs/testing-bundledeps-c",
"type": "prod",
},
},
"location": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"name": "@isaacs/testing-bundledeps",
"path": "{CWD}/test/arborist/tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it/node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps/-/testing-bundledeps-1.0.0.tgz",
"version": "1.0.0",
},
},
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "@isaacs/testing-bundledeps-parent",
"spec": "*",
"type": "prod",
},
},
"edgesOut": Map {
"@isaacs/testing-bundledeps" => EdgeOut {
"name": "@isaacs/testing-bundledeps",
"spec": "^1.0.0",
"to": "node_modules/@isaacs/testing-bundledeps-parent/node_modules/@isaacs/testing-bundledeps",
"type": "prod",
},
},
"location": "node_modules/@isaacs/testing-bundledeps-parent",
"name": "@isaacs/testing-bundledeps-parent",
"path": "{CWD}/test/arborist/tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it/node_modules/@isaacs/testing-bundledeps-parent",
"resolved": "https://registry.npmjs.org/@isaacs/testing-bundledeps-parent/-/testing-bundledeps-parent-2.0.0.tgz",
"version": "2.0.0",
},
},
"edgesOut": Map {
"@isaacs/testing-bundledeps-parent" => EdgeOut {
"name": "@isaacs/testing-bundledeps-parent",
"spec": "*",
"to": "node_modules/@isaacs/testing-bundledeps-parent",
"type": "prod",
},
},
"isProjectRoot": true,
"location": "",
"name": "tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it",
"packageName": "testing-bundledeps-3",
"path": "{CWD}/test/arborist/tap-testdir-reify-rollbacks-fail-retiring-node-but-then-rimraf-fixes-it",
"version": "1.0.0",
}
`

exports[`test/arborist/reify.js TAP running lifecycle scripts of unchanged link nodes on reify > result 1`] = `
ArboristNode {
"children": Map {
Expand Down Expand Up @@ -32826,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 @@ -32838,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 @@ -32917,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