From b84ce2ad55e120d5323afaa88338880f68fc6a3e Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Sat, 18 Dec 2021 16:39:55 +0100
Subject: [PATCH] refactor(npm): updateLockedDependency result (#13182)
---
.../update/locked-dependency/index.spec.ts | 22 ++++++------
.../npm/update/locked-dependency/index.ts | 13 +++----
.../locked-dependency/package-lock/index.ts | 35 ++++++++++---------
lib/manager/types.ts | 7 +++-
lib/workers/branch/get-updated.spec.ts | 6 ++--
lib/workers/branch/get-updated.ts | 2 +-
6 files changed, 47 insertions(+), 38 deletions(-)
diff --git a/lib/manager/npm/update/locked-dependency/index.spec.ts b/lib/manager/npm/update/locked-dependency/index.spec.ts
index 022b2f4077..ca7266083b 100644
--- a/lib/manager/npm/update/locked-dependency/index.spec.ts
+++ b/lib/manager/npm/update/locked-dependency/index.spec.ts
@@ -33,7 +33,7 @@ describe('manager/npm/update/locked-dependency/index', () => {
it('validates filename', async () => {
expect(
await updateLockedDependency({ ...config, lockFile: 'yarn.lock' })
- ).toBeNull();
+ ).toMatchObject({});
});
it('validates versions', async () => {
expect(
@@ -41,12 +41,12 @@ describe('manager/npm/update/locked-dependency/index', () => {
...config,
newVersion: '^2.0.0',
})
- ).toBeNull();
+ ).toMatchObject({});
});
it('returns null for unparseable files', async () => {
expect(
await updateLockedDependency({ ...config, lockFileContent: 'not json' })
- ).toBeNull();
+ ).toMatchObject({});
});
it('rejects lockFileVersion 2', async () => {
expect(
@@ -54,10 +54,10 @@ describe('manager/npm/update/locked-dependency/index', () => {
...config,
lockFileContent: lockFileContent.replace(': 1,', ': 2,'),
})
- ).toBeNull();
+ ).toMatchObject({});
});
it('returns null if no locked deps', async () => {
- expect(await updateLockedDependency(config)).toBeNull();
+ expect(await updateLockedDependency(config)).toMatchObject({});
});
it('rejects null if no constraint found', async () => {
expect(
@@ -68,7 +68,7 @@ describe('manager/npm/update/locked-dependency/index', () => {
currentVersion: '10.0.0',
newVersion: '11.0.0',
})
- ).toBeNull();
+ ).toMatchObject({});
});
it('remediates in-range', async () => {
const res = await updateLockedDependency({
@@ -78,7 +78,7 @@ describe('manager/npm/update/locked-dependency/index', () => {
newVersion: '1.2.12',
});
expect(
- JSON.parse(res['package-lock.json']).dependencies.mime.version
+ JSON.parse(res.files['package-lock.json']).dependencies.mime.version
).toBe('1.2.12');
});
it('fails to remediate if parent dep cannot support', async () => {
@@ -98,15 +98,15 @@ describe('manager/npm/update/locked-dependency/index', () => {
currentVersion: '1.0.0',
newVersion: '2.0.0',
});
- expect(res).toBeNull();
+ expect(res).toMatchObject({});
});
it('remediates express', async () => {
config.depName = 'express';
config.currentVersion = '4.0.0';
config.newVersion = '4.1.0';
const res = await updateLockedDependency(config);
- expect(res['package.json']).toContain('"express": "4.1.0"');
- const packageLock = JSON.parse(res['package-lock.json']);
+ expect(res.files['package.json']).toContain('"express": "4.1.0"');
+ const packageLock = JSON.parse(res.files['package-lock.json']);
expect(packageLock.dependencies.express.version).toBe('4.1.0');
});
it('remediates mime', async () => {
@@ -130,7 +130,7 @@ describe('manager/npm/update/locked-dependency/index', () => {
.get('/type-is')
.reply(200, typeIsJson);
const res = await updateLockedDependency(config);
- const packageLock = JSON.parse(res['package-lock.json']);
+ const packageLock = JSON.parse(res.files['package-lock.json']);
expect(packageLock.dependencies.mime.version).toBe('1.4.1');
expect(packageLock.dependencies.express.version).toBe('4.16.0');
expect(httpMock.getTrace()).toMatchSnapshot();
diff --git a/lib/manager/npm/update/locked-dependency/index.ts b/lib/manager/npm/update/locked-dependency/index.ts
index 08198cd155..e32de991b9 100644
--- a/lib/manager/npm/update/locked-dependency/index.ts
+++ b/lib/manager/npm/update/locked-dependency/index.ts
@@ -1,19 +1,20 @@
import { logger } from '../../../../logger';
import * as semver from '../../../../versioning/semver';
-import type { UpdateLockedConfig } from '../../../types';
+import type { UpdateLockedConfig, UpdateLockedResult } from '../../../types';
import * as packageLock from './package-lock';
-export function updateLockedDependency(
+export async function updateLockedDependency(
config: UpdateLockedConfig
-): Promise<Record<string, string>> {
+): Promise<UpdateLockedResult> {
const { currentVersion, newVersion, lockFile } = config;
if (!(semver.isVersion(currentVersion) && semver.isVersion(newVersion))) {
logger.warn({ config }, 'Update versions are not valid');
- return null;
+ return { status: 'update-failed' };
}
if (lockFile.endsWith('package-lock.json')) {
- return packageLock.updateLockedDependency(config);
+ const res = await packageLock.updateLockedDependency(config);
+ return res;
}
logger.debug({ lockFile }, 'Unsupported lock file');
- return null;
+ return { status: 'update-failed' };
}
diff --git a/lib/manager/npm/update/locked-dependency/package-lock/index.ts b/lib/manager/npm/update/locked-dependency/package-lock/index.ts
index 0aa2d7e458..43fcc91f78 100644
--- a/lib/manager/npm/update/locked-dependency/package-lock/index.ts
+++ b/lib/manager/npm/update/locked-dependency/package-lock/index.ts
@@ -2,7 +2,7 @@ import detectIndent from 'detect-indent';
import type { PackageJson } from 'type-fest';
import { logger } from '../../../../../logger';
import { api as semver } from '../../../../../versioning/npm';
-import type { UpdateLockedConfig } from '../../../../types';
+import type { UpdateLockedConfig, UpdateLockedResult } from '../../../../types';
import { updateDependency } from '../../dependency';
import { findFirstParentVersion } from '../common/parent-version';
import { findDepConstraints } from './dep-constraints';
@@ -12,7 +12,7 @@ import type { PackageLockOrEntry } from './types';
export async function updateLockedDependency(
config: UpdateLockedConfig,
isParentUpdate = false
-): Promise<Record<string, string>> {
+): Promise<UpdateLockedResult> {
const {
depName,
currentVersion,
@@ -35,11 +35,11 @@ export async function updateLockedDependency(
packageLockJson = JSON.parse(lockFileContent);
} catch (err) {
logger.warn({ err }, 'Failed to parse files');
- return null;
+ return { status: 'update-failed' };
}
if (packageLockJson.lockfileVersion === 2) {
logger.debug('Only lockfileVersion 1 is supported');
- return null;
+ return { status: 'update-failed' };
}
const lockedDeps = getLockedDependencies(
packageLockJson,
@@ -50,15 +50,15 @@ export async function updateLockedDependency(
logger.debug(
`${depName}@${currentVersion} not found in ${lockFile} - no work to do`
);
- // Don't return null if we're a parent update or else the whole update will fail
+ // Don't return {} if we're a parent update or else the whole update will fail
// istanbul ignore if: too hard to replicate
if (isParentUpdate) {
- const res = {};
- res[packageFile] = packageFileContent;
- res[lockFile] = lockFileContent;
+ const res: UpdateLockedResult = { status: 'update-failed', files: {} };
+ res.files[packageFile] = packageFileContent;
+ res.files[lockFile] = lockFileContent;
return res;
}
- return null;
+ return { status: 'update-failed' };
}
logger.debug(
`Found matching dependencies with length ${lockedDeps.length}`
@@ -76,7 +76,7 @@ export async function updateLockedDependency(
{ depName, currentVersion, newVersion },
'Could not find constraints for the locked dependency - cannot remediate'
);
- return null;
+ return { status: 'update-failed' };
}
const parentUpdates: UpdateLockedConfig[] = [];
for (const {
@@ -122,7 +122,7 @@ export async function updateLockedDependency(
logger.debug(
`Update of ${depName} to ${newVersion} cannot be achieved due to parent ${parentDepName}`
);
- return null;
+ return { status: 'update-failed' };
}
} else if (depType) {
// The constaint comes from the package.json file, so we need to update it
@@ -162,15 +162,16 @@ export async function updateLockedDependency(
true
);
// istanbul ignore if: hard to test due to recursion
- if (!parentUpdateResult) {
+ if (!parentUpdateResult.files) {
logger.debug(
`Update of ${depName} to ${newVersion} impossible due to failed update of parent ${parentUpdate.depName} to ${parentUpdate.newVersion}`
);
- return null;
+ return { status: 'update-failed' };
}
newPackageJsonContent =
- parentUpdateResult[packageFile] || newPackageJsonContent;
- newLockFileContent = parentUpdateResult[lockFile] || newLockFileContent;
+ parentUpdateResult.files[packageFile] || newPackageJsonContent;
+ newLockFileContent =
+ parentUpdateResult.files[lockFile] || newLockFileContent;
}
const files = {};
if (newLockFileContent) {
@@ -179,9 +180,9 @@ export async function updateLockedDependency(
if (newPackageJsonContent) {
files[packageFile] = newPackageJsonContent;
}
- return files;
+ return { status: 'updated', files };
} catch (err) /* istanbul ignore next */ {
logger.error({ err }, 'updateLockedDependency() error');
- return null;
+ return { status: 'update-failed' };
}
}
diff --git a/lib/manager/types.ts b/lib/manager/types.ts
index 036ae7ce56..22a154a290 100644
--- a/lib/manager/types.ts
+++ b/lib/manager/types.ts
@@ -230,6 +230,11 @@ export interface UpdateLockedConfig {
newVersion?: string;
}
+export interface UpdateLockedResult {
+ status: 'updated' | 'update-failed';
+ files?: Record<string, string>;
+}
+
export interface GlobalManagerConfig {
npmrc?: string;
npmrcMerge?: boolean;
@@ -271,7 +276,7 @@ export interface ManagerApi extends ModuleApi {
updateLockedDependency?(
config: UpdateLockedConfig
- ): Result<Record<string, string | null>>;
+ ): Result<UpdateLockedResult>;
}
// TODO: name and properties used by npm manager
diff --git a/lib/workers/branch/get-updated.spec.ts b/lib/workers/branch/get-updated.spec.ts
index 8e9b19a2c4..e8633cc31a 100644
--- a/lib/workers/branch/get-updated.spec.ts
+++ b/lib/workers/branch/get-updated.spec.ts
@@ -160,7 +160,8 @@ describe('workers/branch/get-updated', () => {
isRemediation: true,
} as never);
npm.updateLockedDependency.mockResolvedValueOnce({
- 'package-lock.json': 'new contents',
+ status: 'updated',
+ files: { 'package-lock.json': 'new contents' },
});
const res = await getUpdatedPackageFiles(config);
expect(res).toMatchSnapshot({
@@ -177,7 +178,8 @@ describe('workers/branch/get-updated', () => {
config.reuseExistingBranch = true;
git.getFile.mockResolvedValueOnce('existing content');
npm.updateLockedDependency.mockResolvedValue({
- 'package-lock.json': 'new contents',
+ status: 'updated',
+ files: { 'package-lock.json': 'new contents' },
});
const res = await getUpdatedPackageFiles(config);
expect(res).toMatchSnapshot({
diff --git a/lib/workers/branch/get-updated.ts b/lib/workers/branch/get-updated.ts
index 2bd880163a..8bc474b7d1 100644
--- a/lib/workers/branch/get-updated.ts
+++ b/lib/workers/branch/get-updated.ts
@@ -74,7 +74,7 @@ export async function getUpdatedPackageFiles(
});
}
const updateLockedDependency = get(manager, 'updateLockedDependency');
- const files = await updateLockedDependency({
+ const { files } = await updateLockedDependency({
...upgrade,
packageFileContent,
lockFileContent,
--
GitLab