From ba16ac494ad48c3109476da1bfd3846822ba262d Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Tue, 17 Oct 2023 12:30:36 -0500 Subject: [PATCH 01/10] fix: npm link does not respect --no-save --- lib/commands/link.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/commands/link.js b/lib/commands/link.js index 0c0929115a557..2e7ee03cdd57f 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -108,6 +108,7 @@ class Link extends ArboristWorkspaceCmd { // npm link should not save=true by default unless you're // using any of --save-dev or other types const save = + !Boolean(this.npm.config.argv[3] === '--no-save') && Boolean( this.npm.config.find('save') !== 'default' || this.npm.config.get('save-optional') || From 126a3aad0e7c4db3ed2ab72f81bd42e6ae5df0d6 Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Thu, 26 Oct 2023 11:23:29 -0500 Subject: [PATCH 02/10] fix: npm link does not respect --no-save --- lib/commands/link.js | 6 +++--- test/lib/commands/link.js | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/commands/link.js b/lib/commands/link.js index 2e7ee03cdd57f..a2b1df1aee97d 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -64,6 +64,7 @@ class Link extends ArboristWorkspaceCmd { } async linkInstall (args) { + // load current packages from the global space, // and then add symlinks installs locally const globalTop = resolve(this.npm.globalDir, '..') @@ -108,15 +109,14 @@ class Link extends ArboristWorkspaceCmd { // npm link should not save=true by default unless you're // using any of --save-dev or other types const save = - !Boolean(this.npm.config.argv[3] === '--no-save') && Boolean( - this.npm.config.find('save') !== 'default' || + (this.npm.config.find('save') !== 'default' && + this.npm.config.get('save')) || this.npm.config.get('save-optional') || this.npm.config.get('save-peer') || this.npm.config.get('save-dev') || this.npm.config.get('save-prod') ) - // create a new arborist instance for the local prefix and // reify all the pending names as symlinks there const localArb = new Arborist({ diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index 65792fd141acb..81a94641c1fe9 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -315,14 +315,13 @@ t.test('link pkg already in global space', async t => { }) // XXX: how to convert this to a config that gets passed in? - npm.config.find = () => 'default' + npm.config.find = () => '--save' await link.exec(['@myscope/linked']) - - t.equal( + t.match( require(resolve(prefix, 'package.json')).dependencies, - undefined, - 'should not save to package.json upon linking' + { '@myscope/linked': 'file:../other/scoped-linked' }, + 'should save to package.json upon linking' ) t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') From 5abafa12b6277513cdc975546123f06911863802 Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Thu, 26 Oct 2023 11:50:23 -0500 Subject: [PATCH 03/10] fix: npm link does not respect --no-save --- lib/commands/link.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/commands/link.js b/lib/commands/link.js index a2b1df1aee97d..cdc248569849c 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -64,7 +64,6 @@ class Link extends ArboristWorkspaceCmd { } async linkInstall (args) { - // load current packages from the global space, // and then add symlinks installs locally const globalTop = resolve(this.npm.globalDir, '..') @@ -110,7 +109,7 @@ class Link extends ArboristWorkspaceCmd { // using any of --save-dev or other types const save = Boolean( - (this.npm.config.find('save') !== 'default' && + (this.npm.config.find('save') !== 'default' && this.npm.config.get('save')) || this.npm.config.get('save-optional') || this.npm.config.get('save-peer') || From 167533a355645286175a6c104a7acb12b9763cdc Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Mon, 30 Oct 2023 10:42:46 -0500 Subject: [PATCH 04/10] New Test Case Added --- test/lib/commands/link.js | 41 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index 81a94641c1fe9..8dba0edb5b8eb 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -314,6 +314,45 @@ t.test('link pkg already in global space', async t => { }, }) + // XXX: how to convert this to a config that gets passed in? + npm.config.find = () => 'default' + + await link.exec(['@myscope/linked']) + t.equal( + require(resolve(prefix, 'package.json')).dependencies, + undefined, + 'should not save to package.json upon linking' + ) + + t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') +}) + + +t.test('saving link to package file', async t => { + const { npm, link, printLinks, prefix } = await mockLink(t, { + globalPrefixDir: { + node_modules: { + '@myscope': { + linked: t.fixture('symlink', '../../../other/scoped-linked'), + }, + }, + }, + otherDirs: { + 'scoped-linked': { + 'package.json': JSON.stringify({ + name: '@myscope/linked', + version: '1.0.0', + }), + }, + }, + prefixDir: { + 'package.json': JSON.stringify({ + name: 'my-project', + version: '1.0.0', + }), + }, + }) + // XXX: how to convert this to a config that gets passed in? npm.config.find = () => '--save' @@ -323,8 +362,6 @@ t.test('link pkg already in global space', async t => { { '@myscope/linked': 'file:../other/scoped-linked' }, 'should save to package.json upon linking' ) - - t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') }) t.test('link pkg already in global space when prefix is a symlink', async t => { From 2bae2e1be0a694e7edd5257c4ac4cbf6cc15c651 Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Mon, 30 Oct 2023 10:48:53 -0500 Subject: [PATCH 05/10] New Test Case Added --- test/lib/commands/link.js | 67 ++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index 8dba0edb5b8eb..68bdc43405807 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -316,8 +316,9 @@ t.test('link pkg already in global space', async t => { // XXX: how to convert this to a config that gets passed in? npm.config.find = () => 'default' - + await link.exec(['@myscope/linked']) + t.equal( require(resolve(prefix, 'package.json')).dependencies, undefined, @@ -327,17 +328,19 @@ t.test('link pkg already in global space', async t => { t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') }) - -t.test('saving link to package file', async t => { +t.test('link pkg already in global space when prefix is a symlink', async t => { const { npm, link, printLinks, prefix } = await mockLink(t, { - globalPrefixDir: { - node_modules: { - '@myscope': { - linked: t.fixture('symlink', '../../../other/scoped-linked'), - }, - }, - }, + globalPrefixDir: t.fixture('symlink', './other/real-global-prefix'), otherDirs: { + // mockNpm does this automatically but only for globalPrefixDir so here we + // need to do it manually since we are making a symlink somewhere else + 'real-global-prefix': mockNpm.setGlobalNodeModules({ + node_modules: { + '@myscope': { + linked: t.fixture('symlink', '../../../scoped-linked'), + }, + }, + }), 'scoped-linked': { 'package.json': JSON.stringify({ name: '@myscope/linked', @@ -353,30 +356,29 @@ t.test('saving link to package file', async t => { }, }) - // XXX: how to convert this to a config that gets passed in? - npm.config.find = () => '--save' + npm.config.find = () => 'default' await link.exec(['@myscope/linked']) - t.match( + + t.equal( require(resolve(prefix, 'package.json')).dependencies, - { '@myscope/linked': 'file:../other/scoped-linked' }, - 'should save to package.json upon linking' + undefined, + 'should not save to package.json upon linking' ) + + t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') }) -t.test('link pkg already in global space when prefix is a symlink', async t => { +t.test('saving link to package file', async t => { const { npm, link, printLinks, prefix } = await mockLink(t, { - globalPrefixDir: t.fixture('symlink', './other/real-global-prefix'), - otherDirs: { - // mockNpm does this automatically but only for globalPrefixDir so here we - // need to do it manually since we are making a symlink somewhere else - 'real-global-prefix': mockNpm.setGlobalNodeModules({ - node_modules: { - '@myscope': { - linked: t.fixture('symlink', '../../../scoped-linked'), - }, + globalPrefixDir: { + node_modules: { + '@myscope': { + linked: t.fixture('symlink', '../../../other/scoped-linked'), }, - }), + }, + }, + otherDirs: { 'scoped-linked': { 'package.json': JSON.stringify({ name: '@myscope/linked', @@ -392,17 +394,16 @@ t.test('link pkg already in global space when prefix is a symlink', async t => { }, }) - npm.config.find = () => 'default' + // XXX: how to convert this to a config that gets passed in? + npm.config.find = () => '--save' await link.exec(['@myscope/linked']) - - t.equal( + + t.match( require(resolve(prefix, 'package.json')).dependencies, - undefined, - 'should not save to package.json upon linking' + { '@myscope/linked': 'file:../other/scoped-linked' }, + 'should save to package.json upon linking' ) - - t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') }) t.test('should not prune dependencies when linking packages', async t => { From a427a182aa0e4b161ece528747be0b344f02c46a Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Mon, 30 Oct 2023 10:55:15 -0500 Subject: [PATCH 06/10] New Test Case Added --- test/lib/commands/link.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index 68bdc43405807..04e199b2380ee 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -370,7 +370,7 @@ t.test('link pkg already in global space when prefix is a symlink', async t => { }) t.test('saving link to package file', async t => { - const { npm, link, printLinks, prefix } = await mockLink(t, { + const { npm, link, prefix } = await mockLink(t, { globalPrefixDir: { node_modules: { '@myscope': { @@ -398,7 +398,6 @@ t.test('saving link to package file', async t => { npm.config.find = () => '--save' await link.exec(['@myscope/linked']) - t.match( require(resolve(prefix, 'package.json')).dependencies, { '@myscope/linked': 'file:../other/scoped-linked' }, From 8652845854862ce18eb0aeb7639268c17b6d058f Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Mon, 30 Oct 2023 13:23:49 -0500 Subject: [PATCH 07/10] Test Cases Changes For --no-save --- test/lib/commands/link.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index 04e199b2380ee..a6794503406f0 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -369,7 +369,7 @@ t.test('link pkg already in global space when prefix is a symlink', async t => { t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') }) -t.test('saving link to package file', async t => { +t.test('should not saving link to package file', async t => { const { npm, link, prefix } = await mockLink(t, { globalPrefixDir: { node_modules: { @@ -392,16 +392,15 @@ t.test('saving link to package file', async t => { version: '1.0.0', }), }, + config: { save: false }, }) - // XXX: how to convert this to a config that gets passed in? - npm.config.find = () => '--save' - + npm.config.find = () => 'cli' await link.exec(['@myscope/linked']) t.match( require(resolve(prefix, 'package.json')).dependencies, - { '@myscope/linked': 'file:../other/scoped-linked' }, - 'should save to package.json upon linking' + undefined, + 'should not save to package.json upon linking' ) }) From 7f27dda340bbe49befab78f11d6467b63b20cecb Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Mon, 30 Oct 2023 13:29:36 -0500 Subject: [PATCH 08/10] Test Cases Changes For --no-save --- test/lib/commands/link.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index a6794503406f0..ef1d968733f3c 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -395,7 +395,7 @@ t.test('should not saving link to package file', async t => { config: { save: false }, }) - npm.config.find = () => 'cli' + console.log('222222222222222 ', npm.config.find('save')) await link.exec(['@myscope/linked']) t.match( require(resolve(prefix, 'package.json')).dependencies, From b832059d9f077f5dc30c34d7ae6536071d3d1c66 Mon Sep 17 00:00:00 2001 From: santoshRaj Date: Mon, 30 Oct 2023 13:35:31 -0500 Subject: [PATCH 09/10] issue --no-save fixed --- test/lib/commands/link.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index ef1d968733f3c..c2da1c16f2da9 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -370,7 +370,7 @@ t.test('link pkg already in global space when prefix is a symlink', async t => { }) t.test('should not saving link to package file', async t => { - const { npm, link, prefix } = await mockLink(t, { + const { link, prefix } = await mockLink(t, { globalPrefixDir: { node_modules: { '@myscope': { @@ -395,7 +395,6 @@ t.test('should not saving link to package file', async t => { config: { save: false }, }) - console.log('222222222222222 ', npm.config.find('save')) await link.exec(['@myscope/linked']) t.match( require(resolve(prefix, 'package.json')).dependencies, From 57b8e64bc2736221bc48f86e2a6a0be1b5a7552a Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 31 Oct 2023 08:49:23 -0700 Subject: [PATCH 10/10] fixup! Update test/lib/commands/link.js --- test/lib/commands/link.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/commands/link.js b/test/lib/commands/link.js index c2da1c16f2da9..85bada28d67b1 100644 --- a/test/lib/commands/link.js +++ b/test/lib/commands/link.js @@ -369,7 +369,7 @@ t.test('link pkg already in global space when prefix is a symlink', async t => { t.matchSnapshot(await printLinks(), 'should create a local symlink to global pkg') }) -t.test('should not saving link to package file', async t => { +t.test('should not save link to package file', async t => { const { link, prefix } = await mockLink(t, { globalPrefixDir: { node_modules: {