From 458c399909f551c7c8a0f05d4c12c118165ae088 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Tue, 2 Mar 2021 13:02:07 -0500 Subject: [PATCH 1/5] feat: dev & peer dependency handling Handle the dev and peer dependencies in the same way as normal dependencies --- lib/context.js | 16 +++++++++++++++- lib/test.js | 9 +++++---- test/fixtures/config.js | 28 ++++++++++++++++++++++++++++ test/test.js | 17 ++++++++++++++++- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/lib/context.js b/lib/context.js index 1b73ef3..1793878 100644 --- a/lib/context.js +++ b/lib/context.js @@ -11,6 +11,20 @@ exports.getLocalPackageJSON = async function getLocalPackageJSON () { return JSON.parse(pkg) } +/* +The check should search both the dev and peer dependencies to find out if the dependency is +regular, dev or peer. We only want to check on this the once :) + */ exports.checkPackageInPackageJSON = function checkPackageInPackageJSON (dep, packageJSON) { - return Object.prototype.hasOwnProperty.call(packageJSON.dependencies, dep) + return ( + (Object.prototype.hasOwnProperty.call(packageJSON, 'dependencies') && Object.prototype.hasOwnProperty.call(packageJSON.dependencies, dep) + ? 'dependencies' + : false) || + (Object.prototype.hasOwnProperty.call(packageJSON, 'devDependencies') && Object.prototype.hasOwnProperty.call(packageJSON.devDependencies, dep) + ? 'devDependencies' + : false) || + (Object.prototype.hasOwnProperty.call(packageJSON, 'peerDependencies') && Object.prototype.hasOwnProperty.call(packageJSON.peerDependencies, dep) + ? 'peerDependencies' + : false) + ) } diff --git a/lib/test.js b/lib/test.js index ccf9501..c02861b 100644 --- a/lib/test.js +++ b/lib/test.js @@ -47,12 +47,13 @@ const getCommitURL = async function getCommitURL (owner, repo, hash) { } const applyPatch = module.exports.applyPatch = -function applyPatch (patch, module, packageJSON) { - if (!context.checkPackageInPackageJSON(module, packageJSON)) { +function applyPatch (patch, module, dependentPkgJSON) { + const dependencyType = context.checkPackageInPackageJSON(module, dependentPkgJSON) + if (!dependencyType) { throw new Error('Dependency not found in package.json') } - packageJSON.dependencies[module] = patch - return packageJSON + dependentPkgJSON[dependencyType][module] = patch + return dependentPkgJSON } async function pushPatch (packageJSON, owner, repo, dep) { diff --git a/test/fixtures/config.js b/test/fixtures/config.js index 18275b2..a29d60e 100644 --- a/test/fixtures/config.js +++ b/test/fixtures/config.js @@ -8,7 +8,33 @@ module.exports.PATCHED_PKGJSON = Object.assign({}, module.exports.PKGJSON, { { '@pkgjs/wiby': 'pkgjs/wiby#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' } ) }) + +module.exports.PKGJSON_DEV_DEPS = Object.assign({}, module.exports.PKGJSON, { + devDependencies: { + '@other/name': '*' + } +}) +module.exports.PATCHED_DEV_DEPS_PKGJSON = Object.assign({}, module.exports.PKGJSON, { + devDependencies: Object.assign( + {}, + { '@other/name': 'other/name#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' } + ) +}) + +module.exports.PKGJSON_PEER_DEPS = Object.assign({}, module.exports.PKGJSON, { + peerDependencies: { + '@other/plugin': '*' + } +}) +module.exports.PATCHED_PEER_DEPS_PKGJSON = Object.assign({}, module.exports.PKGJSON, { + peerDependencies: Object.assign( + {}, + { '@other/plugin': 'other/plugin#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' } + ) +}) module.exports.PATCH = 'pkgjs/wiby#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' +module.exports.DEV_DEP_PATCH = 'other/name#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' +module.exports.PEER_DEP_PATCH = 'other/plugin#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' module.exports.PKG_NAME_UNIT = '@pkgjs/wiby' module.exports.PKG_NAME_INTEGRATION = 'wiby' module.exports.PKG_REPO = 'wiby' @@ -17,3 +43,5 @@ module.exports.PKG_HEAD_SHA = '577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' module.exports.DEP_REPO = 'pass' module.exports.DEP_ORG = 'wiby-test' module.exports.DEP_REPO_PERM = 'ADMIN' +module.exports.DEV_DEP_PKG_NAME_UNIT = '@other/name' +module.exports.DEV_PEER_PKG_NAME_UNIT = '@other/plugin' diff --git a/test/test.js b/test/test.js index ef60c7b..62c555b 100644 --- a/test/test.js +++ b/test/test.js @@ -15,10 +15,19 @@ tap.afterEach(async () => { nock.enableNetConnect() }) -tap.test('Check patch applied to package.json successfully', tap => { +tap.test('Check patch applied to dependency package.json successfully', tap => { tap.equal(JSON.stringify(wiby.test.applyPatch(CONFIG.PATCH, CONFIG.PKG_NAME_UNIT, CONFIG.PKGJSON)), JSON.stringify(CONFIG.PATCHED_PKGJSON)) tap.end() }) +tap.test('Check patch applied to dev dependency package.json successfully', tap => { + tap.equal(JSON.stringify(wiby.test.applyPatch(CONFIG.DEV_DEP_PATCH, CONFIG.DEV_DEP_PKG_NAME_UNIT, CONFIG.PKGJSON_DEV_DEPS)), JSON.stringify(CONFIG.PATCHED_DEV_DEPS_PKGJSON)) + tap.end() +}) + +tap.test('Check patch applied to peer dependency package.json successfully', tap => { + tap.equal(JSON.stringify(wiby.test.applyPatch(CONFIG.PEER_DEP_PATCH, CONFIG.DEV_PEER_PKG_NAME_UNIT, CONFIG.PKGJSON_PEER_DEPS)), JSON.stringify(CONFIG.PATCHED_PEER_DEPS_PKGJSON)) + tap.end() +}) tap.test('applyPatch() checks package exists in dependant package.json', tap => { tap.throws( @@ -29,6 +38,12 @@ tap.test('applyPatch() checks package exists in dependant package.json', tap => { dependencies: { 'other-package': '*' + }, + devDependencies: { + 'further-packages': '*' + }, + peerDependencies: { + 'some-plugin': '*' } } ) From 273d749de029dd912906b25a2baf13480998f9b6 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Tue, 2 Mar 2021 14:49:23 -0500 Subject: [PATCH 2/5] feat: dev & peer dependency handling add optional dependencies --- lib/context.js | 22 +++++++++++----------- test/test.js | 3 +++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/context.js b/lib/context.js index 1793878..8449b61 100644 --- a/lib/context.js +++ b/lib/context.js @@ -16,15 +16,15 @@ The check should search both the dev and peer dependencies to find out if the de regular, dev or peer. We only want to check on this the once :) */ exports.checkPackageInPackageJSON = function checkPackageInPackageJSON (dep, packageJSON) { - return ( - (Object.prototype.hasOwnProperty.call(packageJSON, 'dependencies') && Object.prototype.hasOwnProperty.call(packageJSON.dependencies, dep) - ? 'dependencies' - : false) || - (Object.prototype.hasOwnProperty.call(packageJSON, 'devDependencies') && Object.prototype.hasOwnProperty.call(packageJSON.devDependencies, dep) - ? 'devDependencies' - : false) || - (Object.prototype.hasOwnProperty.call(packageJSON, 'peerDependencies') && Object.prototype.hasOwnProperty.call(packageJSON.peerDependencies, dep) - ? 'peerDependencies' - : false) - ) + const dependencyKeyNames = [ + 'dependencies', + 'devDependencies', + 'peerDependencies', + 'optionalDependencies' + ] + const checkKeyHasDep = (key) => Object.prototype.hasOwnProperty.call(packageJSON, key) && Object.prototype.hasOwnProperty.call(packageJSON[key], dep) + for (let i = 0; i < dependencyKeyNames.length; i++) { + if (checkKeyHasDep(dependencyKeyNames[i])) return dependencyKeyNames[i] + } + return false } diff --git a/test/test.js b/test/test.js index 62c555b..36065b5 100644 --- a/test/test.js +++ b/test/test.js @@ -44,6 +44,9 @@ tap.test('applyPatch() checks package exists in dependant package.json', tap => }, peerDependencies: { 'some-plugin': '*' + }, + optionalDependencies: { + 'some-optional-dep': '*' } } ) From cb40834546a3f41e607cdcc677186a07e6edff31 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Sat, 6 Mar 2021 08:09:43 -0500 Subject: [PATCH 3/5] feat: dev & peer dependency handling PR review feedback, the code is better for it :) --- lib/context.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/context.js b/lib/context.js index 8449b61..53cd528 100644 --- a/lib/context.js +++ b/lib/context.js @@ -23,8 +23,5 @@ exports.checkPackageInPackageJSON = function checkPackageInPackageJSON (dep, pac 'optionalDependencies' ] const checkKeyHasDep = (key) => Object.prototype.hasOwnProperty.call(packageJSON, key) && Object.prototype.hasOwnProperty.call(packageJSON[key], dep) - for (let i = 0; i < dependencyKeyNames.length; i++) { - if (checkKeyHasDep(dependencyKeyNames[i])) return dependencyKeyNames[i] - } - return false + return dependencyKeyNames.find(x => checkKeyHasDep(x)) } From e9b8997924632b8af510ca7e100b4a25e888b6e5 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Thu, 11 Mar 2021 13:07:53 -0500 Subject: [PATCH 4/5] feat: dev & peer dependency handling remove extra object assigns. --- test/fixtures/config.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/fixtures/config.js b/test/fixtures/config.js index a29d60e..0698607 100644 --- a/test/fixtures/config.js +++ b/test/fixtures/config.js @@ -15,10 +15,8 @@ module.exports.PKGJSON_DEV_DEPS = Object.assign({}, module.exports.PKGJSON, { } }) module.exports.PATCHED_DEV_DEPS_PKGJSON = Object.assign({}, module.exports.PKGJSON, { - devDependencies: Object.assign( - {}, + devDependencies: { '@other/name': 'other/name#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' } - ) }) module.exports.PKGJSON_PEER_DEPS = Object.assign({}, module.exports.PKGJSON, { @@ -27,10 +25,8 @@ module.exports.PKGJSON_PEER_DEPS = Object.assign({}, module.exports.PKGJSON, { } }) module.exports.PATCHED_PEER_DEPS_PKGJSON = Object.assign({}, module.exports.PKGJSON, { - peerDependencies: Object.assign( - {}, + peerDependencies: { '@other/plugin': 'other/plugin#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' } - ) }) module.exports.PATCH = 'pkgjs/wiby#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' module.exports.DEV_DEP_PATCH = 'other/name#577c08e8fd5e1b3156ce75b2e5d9e3023dac180e' From 220c6bc6bdeb58fb99ab540dad2e580b1fb0b5c5 Mon Sep 17 00:00:00 2001 From: Glenn Hinks Date: Thu, 11 Mar 2021 16:28:22 -0500 Subject: [PATCH 5/5] feat: dev & peer dependency handling update function comment as per review. --- lib/context.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/context.js b/lib/context.js index 53cd528..59d8a5e 100644 --- a/lib/context.js +++ b/lib/context.js @@ -13,7 +13,7 @@ exports.getLocalPackageJSON = async function getLocalPackageJSON () { /* The check should search both the dev and peer dependencies to find out if the dependency is -regular, dev or peer. We only want to check on this the once :) +regular, dev or peer. */ exports.checkPackageInPackageJSON = function checkPackageInPackageJSON (dep, packageJSON) { const dependencyKeyNames = [