From d2ad057a33a94fd9f382fd9584c640fec77123d7 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Wed, 2 Aug 2017 14:05:45 +0200 Subject: [PATCH] feat: Improve depTypes configuration (#577) depTypes configuration is now refactored from being an array of strings/objects to having each type of dep (`dependencies`, `devDependencies`, `optionalDependencies`, `peerDependencies`) be a first class object in the namespace. The "old" way of configuring is still supported but is transformed using a migrateConfig script. Later, PRs will be raised to impacted repositories to suggest the new config. --- docs/configuration.md | 42 +++++++++++++----- lib/config/definitions.js | 44 +++++++++++++++---- lib/config/index.js | 31 +++---------- lib/config/migration.js | 16 +++++++ lib/workers/package-file/index.js | 40 +++++++++-------- lib/workers/repository/apis.js | 2 +- lib/workers/repository/onboarding.js | 9 ++-- test/config/__snapshots__/index.spec.js.snap | 39 +--------------- .../__snapshots__/migration.spec.js.snap | 3 ++ test/config/index.spec.js | 22 ++-------- test/config/migration.spec.js | 9 ++++ test/workers/package-file/index.spec.js | 6 +-- .../__snapshots__/onboarding.spec.js.snap | 6 +-- test/workers/repository/onboarding.spec.js | 8 +--- 14 files changed, 137 insertions(+), 140 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index baaefc4fba..ee0dc18522 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -84,7 +84,6 @@ $ node renovate --help --github-app-key <string> GitHub App Private Key (.pem file contents) --package-files <list> Package file paths --ignore-node-modules [boolean] Skip any package.json files found within node_modules folders - --dep-types <list> Dependency types --ignore-deps <list> Dependencies to ignore --pin-versions [boolean] Convert ranged versions in package.json to pinned versions --separate-major-releases [boolean] If set to false, it will upgrade dependencies to latest release only, and not separate major/minor branches @@ -289,17 +288,36 @@ Obviously, you can't set repository or package file location with this method. <td>`--ignore-node-modules`<td> </tr> <tr> - <td>`depTypes`</td> - <td>Dependency types</td> - <td>list</td> - <td><pre>[ - {"depType": "dependencies", "semanticPrefix": "fix(deps): "}, - "devDependencies", - "optionalDependencies", - {"depType": "peerDependencies", "enabled": false} -]</pre></td> - <td>`RENOVATE_DEP_TYPES`</td> - <td>`--dep-types`<td> + <td>`dependencies`</td> + <td>Configuration specifically for `package.json`>`dependencies`</td> + <td>json</td> + <td><pre>{}</pre></td> + <td>`RENOVATE_DEPENDENCIES`</td> + <td><td> +</tr> +<tr> + <td>`devDependencies`</td> + <td>Configuration specifically for `package.json`>`devDependencies`</td> + <td>json</td> + <td><pre>{}</pre></td> + <td>`RENOVATE_DEV_DEPENDENCIES`</td> + <td><td> +</tr> +<tr> + <td>`optionalDependencies`</td> + <td>Configuration specifically for `package.json`>`optionalDependencies`</td> + <td>json</td> + <td><pre>{}</pre></td> + <td>`RENOVATE_OPTIONAL_DEPENDENCIES`</td> + <td><td> +</tr> +<tr> + <td>`peerDependencies`</td> + <td>Configuration specifically for `package.json`>`peerDependencies`</td> + <td>json</td> + <td><pre>{"enabled": false}</pre></td> + <td>`RENOVATE_PEER_DEPENDENCIES`</td> + <td><td> </tr> <tr> <td>`ignoreDeps`</td> diff --git a/lib/config/definitions.js b/lib/config/definitions.js index b62bb7f83a..4db8770bad 100644 --- a/lib/config/definitions.js +++ b/lib/config/definitions.js @@ -142,17 +142,43 @@ const options = [ stage: 'repository', }, { - name: 'depTypes', - description: 'Dependency types', + name: 'dependencies', + description: 'Configuration specifically for `package.json`>`dependencies`', stage: 'packageFile', - type: 'list', - default: [ - { depType: 'dependencies', semanticPrefix: 'fix(deps): ' }, - 'devDependencies', - 'optionalDependencies', - { depType: 'peerDependencies', enabled: false }, - ], + type: 'json', + default: {}, + mergeable: true, + cli: false, + }, + { + name: 'devDependencies', + description: + 'Configuration specifically for `package.json`>`devDependencies`', + stage: 'packageFile', + type: 'json', + default: {}, mergeable: true, + cli: false, + }, + { + name: 'optionalDependencies', + description: + 'Configuration specifically for `package.json`>`optionalDependencies`', + stage: 'packageFile', + type: 'json', + default: {}, + mergeable: true, + cli: false, + }, + { + name: 'peerDependencies', + description: + 'Configuration specifically for `package.json`>`peerDependencies`', + stage: 'packageFile', + type: 'json', + default: { enabled: false }, + mergeable: true, + cli: false, }, // depType { diff --git a/lib/config/index.js b/lib/config/index.js index b5439da381..81bee9201e 100644 --- a/lib/config/index.js +++ b/lib/config/index.js @@ -118,32 +118,13 @@ function mergeChildConfig(parentConfig, childConfig) { for (const option of definitions.getOptions()) { if (option.mergeable && childConfig[option.name]) { logger.debug(`mergeable option: ${option.name}`); - if (option.name === 'depTypes') { - const depTypes = []; - for (let parent of parentConfig.depTypes) { - if (typeof parent === 'string') { - parent = { depType: parent }; - } - for (let child of childConfig.depTypes) { - if (typeof child === 'string') { - child = { depType: child }; - } - if (parent.depType === child.depType) { - Object.assign(parent, child); - } - } - depTypes.push(parent); - } - config.depTypes = depTypes; - } else { - config[option.name] = Object.assign( - {}, - parentConfig[option.name], - childConfig[option.name] - ); - } - logger.debug({ option: config[option.name] }, `config.${option.name}`); + config[option.name] = Object.assign( + {}, + parentConfig[option.name], + childConfig[option.name] + ); } + logger.trace({ value: config[option.name] }, `config.${option.name}`); } return config; } diff --git a/lib/config/migration.js b/lib/config/migration.js index 13d98b66e3..9c017cf294 100644 --- a/lib/config/migration.js +++ b/lib/config/migration.js @@ -20,10 +20,26 @@ function migrateConfig(config) { let isMigrated = false; const migratedConfig = Object.assign({}, config); for (const key of Object.keys(config)) { + const val = config[key]; if (removedOptions.includes(key)) { isMigrated = true; delete migratedConfig[key]; + } else if (key === 'depTypes' && Array.isArray(val)) { + val.forEach(depType => { + if (isObject(depType)) { + const depTypeName = depType.depType; + if (depTypeName) { + migratedConfig[depTypeName] = Object.assign({}, depType); + delete migratedConfig[depTypeName].depType; + } + } + }); + delete migratedConfig.depTypes; } } return { isMigrated, migratedConfig }; } + +function isObject(obj) { + return Object.prototype.toString.call(obj) === '[object Object]'; +} diff --git a/lib/workers/package-file/index.js b/lib/workers/package-file/index.js index 7c31db63e4..3ac26cf119 100644 --- a/lib/workers/package-file/index.js +++ b/lib/workers/package-file/index.js @@ -8,7 +8,6 @@ let logger = require('../../logger'); module.exports = { renovatePackageFile, - getDepTypeConfig, }; async function renovatePackageFile(packageFileConfig) { @@ -72,7 +71,7 @@ async function renovatePackageFile(packageFileConfig) { */ } // package.json>renovate config takes precedence over existing config - config = configParser.mergeChildConfig(config, packageContent.renovate); + config = configParser.mergeChildConfig(config, migratedConfig); } else { logger.debug('Package file has no renovate configuration'); } @@ -82,9 +81,26 @@ async function renovatePackageFile(packageFileConfig) { return upgrades; } - const depTypeConfigs = config.depTypes.map(depType => - module.exports.getDepTypeConfig(config, depType) - ); + const depTypes = [ + 'dependencies', + 'devDependencies', + 'optionalDependencies', + 'peerDependencies', + ]; + const depTypeConfigs = depTypes.map(depType => { + const depTypeConfig = configParser.mergeChildConfig( + packageFileConfig, + packageFileConfig[depType] + ); + depTypeConfig.depType = depType; + depTypeConfig.logger = logger.child({ + repository: depTypeConfig.repository, + packageFile: depTypeConfig.packageFile, + depType: depTypeConfig.depType, + }); + logger.debug({ config: depTypeConfig }, 'depTypeConfig'); + return configParser.filterConfig(depTypeConfig, 'depType'); + }); logger.trace({ config: depTypeConfigs }, `depTypeConfigs`); for (const depTypeConfig of depTypeConfigs) { upgrades = upgrades.concat( @@ -128,17 +144,3 @@ async function renovatePackageFile(packageFileConfig) { logger.info('Finished processing package file'); return upgrades; } - -function getDepTypeConfig(packageFileConfig, depType) { - let depTypeConfig = typeof depType === 'string' ? { depType } : depType; - depTypeConfig = configParser.mergeChildConfig( - packageFileConfig, - depTypeConfig - ); - depTypeConfig.logger = logger.child({ - repository: depTypeConfig.repository, - packageFile: depTypeConfig.packageFile, - depType: depTypeConfig.depType, - }); - return configParser.filterConfig(depTypeConfig, 'depType'); -} diff --git a/lib/workers/repository/apis.js b/lib/workers/repository/apis.js index b893f0e8f1..a95bf979bb 100644 --- a/lib/workers/repository/apis.js +++ b/lib/workers/repository/apis.js @@ -146,7 +146,7 @@ async function mergeRenovateJson(config, branchName) { ); }); */ } - returnConfig = configParser.mergeChildConfig(returnConfig, renovateJson); + returnConfig = configParser.mergeChildConfig(returnConfig, migratedConfig); returnConfig.renovateJsonPresent = true; } catch (err) { // Add to config.errors diff --git a/lib/workers/repository/onboarding.js b/lib/workers/repository/onboarding.js index 258a6908bf..87190e2953 100644 --- a/lib/workers/repository/onboarding.js +++ b/lib/workers/repository/onboarding.js @@ -49,12 +49,9 @@ async function createBranch(config) { config.logger.debug('Repo is private - pinning dependencies versions'); } else { config.logger.debug('Repo is not private - unpinning versions'); - onboardingConfig.depTypes = [ - { - depType: 'dependencies', - pinVersions: false, - }, - ]; + onboardingConfig.dependencies = { + pinVersions: false, + }; } if (config.foundNodeModules) { onboardingConfig.ignoreNodeModules = true; diff --git a/test/config/__snapshots__/index.spec.js.snap b/test/config/__snapshots__/index.spec.js.snap index 9b7d4174d2..b0d69c50a6 100644 --- a/test/config/__snapshots__/index.spec.js.snap +++ b/test/config/__snapshots__/index.spec.js.snap @@ -47,41 +47,4 @@ This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](ht } `; -exports[`config/index mergeChildConfig(parentConfig, childConfig) merges depTypes with meaningful child 1`] = ` -Array [ - Object { - "depType": "dependencies", - "semanticPrefix": "fix(deps): ", - }, - Object { - "depType": "devDependencies", - "foo": 1, - }, - Object { - "depType": "optionalDependencies", - }, - Object { - "depType": "peerDependencies", - "enabled": false, - }, -] -`; - -exports[`config/index mergeChildConfig(parentConfig, childConfig) merges depTypes with no child config 1`] = ` -Array [ - Object { - "depType": "dependencies", - "semanticPrefix": "fix(deps): ", - }, - Object { - "depType": "devDependencies", - }, - Object { - "depType": "optionalDependencies", - }, - Object { - "depType": "peerDependencies", - "enabled": false, - }, -] -`; +exports[`config/index mergeChildConfig(parentConfig, childConfig) merges depTypes 1`] = `undefined`; diff --git a/test/config/__snapshots__/migration.spec.js.snap b/test/config/__snapshots__/migration.spec.js.snap index 4367759db0..6ea6cee4fc 100644 --- a/test/config/__snapshots__/migration.spec.js.snap +++ b/test/config/__snapshots__/migration.spec.js.snap @@ -3,5 +3,8 @@ exports[`config/migration migrateConfig(config) it migrates config 1`] = ` Object { "enabled": true, + "optionalDependencies": Object { + "respectLatest": false, + }, } `; diff --git a/test/config/index.spec.js b/test/config/index.spec.js index 206789ed94..5a23ceb0ea 100644 --- a/test/config/index.spec.js +++ b/test/config/index.spec.js @@ -162,26 +162,12 @@ describe('config/index', () => { expect(config.lockFileMaintenance.schedule).toEqual('on monday'); expect(config.lockFileMaintenance).toMatchSnapshot(); }); - it('merges depTypes with no child config', () => { + it('merges depTypes', () => { const parentConfig = Object.assign({}, defaultConfig); const childConfig = { - depTypes: ['dependencies'], - }; - const configParser = require('../../lib/config/index.js'); - const config = configParser.mergeChildConfig(parentConfig, childConfig); - expect(config.depTypes).toMatchSnapshot(); - }); - it('merges depTypes with meaningful child', () => { - const parentConfig = Object.assign({}, defaultConfig); - const childConfig = { - depTypes: [ - 'dependencies', - { - depType: 'devDependencies', - foo: 1, - }, - 'peerDependencies', - ], + dependencies: {}, + devDependencies: { foo: 1 }, + peerDependencies: {}, }; const configParser = require('../../lib/config/index.js'); const config = configParser.mergeChildConfig(parentConfig, childConfig); diff --git a/test/config/migration.spec.js b/test/config/migration.spec.js index 120aab658c..eebde0d06b 100644 --- a/test/config/migration.spec.js +++ b/test/config/migration.spec.js @@ -6,11 +6,20 @@ describe('config/migration', () => { const config = { enabled: true, maintainYarnLock: true, + depTypes: [ + 'dependencies', + { + depType: 'optionalDependencies', + respectLatest: false, + }, + ], }; const { isMigrated, migratedConfig } = configMigration.migrateConfig( config ); expect(isMigrated).toBe(true); + expect(migratedConfig.depTypes).not.toBeDefined(); + expect(migratedConfig.optionalDependencies.respectLatest).toBe(false); expect(migratedConfig).toMatchSnapshot(); }); it('it does not migrate config', () => { diff --git a/test/workers/package-file/index.spec.js b/test/workers/package-file/index.spec.js index 4223c38587..859b0b8452 100644 --- a/test/workers/package-file/index.spec.js +++ b/test/workers/package-file/index.spec.js @@ -18,7 +18,6 @@ describe('packageFileWorker', () => { getFileContent: jest.fn(), getFileJson: jest.fn(), }, - depTypes: ['dependencies', 'devDependencies'], logger, }); }); @@ -66,6 +65,8 @@ describe('packageFileWorker', () => { config.api.getFileJson.mockReturnValueOnce({}); depTypeWorker.renovateDepType.mockReturnValueOnce([{}]); depTypeWorker.renovateDepType.mockReturnValueOnce([{}, {}]); + depTypeWorker.renovateDepType.mockReturnValueOnce([]); + depTypeWorker.renovateDepType.mockReturnValueOnce([]); const res = await packageFileWorker.renovatePackageFile(config); expect(res).toHaveLength(3); }); @@ -73,8 +74,7 @@ describe('packageFileWorker', () => { config.api.getFileJson.mockReturnValueOnce({}); config.api.getFileContent.mockReturnValueOnce('some-content-1'); config.api.getFileContent.mockReturnValueOnce('some-content-2'); - depTypeWorker.renovateDepType.mockReturnValueOnce([]); - depTypeWorker.renovateDepType.mockReturnValueOnce([]); + depTypeWorker.renovateDepType.mockReturnValue([]); const res = await packageFileWorker.renovatePackageFile(config); expect(res).toHaveLength(1); }); diff --git a/test/workers/repository/__snapshots__/onboarding.spec.js.snap b/test/workers/repository/__snapshots__/onboarding.spec.js.snap index d259073455..56e97505d5 100644 --- a/test/workers/repository/__snapshots__/onboarding.spec.js.snap +++ b/test/workers/repository/__snapshots__/onboarding.spec.js.snap @@ -218,7 +218,7 @@ Array [ Object { "contents": "{ \\"pinVersions\\": true, - \\"depTypes\\": [{\\"depType\\": \\"dependencies\\", \\"pinVersions\\": false}], + \\"dependencies\\": {\\"pinVersions\\": false}, \\"ignoreNodeModules\\": true } ", @@ -236,7 +236,7 @@ Array [ Object { "contents": "{ \\"pinVersions\\": true, - \\"depTypes\\": [{\\"depType\\": \\"dependencies\\", \\"pinVersions\\": false}], + \\"dependencies\\": {\\"pinVersions\\": false}, \\"ignoreNodeModules\\": true } ", @@ -255,7 +255,7 @@ Array [ "contents": "{ \\"pinVersions\\": true, \\"semanticCommits\\": true, - \\"depTypes\\": [{\\"depType\\": \\"dependencies\\", \\"pinVersions\\": false}], + \\"dependencies\\": {\\"pinVersions\\": false}, \\"ignoreNodeModules\\": true } ", diff --git a/test/workers/repository/onboarding.spec.js b/test/workers/repository/onboarding.spec.js index 623748329c..3e6cd815ec 100644 --- a/test/workers/repository/onboarding.spec.js +++ b/test/workers/repository/onboarding.spec.js @@ -281,12 +281,8 @@ describe('lib/workers/repository/onboarding', () => { expect(config.api.commitFilesToBranch.mock.calls[0]).toMatchSnapshot(); }); it('skips commit files if existing content matches', async () => { - const existingContent = `{ - "pinVersions": true, - "depTypes": [{"depType": "dependencies", "pinVersions": false}], - "ignoreNodeModules": true -} -`; + const existingContent = + '{\n "pinVersions": true,\n "dependencies": {"pinVersions": false},\n "ignoreNodeModules": true\n}\n'; config.api.getFileContent.mockReturnValueOnce(existingContent); const res = await onboarding.getOnboardingStatus(config); expect(res).toEqual(false); -- GitLab