Skip to content

Commit 491caba

Browse files
jdaltonjdalton
committed
Rework loop logic in pnpm fix to try to open more prs
1 parent 2020a2f commit 491caba

File tree

1 file changed

+118
-142
lines changed

1 file changed

+118
-142
lines changed

‎src/commands/fix/pnpm-fix.ts‎

Lines changed: 118 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -157,31 +157,47 @@ export async function pnpmFix(
157157
continue
158158
}
159159

160-
const failedSpecs = new Set<string>()
161160
const fixedSpecs = new Set<string>()
162-
const installedSpecs = new Set<string>()
163-
const testedSpecs = new Set<string>()
164-
const unavailableSpecs = new Set<string>()
165-
const revertedSpecs = new Set<string>()
166161

167162
for (const pkgJsonPath of pkgJsonPaths) {
163+
// Re-read actualTree to avoid lockfile state issues
164+
// eslint-disable-next-line no-await-in-loop
165+
actualTree = await getActualTree(cwd)
166+
167+
const pkgPath = path.dirname(pkgJsonPath)
168+
const isWorkspaceRoot =
169+
pkgJsonPath === pkgEnvDetails.editablePkgJson.filename
170+
const workspaceName = isWorkspaceRoot
171+
? 'root'
172+
: path.relative(rootPath, pkgPath)
173+
174+
const editablePkgJson = isWorkspaceRoot
175+
? pkgEnvDetails.editablePkgJson
176+
: // eslint-disable-next-line no-await-in-loop
177+
await readPackageJson(pkgJsonPath, { editable: true })
178+
179+
// Get current overrides for revert logic
180+
const oldPnpmSection = editablePkgJson.content[PNPM] as
181+
| StringKeyValueObject
182+
| undefined
183+
const oldOverrides = oldPnpmSection?.[OVERRIDES] as
184+
| Record<string, string>
185+
| undefined
186+
168187
for (const oldVersion of oldVersions) {
169188
const oldSpec = `${name}@${oldVersion}`
170189
const oldPurl = `pkg:npm/${oldSpec}`
171190

191+
const node = findPackageNode(actualTree, name, oldVersion)
192+
if (!node) {
193+
debugLog(`Skipping ${oldSpec}, no node found in ${pkgJsonPath}`)
194+
continue
195+
}
196+
172197
for (const {
173198
firstPatchedVersionIdentifier,
174199
vulnerableVersionRange
175200
} of infos) {
176-
const node = findPackageNode(actualTree, name, oldVersion)
177-
if (!node) {
178-
debugLog(
179-
`Skipping ${oldSpec}, no node found in arborist.actualTree`,
180-
pkgJsonPath
181-
)
182-
continue
183-
}
184-
185201
const availableVersions = Object.keys(packument.versions)
186202
const newVersion = findBestPatchVersion(
187203
node,
@@ -194,99 +210,65 @@ export async function pnpmFix(
194210
: undefined
195211

196212
if (!(newVersion && newVersionPackument)) {
197-
if (!unavailableSpecs.has(oldSpec)) {
198-
unavailableSpecs.add(oldSpec)
199-
spinner?.fail(`No update available for ${oldSpec}`)
200-
}
213+
spinner?.fail(`No update available for ${oldSpec}`)
201214
continue
202215
}
203216

204-
const isWorkspaceRoot =
205-
pkgJsonPath === pkgEnvDetails.editablePkgJson.filename
206-
const workspaceName = isWorkspaceRoot
207-
? ''
208-
: path.relative(rootPath, path.dirname(pkgJsonPath))
209-
const workspaceDetails = workspaceName ? ` in ${workspaceName}` : ''
210-
const editablePkgJson = isWorkspaceRoot
211-
? pkgEnvDetails.editablePkgJson
212-
: // eslint-disable-next-line no-await-in-loop
213-
await readPackageJson(pkgJsonPath, { editable: true })
214-
215-
const oldPnpm = editablePkgJson.content[PNPM] as
216-
| StringKeyValueObject
217-
| undefined
218-
const oldPnpmKeyCount = oldPnpm ? Object.keys(oldPnpm).length : 0
219-
const oldOverrides = (oldPnpm as StringKeyValueObject)?.[OVERRIDES] as
220-
| Record<string, string>
221-
| undefined
222-
const oldOverridesCount = oldOverrides
223-
? Object.keys(oldOverrides).length
224-
: 0
225-
226217
const overrideKey = `${name}@${vulnerableVersionRange}`
227218
const newVersionRange = applyRange(
228219
oldOverrides?.[overrideKey] ?? oldVersion,
229220
newVersion,
230221
rangeStyle
231222
)
232223
const newSpec = `${name}@${newVersionRange}`
233-
const newSpecKey = `${workspaceName ? `${workspaceName}>` : ''}${newSpec}`
224+
const newSpecKey = `${workspaceName}:${newSpec}`
225+
226+
if (fixedSpecs.has(newSpecKey)) {
227+
debugLog(`Already fixed ${newSpec} in ${workspaceName}, skipping`)
228+
continue
229+
}
234230

235231
const updateData = isWorkspaceRoot
236-
? {
232+
? ({
237233
[PNPM]: {
238-
...oldPnpm,
234+
...oldPnpmSection,
239235
[OVERRIDES]: {
240-
[overrideKey]: newVersionRange,
241-
...oldOverrides
236+
...oldOverrides,
237+
[overrideKey]: newVersionRange
242238
}
243239
}
244-
}
240+
} as PackageJson)
245241
: undefined
246242

247243
const revertData = {
248244
...(isWorkspaceRoot
249245
? {
250-
[PNPM]: oldPnpmKeyCount
251-
? {
252-
...oldPnpm,
253-
[OVERRIDES]:
254-
oldOverridesCount === 1
255-
? undefined
256-
: {
257-
[overrideKey]: undefined,
258-
...oldOverrides
259-
}
260-
}
261-
: undefined
246+
[PNPM]: {
247+
...oldPnpmSection,
248+
[OVERRIDES]:
249+
oldOverrides && Object.keys(oldOverrides).length > 1
250+
? {
251+
...oldOverrides,
252+
[overrideKey]: undefined
253+
}
254+
: undefined
255+
}
262256
}
263257
: {}),
264-
...(editablePkgJson.content.dependencies
265-
? { dependencies: editablePkgJson.content.dependencies }
266-
: undefined),
267-
...(editablePkgJson.content.optionalDependencies
268-
? {
269-
optionalDependencies:
270-
editablePkgJson.content.optionalDependencies
271-
}
272-
: undefined),
273-
...(editablePkgJson.content.peerDependencies
274-
? { peerDependencies: editablePkgJson.content.peerDependencies }
275-
: undefined)
258+
...(editablePkgJson.content.dependencies && {
259+
dependencies: editablePkgJson.content.dependencies
260+
}),
261+
...(editablePkgJson.content.optionalDependencies && {
262+
optionalDependencies: editablePkgJson.content.optionalDependencies
263+
}),
264+
...(editablePkgJson.content.peerDependencies && {
265+
peerDependencies: editablePkgJson.content.peerDependencies
266+
})
276267
} as PackageJson
277268

278-
const branch = isCi
279-
? getSocketBranchName(oldPurl, newVersion, workspaceName)
280-
: ''
281-
const shouldOpenPr = isCi
282-
? // eslint-disable-next-line no-await-in-loop
283-
!(await doesPullRequestExistForBranch(owner, repo, branch))
284-
: false
285-
286269
if (updateData) {
287270
editablePkgJson.update(updateData)
288271
}
289-
290272
const modded = updatePackageJsonFromNode(
291273
editablePkgJson,
292274
actualTree,
@@ -296,104 +278,98 @@ export async function pnpmFix(
296278
)
297279
debugLog(`Updated package.json from node: ${modded}`)
298280

299-
let error: unknown
300-
let errored = false
301-
let installed = false
302-
303281
// eslint-disable-next-line no-await-in-loop
304282
if (!(await editablePkgJson.save())) {
305-
debugLog(`Skipping nothing changed in ${editablePkgJson.filename}`)
283+
debugLog(`No changes saved for ${pkgJsonPath}, skipping install`)
306284
continue
307285
}
308286

309-
if (!installedSpecs.has(newSpecKey)) {
310-
installedSpecs.add(newSpecKey)
311-
spinner?.info(`Installing ${newSpec}${workspaceDetails}`)
312-
}
287+
spinner?.info(`Installing ${newSpec} in ${workspaceName}`)
288+
289+
let errored = false
290+
let error: unknown
313291

314292
try {
315293
// eslint-disable-next-line no-await-in-loop
316294
actualTree = await install(pkgEnvDetails, { spinner })
317-
installed = true
318295

319296
if (test) {
320-
if (!testedSpecs.has(newSpecKey)) {
321-
testedSpecs.add(newSpecKey)
322-
spinner?.info(`Testing ${newSpec}${workspaceDetails}`)
323-
}
297+
spinner?.info(`Testing ${newSpec} in ${workspaceName}`)
324298
// eslint-disable-next-line no-await-in-loop
325299
await runScript(testScript, [], { spinner, stdio: 'ignore' })
326300
}
327-
if (!fixedSpecs.has(newSpecKey)) {
328-
fixedSpecs.add(newSpecKey)
329-
spinner?.successAndStop(`Fixed ${name}${workspaceDetails}`)
330-
spinner?.start()
331-
}
332-
} catch (e) {
333-
error = e
334-
errored = true
335-
}
336301

337-
if (
338-
!errored &&
339-
shouldOpenPr &&
340-
// eslint-disable-next-line no-await-in-loop
341-
(await gitCreateAndPushBranchIfNeeded(
342-
branch,
343-
getSocketCommitMessage(oldPurl, newVersion, workspaceName),
344-
cwd
345-
))
346-
) {
347-
// eslint-disable-next-line no-await-in-loop
348-
const prResponse = await openGitHubPullRequest(
349-
owner,
350-
repo,
351-
baseBranch,
352-
branch,
302+
fixedSpecs.add(newSpecKey)
303+
spinner?.successAndStop(`Fixed ${name} in ${workspaceName}`)
304+
spinner?.start()
305+
306+
const branch = getSocketBranchName(
353307
oldPurl,
354308
newVersion,
355-
{
356-
cwd,
357-
workspaceName
358-
}
309+
workspaceName
359310
)
360-
if (prResponse) {
361-
const { data } = prResponse
362-
spinner?.info(`PR #${data.number} opened.`)
363-
if (autoMerge) {
364-
// eslint-disable-next-line no-await-in-loop
365-
await enableAutoMerge(data)
311+
const shouldOpenPr = isCi
312+
? // eslint-disable-next-line no-await-in-loop
313+
!(await doesPullRequestExistForBranch(owner, repo, branch))
314+
: false
315+
316+
if (
317+
isCi &&
318+
shouldOpenPr &&
319+
// eslint-disable-next-line no-await-in-loop
320+
(await gitCreateAndPushBranchIfNeeded(
321+
branch,
322+
getSocketCommitMessage(oldPurl, newVersion, workspaceName),
323+
cwd
324+
))
325+
) {
326+
// eslint-disable-next-line no-await-in-loop
327+
const prResponse = await openGitHubPullRequest(
328+
owner,
329+
repo,
330+
baseBranch,
331+
branch,
332+
oldPurl,
333+
newVersion,
334+
{
335+
cwd,
336+
workspaceName
337+
}
338+
)
339+
if (prResponse) {
340+
const { data } = prResponse
341+
spinner?.info(`PR #${data.number} opened.`)
342+
if (autoMerge) {
343+
// eslint-disable-next-line no-await-in-loop
344+
await enableAutoMerge(data)
345+
}
366346
}
367347
}
348+
} catch (e) {
349+
error = e
350+
errored = true
368351
}
369352

370353
if (errored || isCi) {
371-
if (errored) {
372-
if (!revertedSpecs.has(newSpecKey)) {
373-
revertedSpecs.add(newSpecKey)
374-
spinner?.error(`Reverting ${newSpec}${workspaceDetails}`, error)
375-
}
376-
}
377-
378354
editablePkgJson.update(revertData)
355+
379356
// eslint-disable-next-line no-await-in-loop
380357
await Promise.all([
381358
removeNodeModules(cwd),
382359
...(isCi
383360
? [gitCheckoutBaseBranchIfAvailable(baseBranch, cwd)]
384361
: []),
385-
...(installed && !isCi ? [editablePkgJson.save()] : [])
362+
...(isCi ? [] : [editablePkgJson.save()])
386363
])
364+
387365
// eslint-disable-next-line no-await-in-loop
388366
actualTree = await install(pkgEnvDetails, { spinner })
389367

390368
if (errored) {
391-
if (!failedSpecs.has(newSpecKey)) {
392-
failedSpecs.add(newSpecKey)
393-
spinner?.failAndStop(
394-
`Update failed for ${oldSpec}${workspaceDetails}`
395-
)
396-
}
369+
spinner?.failAndStop(
370+
`Update failed for ${oldSpec} in ${workspaceName}`,
371+
error
372+
)
397373
}
398374
}
399375
}

0 commit comments

Comments
 (0)