From c2f0a46d9a3f6d25fbb4430cde01514a50eb2b2e Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Thu, 8 Mar 2018 09:39:32 +0100 Subject: [PATCH] feat: npm shrinkwrap Adds support for npm-shrinkwrap.json files. Closes #67 --- lib/manager/npm/extract.js | 16 ++-- lib/manager/npm/package.js | 2 +- lib/manager/npm/resolve.js | 11 +++ lib/platform/github/index.js | 1 + lib/workers/branch/lock-files.js | 76 ++++++++++++++++- lib/workers/branch/npm.js | 11 +-- lib/workers/package-file/index.js | 12 +++ .../__snapshots__/resolve.spec.js.snap | 1 + test/manager/resolve.spec.js | 1 + .../__snapshots__/lock-files.spec.js.snap | 5 ++ test/workers/branch/lock-files.spec.js | 83 +++++++++++++++++-- test/workers/branch/npm.spec.js | 30 +++++-- test/workers/package-file/index.spec.js | 11 ++- 13 files changed, 232 insertions(+), 28 deletions(-) diff --git a/lib/manager/npm/extract.js b/lib/manager/npm/extract.js index 9ee4907893..7c0c7be719 100644 --- a/lib/manager/npm/extract.js +++ b/lib/manager/npm/extract.js @@ -3,7 +3,12 @@ module.exports = { }; function extractDependencies(packageJson, config) { - const { depType, packageLockParsed, yarnLockParsed } = config; + const { + depType, + packageLockParsed, + npmShrinkwrapParsed, + yarnLockParsed, + } = config; const depNames = packageJson[depType] ? Object.keys(packageJson[depType]) : []; @@ -14,11 +19,12 @@ function extractDependencies(packageJson, config) { let lockedVersion; try { logger.debug('Looking for locked version'); - if (packageLockParsed) { - logger.debug({ currentVersion }, 'Found parsed package-lock.json'); - if (packageLockParsed.dependencies[depName]) { + const lockFile = packageLockParsed || npmShrinkwrapParsed; + if (lockFile) { + logger.debug({ currentVersion }, 'Found parsed npm lock'); + if (lockFile.dependencies[depName]) { logger.debug('Found match'); - lockedVersion = packageLockParsed.dependencies[depName].version; + lockedVersion = lockFile.dependencies[depName].version; } else { logger.debug('No match'); } diff --git a/lib/manager/npm/package.js b/lib/manager/npm/package.js index f4b0dd4f1b..001fa2f0f1 100644 --- a/lib/manager/npm/package.js +++ b/lib/manager/npm/package.js @@ -50,7 +50,7 @@ async function getPackageUpdates(config) { type: 'warning', message: 'Failed to look up dependency', }; - if (config.yarnLock || config.packageLock) { + if (config.yarnLock || config.packageLock || config.npmShrinkwrap) { result.message += '. This will block *all* dependencies from being updated due to presence of lock file.'; } diff --git a/lib/manager/npm/resolve.js b/lib/manager/npm/resolve.js index c4798d4154..786d52cb45 100644 --- a/lib/manager/npm/resolve.js +++ b/lib/manager/npm/resolve.js @@ -94,6 +94,17 @@ async function resolvePackageFile(config, inputFile) { ); packageFile.packageLock = packageLockFileName; } + const npmShrinkwrapFileName = upath.join( + path.dirname(packageFile.packageFile), + 'npm-shrinkwrap.json' + ); + if (fileList.includes(npmShrinkwrapFileName)) { + logger.info( + { packageFile: packageFile.packageFile }, + 'Found npm-shrinkwrap.json' + ); + packageFile.npmShrinkwrap = npmShrinkwrapFileName; + } const shrinkwrapFileName = upath.join( path.dirname(packageFile.packageFile), 'shrinkwrap.yaml' diff --git a/lib/platform/github/index.js b/lib/platform/github/index.js index 502c896819..a76a1355a9 100644 --- a/lib/platform/github/index.js +++ b/lib/platform/github/index.js @@ -1036,6 +1036,7 @@ async function commitFilesToBranch( files: files.filter( file => !file.name.endsWith('package-lock.json') && + !file.name.endsWith('npm-shrinkwrap.json') && !file.name.endsWith('yarn.lock') ), }); diff --git a/lib/workers/branch/lock-files.js b/lib/workers/branch/lock-files.js index f361f53d61..236125de19 100644 --- a/lib/workers/branch/lock-files.js +++ b/lib/workers/branch/lock-files.js @@ -8,6 +8,7 @@ const pnpm = require('./pnpm'); module.exports = { hasPackageLock, + hasNpmShrinkwrap, hasYarnLock, hasShrinkwrapYaml, determineLockFileDirs, @@ -32,6 +33,22 @@ function hasPackageLock(config, packageFile) { throw new Error(`hasPackageLock cannot find ${packageFile}`); } +function hasNpmShrinkwrap(config, packageFile) { + logger.trace( + { packageFiles: config.packageFiles, packageFile }, + 'hasNpmShrinkwrap' + ); + for (const p of config.packageFiles) { + if (p.packageFile === packageFile) { + if (p.npmShrinkwrap) { + return true; + } + return false; + } + } + throw new Error(`hasPackageLock cannot find ${packageFile}`); +} + function hasYarnLock(config, packageFile) { logger.trace( { packageFiles: config.packageFiles, packageFile }, @@ -66,6 +83,7 @@ function hasShrinkwrapYaml(config, packageFile) { function determineLockFileDirs(config) { const packageLockFileDirs = []; + const npmShrinkwrapDirs = []; const yarnLockFileDirs = []; const shrinkwrapYamlDirs = []; const lernaDirs = []; @@ -81,6 +99,9 @@ function determineLockFileDirs(config) { if (packageFile.packageLock) { packageLockFileDirs.push(dirname); } + if (packageFile.npmShrinkwrap) { + npmShrinkwrapDirs.push(dirname); + } if (packageFile.shrinkwrapYaml) { shrinkwrapYamlDirs.push(dirname); } @@ -102,6 +123,12 @@ function determineLockFileDirs(config) { ) { packageLockFileDirs.push(path.dirname(packageFile.name)); } + if ( + module.exports.hasNpmShrinkwrap(config, packageFile.name) && + !config.lernaLockFile + ) { + npmShrinkwrapDirs.push(path.dirname(packageFile.name)); + } if (module.exports.hasShrinkwrapYaml(config, packageFile.name)) { shrinkwrapYamlDirs.push(path.dirname(packageFile.name)); } @@ -135,6 +162,7 @@ function determineLockFileDirs(config) { return { yarnLockFileDirs, packageLockFileDirs, + npmShrinkwrapDirs, shrinkwrapYamlDirs, lernaDirs, }; @@ -253,6 +281,20 @@ async function writeExistingFiles(config) { logger.debug(`Removing ${basedir}/package-lock.json`); await fs.remove(upath.join(basedir, 'package-lock.json')); } + // istanbul ignore if + if (packageFile.npmShrinkwrap) { + logger.debug(`Writing npm-shrinkwrap.json to ${basedir}`); + const existingNpmShrinkwrap = + (await platform.branchExists(config.branchName)) && + (await platform.getFile(packageFile.npmShrinkwrap, config.branchName)); + const npmShrinkwrap = + existingNpmShrinkwrap || + (await platform.getFile(packageFile.npmShrinkwrap)); + await fs.outputFile( + upath.join(basedir, 'npm-shrinkwrap.json'), + npmShrinkwrap + ); + } if (packageFile.yarnLock && config.type !== 'lockFileMaintenance') { logger.debug(`Writing yarn.lock to ${basedir}`); const yarnLock = await platform.getFile(packageFile.yarnLock); @@ -349,7 +391,8 @@ async function getUpdatedLockFiles(config) { const lockFileName = upath.join(lockFileDir, 'package-lock.json'); const res = await npm.generateLockFile( upath.join(config.tmpDir.path, lockFileDir), - env + env, + 'package-lock.json' ); if (res.error) { lockFileErrors.push({ @@ -373,6 +416,37 @@ async function getUpdatedLockFiles(config) { } } + // istanbul ignore next + for (const lockFileDir of dirs.npmShrinkwrapDirs) { + logger.debug(`Generating npm-shrinkwrap.json for ${lockFileDir}`); + const lockFileName = upath.join(lockFileDir, 'npm-shrinkwrap.json'); + const res = await npm.generateLockFile( + upath.join(config.tmpDir.path, lockFileDir), + env, + 'npm-shrinkwrap.json' + ); + if (res.error) { + lockFileErrors.push({ + lockFile: lockFileName, + stderr: res.stderr, + }); + } else { + const existingContent = await platform.getFile( + lockFileName, + config.parentBranch + ); + if (res.lockFile !== existingContent) { + logger.debug('npm-shrinkwrap.json needs updating'); + updatedLockFiles.push({ + name: lockFileName, + contents: res.lockFile, + }); + } else { + logger.debug("npm-shrinkwrap.json hasn't changed"); + } + } + } + for (const lockFileDir of dirs.yarnLockFileDirs) { logger.debug(`Generating yarn.lock for ${lockFileDir}`); const lockFileName = upath.join(lockFileDir, 'yarn.lock'); diff --git a/lib/workers/branch/npm.js b/lib/workers/branch/npm.js index 4ce710ca4a..7f2c44848a 100644 --- a/lib/workers/branch/npm.js +++ b/lib/workers/branch/npm.js @@ -7,8 +7,8 @@ module.exports = { generateLockFile, }; -async function generateLockFile(tmpDir, env) { - logger.debug(`Spawning npm install to create ${tmpDir}/package-lock.json`); +async function generateLockFile(tmpDir, env, filename) { + logger.debug(`Spawning npm install to create ${tmpDir}/${filename}`); let lockFile = null; let stdout; let stderr; @@ -64,12 +64,9 @@ async function generateLockFile(tmpDir, env) { logger.debug(`npm stderr:\n${stderr}`); const duration = process.hrtime(startTime); const seconds = Math.round(duration[0] + duration[1] / 1e9); - lockFile = await fs.readFile( - upath.join(tmpDir, 'package-lock.json'), - 'utf8' - ); + lockFile = await fs.readFile(upath.join(tmpDir, filename), 'utf8'); logger.info( - { seconds, type: 'package-lock.json', stdout, stderr }, + { seconds, type: filename, stdout, stderr }, 'Generated lockfile' ); } catch (err) /* istanbul ignore next */ { diff --git a/lib/workers/package-file/index.js b/lib/workers/package-file/index.js index 7399a6bb6b..4d14319b93 100644 --- a/lib/workers/package-file/index.js +++ b/lib/workers/package-file/index.js @@ -93,6 +93,18 @@ async function renovatePackageJson(input) { 'Could not parse package-lock.json' ); } + } else if (config.npmShrinkwrap) { + try { + config.npmShrinkwrapParsed = JSON.parse( + await platform.getFile(config.npmShrinkwrap) + ); + logger.trace({ npmShrinkwrapParsed: config.npmShrinkwrapParsed }); + } catch (err) { + logger.warn( + { npmShrinkwrap: config.npmShrinkwrap }, + 'Could not parse npm-shrinkwrap.json' + ); + } } const depTypes = [ diff --git a/test/manager/__snapshots__/resolve.spec.js.snap b/test/manager/__snapshots__/resolve.spec.js.snap index 7c59c549d5..81567610c4 100644 --- a/test/manager/__snapshots__/resolve.spec.js.snap +++ b/test/manager/__snapshots__/resolve.spec.js.snap @@ -32,6 +32,7 @@ Array [ "currentPackageJsonVersion": "0.0.1", "enabled": true, "manager": "npm", + "npmShrinkwrap": "npm-shrinkwrap.json", "npmrc": "npmrc", "packageFile": "package.json", "packageLock": "package-lock.json", diff --git a/test/manager/resolve.spec.js b/test/manager/resolve.spec.js index 78c9d002b4..a945e81fb5 100644 --- a/test/manager/resolve.spec.js +++ b/test/manager/resolve.spec.js @@ -83,6 +83,7 @@ describe('manager/resolve', () => { 'package.json', 'yarn.lock', 'package-lock.json', + 'npm-shrinkwrap.json', 'shrinkwrap.yaml', ]); platform.getFile.mockReturnValueOnce( diff --git a/test/workers/branch/__snapshots__/lock-files.spec.js.snap b/test/workers/branch/__snapshots__/lock-files.spec.js.snap index 4319d3b79e..4e213fe4c0 100644 --- a/test/workers/branch/__snapshots__/lock-files.spec.js.snap +++ b/test/workers/branch/__snapshots__/lock-files.spec.js.snap @@ -17,6 +17,9 @@ Object { exports[`workers/branch/lock-files determineLockFileDirs returns directories from updated package files 1`] = ` Object { "lernaDirs": Array [], + "npmShrinkwrapDirs": Array [ + "leftend", + ], "packageLockFileDirs": Array [ "backend", ], @@ -34,6 +37,7 @@ Object { "lernaDirs": Array [ ".", ], + "npmShrinkwrapDirs": Array [], "packageLockFileDirs": Array [], "shrinkwrapYamlDirs": Array [], "yarnLockFileDirs": Array [], @@ -43,6 +47,7 @@ Object { exports[`workers/branch/lock-files determineLockFileDirs returns root directory if using yarn workspaces 1`] = ` Object { "lernaDirs": Array [], + "npmShrinkwrapDirs": Array [], "packageLockFileDirs": Array [], "shrinkwrapYamlDirs": Array [], "yarnLockFileDirs": Array [ diff --git a/test/workers/branch/lock-files.spec.js b/test/workers/branch/lock-files.spec.js index 73b3ed149e..3dfc6c5571 100644 --- a/test/workers/branch/lock-files.spec.js +++ b/test/workers/branch/lock-files.spec.js @@ -10,6 +10,7 @@ const lerna = require('../../../lib/workers/branch/lerna'); const { hasPackageLock, + hasNpmShrinkwrap, hasYarnLock, hasShrinkwrapYaml, determineLockFileDirs, @@ -66,6 +67,53 @@ describe('workers/branch/lock-files', () => { expect(e).toBeDefined(); }); }); + describe('hasNpmShrinkWrap', () => { + let config; + beforeEach(() => { + config = { + ...defaultConfig, + }; + }); + it('returns true if found and true', () => { + config.packageFiles = [ + { + packageFile: 'package.json', + npmShrinkwrap: 'some package lock', + }, + ]; + expect(hasNpmShrinkwrap(config, 'package.json')).toBe(true); + }); + it('returns false if found and false', () => { + config.packageFiles = [ + { + packageFile: 'package.json', + npmShrinkwrap: 'some package lock', + }, + { + packageFile: 'backend/package.json', + }, + ]; + expect(hasNpmShrinkwrap(config, 'backend/package.json')).toBe(false); + }); + it('throws error if not found', () => { + config.packageFiles = [ + { + packageFile: 'package.json', + npmShrinkwrap: 'some package lock', + }, + { + packageFile: 'backend/package.json', + }, + ]; + let e; + try { + hasNpmShrinkwrap(config, 'frontend/package.json'); + } catch (err) { + e = err; + } + expect(e).toBeDefined(); + }); + }); describe('hasYarnLock', () => { let config; beforeEach(() => { @@ -178,6 +226,10 @@ describe('workers/branch/lock-files', () => { packageFile: 'frontend/package.json', shrinkwrapYaml: 'some package lock', }, + { + packageFile: 'leftend/package.json', + npmShrinkwrap: 'some package lock', + }, ], }; }); @@ -201,6 +253,10 @@ describe('workers/branch/lock-files', () => { name: 'frontend/package.json', contents: 'some contents', }, + { + name: 'leftend/package.json', + contents: 'some contents', + }, ]; const res = determineLockFileDirs(config); expect(res).toMatchSnapshot(); @@ -288,10 +344,15 @@ describe('workers/branch/lock-files', () => { content: { name: 'package-2', engines: { yarn: '^0.27.5' } }, yarnrc: 'some yarnrc', }, + { + packageFile: 'leftend/package.json', + hasNpmShrinkwrap: true, + content: { name: 'package-3' }, + }, ]; await writeExistingFiles(config); - expect(fs.outputFile.mock.calls).toHaveLength(6); - expect(fs.remove.mock.calls).toHaveLength(6); + expect(fs.outputFile.mock.calls).toHaveLength(7); + expect(fs.remove.mock.calls).toHaveLength(9); }); it('writes package.json of local lib', async () => { const renoPath = upath.join(__dirname, '../../../'); @@ -441,6 +502,7 @@ describe('workers/branch/lock-files', () => { it('returns no error and empty lockfiles if none updated', async () => { lockFiles.determineLockFileDirs.mockReturnValueOnce({ packageLockFileDirs: [], + npmShrinkwrapDirs: [], yarnLockFileDirs: [], shrinkwrapYamlDirs: [], lernaDirs: [], @@ -453,6 +515,7 @@ describe('workers/branch/lock-files', () => { it('tries multiple lock files', async () => { lockFiles.determineLockFileDirs.mockReturnValueOnce({ packageLockFileDirs: ['a', 'b'], + npmShrinkwrapDirs: ['f'], yarnLockFileDirs: ['c', 'd'], shrinkwrapYamlDirs: ['e'], lernaDirs: [], @@ -461,13 +524,14 @@ describe('workers/branch/lock-files', () => { expect(res).toMatchSnapshot(); expect(res.lockFileErrors).toHaveLength(0); expect(res.updatedLockFiles).toHaveLength(0); - expect(npm.generateLockFile.mock.calls).toHaveLength(2); + expect(npm.generateLockFile.mock.calls).toHaveLength(3); expect(yarn.generateLockFile.mock.calls).toHaveLength(2); - expect(platform.getFile.mock.calls).toHaveLength(6); + expect(platform.getFile.mock.calls).toHaveLength(7); }); it('tries lerna npm', async () => { lockFiles.determineLockFileDirs.mockReturnValueOnce({ packageLockFileDirs: ['a', 'b'], + npmShrinkwrapDirs: [], yarnLockFileDirs: [], shrinkwrapYamlDirs: [], lernaDirs: ['.'], @@ -480,6 +544,7 @@ describe('workers/branch/lock-files', () => { it('tries lerna yarn', async () => { lockFiles.determineLockFileDirs.mockReturnValueOnce({ packageLockFileDirs: [], + npmShrinkwrapDirs: [], yarnLockFileDirs: ['c', 'd'], shrinkwrapYamlDirs: [], lernaDirs: ['.'], @@ -492,6 +557,7 @@ describe('workers/branch/lock-files', () => { it('sets error if receiving null', async () => { lockFiles.determineLockFileDirs.mockReturnValueOnce({ packageLockFileDirs: ['a', 'b'], + npmShrinkwrapDirs: ['f'], yarnLockFileDirs: ['c', 'd'], shrinkwrapYamlDirs: ['e'], lernaDirs: [], @@ -502,13 +568,14 @@ describe('workers/branch/lock-files', () => { const res = await getUpdatedLockFiles(config); expect(res.lockFileErrors).toHaveLength(3); expect(res.updatedLockFiles).toHaveLength(0); - expect(npm.generateLockFile.mock.calls).toHaveLength(2); + expect(npm.generateLockFile.mock.calls).toHaveLength(3); expect(yarn.generateLockFile.mock.calls).toHaveLength(2); - expect(platform.getFile.mock.calls).toHaveLength(3); + expect(platform.getFile.mock.calls).toHaveLength(4); }); it('adds multiple lock files', async () => { lockFiles.determineLockFileDirs.mockReturnValueOnce({ packageLockFileDirs: ['a', 'b'], + npmShrinkwrapDirs: ['f'], yarnLockFileDirs: ['c', 'd'], shrinkwrapYamlDirs: ['e'], lernaDirs: [], @@ -519,9 +586,9 @@ describe('workers/branch/lock-files', () => { const res = await getUpdatedLockFiles(config); expect(res.lockFileErrors).toHaveLength(0); expect(res.updatedLockFiles).toHaveLength(3); - expect(npm.generateLockFile.mock.calls).toHaveLength(2); + expect(npm.generateLockFile.mock.calls).toHaveLength(3); expect(yarn.generateLockFile.mock.calls).toHaveLength(2); - expect(platform.getFile.mock.calls).toHaveLength(6); + expect(platform.getFile.mock.calls).toHaveLength(7); }); }); }); diff --git a/test/workers/branch/npm.spec.js b/test/workers/branch/npm.spec.js index bc339bc0b7..496ab86dfd 100644 --- a/test/workers/branch/npm.spec.js +++ b/test/workers/branch/npm.spec.js @@ -19,7 +19,11 @@ describe('generateLockFile', () => { stderror: '', }); fs.readFile = jest.fn(() => 'package-lock-contents'); - const res = await npmHelper.generateLockFile('some-dir'); + const res = await npmHelper.generateLockFile( + 'some-dir', + {}, + 'package-lock.json' + ); expect(fs.readFile.mock.calls.length).toEqual(1); expect(res.error).not.toBeDefined(); expect(res.lockFile).toEqual('package-lock-contents'); @@ -33,7 +37,11 @@ describe('generateLockFile', () => { fs.readFile = jest.fn(() => { throw new Error('not found'); }); - const res = await npmHelper.generateLockFile('some-dir'); + const res = await npmHelper.generateLockFile( + 'some-dir', + {}, + 'package-lock.json' + ); expect(fs.readFile.mock.calls.length).toEqual(1); expect(res.error).toBe(true); expect(res.lockFile).not.toBeDefined(); @@ -51,7 +59,11 @@ describe('generateLockFile', () => { stderror: '', }); fs.readFile = jest.fn(() => 'package-lock-contents'); - const res = await npmHelper.generateLockFile('some-dir'); + const res = await npmHelper.generateLockFile( + 'some-dir', + {}, + 'package-lock.json' + ); expect(fs.readFile.mock.calls.length).toEqual(1); expect(res.lockFile).toEqual('package-lock-contents'); }); @@ -69,7 +81,11 @@ describe('generateLockFile', () => { stderror: '', }); fs.readFile = jest.fn(() => 'package-lock-contents'); - const res = await npmHelper.generateLockFile('some-dir'); + const res = await npmHelper.generateLockFile( + 'some-dir', + {}, + 'package-lock.json' + ); expect(fs.readFile.mock.calls.length).toEqual(1); expect(res.lockFile).toEqual('package-lock-contents'); }); @@ -89,7 +105,11 @@ describe('generateLockFile', () => { stderror: '', }); fs.readFile = jest.fn(() => 'package-lock-contents'); - const res = await npmHelper.generateLockFile('some-dir'); + const res = await npmHelper.generateLockFile( + 'some-dir', + {}, + 'package-lock.json' + ); expect(fs.readFile.mock.calls.length).toEqual(1); expect(res.lockFile).toEqual('package-lock-contents'); }); diff --git a/test/workers/package-file/index.spec.js b/test/workers/package-file/index.spec.js index 78594e4395..0c44c4957c 100644 --- a/test/workers/package-file/index.spec.js +++ b/test/workers/package-file/index.spec.js @@ -82,7 +82,16 @@ describe('packageFileWorker', () => { await packageFileWorker.renovatePackageFile(config); }); it('parses package-lock.json', async () => { - config.packageLock = 'package-lock.lock'; + config.packageLock = 'package-lock.json'; + platform.getFile.mockReturnValueOnce('{}'); + await packageFileWorker.renovatePackageFile(config); + }); + it('skips unparseable npm-shrinkwrap.json', async () => { + config.npmShrinkwrap = 'npm-shrinkwrap.json'; + await packageFileWorker.renovatePackageFile(config); + }); + it('parses npm-shrinkwrap.json', async () => { + config.npmShrinkwrap = 'npm-shrinkwrap.json'; platform.getFile.mockReturnValueOnce('{}'); await packageFileWorker.renovatePackageFile(config); }); -- GitLab