Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] npm install replaces links with downloaded modules #2372

Closed
j1elo opened this issue Dec 17, 2020 · 20 comments
Closed

[BUG] npm install replaces links with downloaded modules #2372

j1elo opened this issue Dec 17, 2020 · 20 comments
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@j1elo
Copy link

j1elo commented Dec 17, 2020

---- EDIT 2021-05-10: Added how to reproduce in comment below.

npm install replaces link with downloaded module

Current Behavior

After running npm link, a symlink is created to the target dependency. Afterwards, other npm commands such as npm install will break the mentioned link:

# ("openvidu-browser" is a global link that has
# been previously created with `sudo npm link`...)

$ npm link openvidu-browser
$ ls -l node_modules | grep openvidu-browser
openvidu-browser -> ../../../openvidu-generateOffer/openvidu-browser
# (Any other install command)
$ npm install --no-save @ionic/cli

# Show the bug: "openvidu-browser" is now a downloaded dependency (following what is defined in package.json)
# instead of the expected symlink to a local version of the dependency.
$ ls -l node_modules | grep openvidu-browser
openvidu-browser # UNEXPECTED: NOT A LINK

Expected Behavior

After running npm link, a symlink is created to the target dependency. Afterwards, other npm commands such as npm install don't alter the mentioned link:

# ("openvidu-browser" is a global link that has
# been previously created with `sudo npm link`...)

$ npm link openvidu-browser
$ ls -l node_modules | grep openvidu-browser
openvidu-browser -> ../../../openvidu-generateOffer/openvidu-browser
# (Any other install command)
$ npm install --no-save @ionic/cli

# Show the expected behavior: "openvidu-browser" is still a symlink to a local version of the dependency.
$ ls -l node_modules | grep openvidu-browser
openvidu-browser -> ../../../openvidu-generateOffer/openvidu-browser

Description

For context, this is a well-known and long-standing bug that has been causing confusion to users and has been reported multiple times in the past. Here are some relevant or related reports:

Some of these reports were closed with the conclusion that the issue would definitely not be fixed in current versions (<= 6) of NPM, and instead a fix would be postponed for a future NPM v7, citing "an overhaul of how npm link works" and this RFC.

After bumping into this bug today, recollecting all this information, and updating my NPM version to 7.2.0, I was surprised to find that links are still lost when any further install operation is done after running npm link.

So I guess the question is: was this really fixed with NPM 7 and is now a regression?

Environment:

  • OS: Ubuntu 18.04
  • Node: Tested on v14 and v15
  • npm: 7.2.0
@j1elo j1elo added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Dec 17, 2020
@j1elo
Copy link
Author

j1elo commented Dec 17, 2020

In case this could be helpful for anyone, I was able to work around this issue by using npm install <path> instead of npm link:

cd /path/to/my-dependency
npm install && npm run build

cd /path/to/my-project
npm install /path/to/my-dependency
npm install

@darcyclarke darcyclarke added Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Jan 29, 2021
@darcyclarke darcyclarke added this to the OSS - Sprint 23 milestone Jan 29, 2021
@nlf
Copy link
Contributor

nlf commented Feb 3, 2021

we've made some changes with regards to how linked modules are handled, can you try updating to the latest release 7.5.2 and let us know if you still experience this problem?

@IronGeek
Copy link

This should be reopen, the bug still not fixed (or maybe regressed) on npm 7.11.2

@j1elo
Copy link
Author

j1elo commented May 10, 2021

I agree this shouldn't have been closed, unless the issue had maybe been fixed and now it is showing up again as a regression. Anyway, this bug is not an intricate or difficult to reproduce one, at all; actually if anything it is pretty trivial to check. So much, that I'm surprised this is not having a bigger impact on lots of NPM users. Are we just using npm link wrong?

For future reference I'm adding here a complete set of example steps that can be followed to see the problem in action; these can be used at any future time to re-check if the problem persists.

@j1elo
Copy link
Author

j1elo commented May 10, 2021

How to reproduce

Check versions:

$ node -v
v14.16.1

$ npm -v
7.12.0

Clone sample project

$ cd /tmp/ \
    && git clone https://github.com/OpenVidu/openvidu.git \
    && cd openvidu/

Create a link

$ pushd openvidu-browser \
    && npm install \
    && sudo npm link \
    && popd

Verify that the link has been created in global prefix

$ find /usr/lib/node_modules -type l -name 'openvidu-browser' -ls
lrwxrwxrwx /usr/lib/node_modules/openvidu-browser -> ../../../tmp/openvidu/openvidu-browser

Make use of the link

$ cd openvidu-testapp/ \
    && npm install \
    && npm link openvidu-browser

Verify that the link has been used

$ find ./node_modules -type l -name 'openvidu-browser' -ls
lrwxrwxrwx ./node_modules/openvidu-browser -> ../../openvidu-browser

Run any other install command. This step is what triggers the reported bug:

$ npm install --no-save @ionic/cli

Verify if the link is still present (good), or if it has been replaced (bad)

$ find ./node_modules -type l -name 'openvidu-browser' -ls
# (No output, which means the link has been replaced)

@SystemParadox
Copy link

I'm surprised this is not having a bigger impact on lots of NPM users. Are we just using npm link wrong?

npm link has never worked right. Most users have just given up on it.

My biggest hope for npm 7 was the ability to finally use npm link, but I ended up here instead.

@ljharb
Copy link
Contributor

ljharb commented May 10, 2021

afaik, since npm 5, this is how npm install has worked. It's annoying, but it's by design.

Any time you install something, you have to manually recreate all your links.

@Venryx
Copy link

Venryx commented May 10, 2021

It's annoying, but it's by design.

It's not by design. The NPM team has talked about it before, and said npm 7 was going to fix it, but then it never was fixed:

We are well aware of npm link issues, and it will be overhauled with npm@7 later this year. As @legodude17 says, please refer to https://npm.community/t/new-npm-link-command/24/1 for more information on this, and discussions about what it should do. Until then, consider npm as it is to be broken and the overhaul to be the fix.

The comment above was made three years ago...

(The original post of the linked thread is not exactly the same issue -- it's about npm install "stealing" subdependencies from npm-linked dependencies rather than outright replacing the npm-linked dependencies -- but it's a similar issue, and those contributing to the thread were referencing both kinds of misbehavior. This is true also for the thread for the merged pull-request, which people were hoping was going to fix it.)

@ljharb
Copy link
Contributor

ljharb commented May 10, 2021

That there's tons of issues with it, and that the plan is to fix it, does not mean it wasn't originally by design.

@Venryx
Copy link

Venryx commented May 18, 2021

If this issue is on the npm dev team's radar, can we get an update on the roadmap/plan for its resolution?

npm link is a core feature for me and many other projects (for example, I use linked packages all the time, for almost every app I work on), and that this critical bug has existed for over three years (requiring me to always use the 3rd-party tool npm-safe-install just to install new packages) makes me think the dev-team may be:

  1. Unaware of the problem.
  2. They don't use package symlinking functionality. (eg. using monorepos for everything)
  3. They've transitioned to using external, 3rd-party tools for it. (eg. npm-safe-install, yarn)

Whichever of those three is the case, the lack of response on it is concerning, as it gives the impression that the dev team may not have the resources/motivation to maintain the command's functionality/reliability.

I've been annoyed by this issue for enough months/years that I'm finally seriously considering switching to Yarn now (despite most of Yarn's previous advantages having been "caught up to" by npm).

That today I learned about Yarn's plugin system (which NPM doesn't have an equivalent of, due partly to NPM's file-based hook scripts not working on Windows) has pushed me right to the edge. (I'm torn between NPM's ubiquity, and Yarn's solving of a few key "npm pain points".)

A resolution would be ideal; but I'll settle for just a dev acknowledgement + discussion at this point!

@SystemParadox
Copy link

If this is still broken I would suggest opening a new ticket to have any chance of being noticed.

@j1elo
Copy link
Author

j1elo commented May 22, 2021

If this is still broken I would suggest opening a new ticket to have any chance of being noticed.

That'd be a full-circle closure for this issue, I have to admit. A self-fulfilling prophecy where this report ends up joining the long trail of closed and/or forgotten others that are listed in the initial description.

If anyone opens a new issue, it might be useful that the past reports research that I did was copied over too, so the full context doesn't get lost. Good luck with that, though; seems their policy is to close well documented reports (like this one) even if the issue is well known, trivial to reproduce, and regardless of whether it is actually fixed or not. I don't care any more to follow up with more reporting.

@rektide
Copy link

rektide commented Jul 4, 2021

@nlf @darcyclarke can we please re-open this issue? this is still present. i feel like the in-depth write-up of this ticket makes this extra-worthy of re-opening versus opening another ticket.

@darcyclarke darcyclarke self-assigned this Jul 7, 2021
@Venryx
Copy link

Venryx commented Aug 17, 2021

Yet again, npm install after a link being present caused the directory that was symlinked to be completely replaced with the latest NPM release. If I did not have online Git backups of that library, I would have lost all progress on it since its last npm release.

Note that not only does the library's working-directory get nuked -- literally everything inside the symlinked folder gets nuked, including the .git folder. So having Git backups is not enough, you need them to also be online, to keep npm install (after npm link) from potentially destroying everything in the symlinked folder permanently. (I'm not sure of the exact conditions which will trigger this, but it has happened several times now)

Beware of using npm link my-library, without a (frequently updated) online copy of the library!

(FWIW, I've since switched to using yarn for all my new projects; haven't converted everything to it yet though. For people annoyed by Yarn 2's inability to have user-specific symlinks, I made a Yarn plugin that adds support for this.)

@SystemParadox
Copy link

SystemParadox commented Aug 17, 2021

@Venryx WOAH really?

This bug is about the links themselves being deleted. If npm is reaching through the link and deleting files inside the linked directory then please open a separate issue as that's incredibly serious and the npm team doesn't seem to be paying much attention to this one.

@isaacs
Copy link
Contributor

isaacs commented Aug 17, 2021

Re: #2372 (comment) (et al)

The issue with the link being removed on a subsequent npm install is owing to the very much by design (and, I agree, confusing and weird) behavior that npm link does not by default save package.json or package-lock.json. There was a fair bit of discussion around this at the time (back in 2017 or so, I believe?) but it is a thing we intend to thoroughly review and significantly change in a future semver-major update. We did not opt to change it in npm v7 for the simple fact that we already were adding some other rather large changes, and wanted to limit the amount of disruption between v6 and v7 (which meant preserving some behaviors that are questionable at best).

As @j1elo mentioned, the workaround for this is to explicitly install a path with npm install file:/some/path/to/thing, which will create a symbolic link and save the dependency to the package.json and lockfile.

Another workaround is to set --save explicitly when running npm link. So, for example, instead of npm link openvidu-browser, you'd do npm link openvidu-browser --save.

So having Git backups is not enough, you need them to also be online, to keep npm install (after npm link) from potentially destroying everything in the symlinked folder permanently. (I'm not sure of the exact conditions which will trigger this, but it has happened several times now)

@Venryx This is a different issue than the OP in this issue. Can you please open a new issue, and provide as much reproduction details as possible? At the very least, operating system and versions of npm and node in use. The best would be a minified reproduction case that shows the problem. That one has been reported a few times, and it's proven very tricky to track down, but it is possible that I'm about to fix it, since it seems like it might (in theory) match the pattern of another issue I'm working on. Thanks!

@thw0rted
Copy link

@isaacs -- it sounds like there isn't a path forward for npm link to just DWIM.

link is necessarily a "local" operation, against the system where I ran the command, and no other. I'd make the strong argument that link should never make a change that winds up in source control -- that, or I fundamentally misunderstand how link is meant to be used. At the same time, I want link changes to persist until I unlink. I want link/unlink to be a mode-switch between "use the repo version of this package" and "use this local directory in place of the package".

I'm not a Node/npm engineer, but if I were designing this feature I'd want to persist link changes in a place other than package{-lock}.json, ideally something that we tell everybody not to check into source control -- something like ~/.npmrc. (There certainly might be reasons why that's a terrible solution, it's just the first place I could think of that meets the criteria.)

@joshjung
Copy link

FWIW, my suggestion on this issue would perhaps be to have a package.local.json that could be ignored by the repo. Every time one does a npm link this file would be updated with the locally linked package so that when npm i is run in the future it can relink everything in that local package.json. This would also allow a lot of scalability for "local only" npm settings in the future.

@mildsunrise
Copy link

it is a thing we intend to thoroughly review and significantly change in a future semver-major update

it's been 3 years and 3 new major versions, and this has AFAIK not been addressed.

can we at least please reopen this? @isaacs

@isaacs
Copy link
Contributor

isaacs commented Jul 24, 2024

I no longer work on npm, haven't for about three years now. Please don't ping me on these issues, I can't help you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests