Nice! Why newlink: vs just usingfile:?

The reason for introducing the new specifier are:

1. Have a specifier that immediately tells someone reading apackage.json what it will do.
2. Not have fights over what a validfile: URL looks like.

Originally there was a plan to actually hard deprecate thefile: specifier and remove them, but we decided that was pointlessly disruptive. Insteadfile: will be an alias forlink:.

@iarna Sounds reasonable 👍

Have you considered the impact of having different module resolution paths when using symlinks?
I currently only use symlinks in development, while in production I let the currentfile: behaviour do its thing. Without the symlinks npm is able to move many shared packages to the top level, decreasing overall install time. This could have significant impact on deploy times if local dependencies share a lot of binary dependencies.

@timoxley It's a fair question. As it's specced right now it would do that. Maybe, if the linked module is inside the top level install root we should flatten its deps down, instead of keeping them inside the linked module. The module loader would still be able to pick them up and it'd not increase disk usage.

if the linked module is inside the top level install root we should flatten its deps down

@iarna that sounds good. Being aware of where symlinks resolve to perhaps could be considered a good general improvement to the dedupe/flattening algorithm.

This is great! I would like to know if you considered supporting the currentnpm ln use-case? I'm thinking oflink:module-name to resolve to the "global" link which in turn links to the actual module directory. Contrary to the relative link paths, this wouldn't enforce a specific directory structure. What do you think?

So after some discussion in air between@zkat and I, we're of a consensus that we should go ahead with justfile: and not add a new specifier. I'll update the RFC as appropriate.

Coverage remained the same at 86.013% when pulling43db052 on link-specifier intoa08189f on latest.

Using symlinks with node, requires a flag set for it to work correctly:nodejs/node#8749 (review). Is the intent to change how node handles symlinks also?

One nice feature about how file works today is for peer dependencies. Due to the copy instead of a symlink, peers with global singletons work as expected. When this moves to symlinks only, the resolution will make certain cases impossible to resolve. Example:

• Module A is a react component and has react as apeerDependency anddevDependency
• While working on A, I want to test this locally in Module B , so I usefile:/path/to/a.
  • Today it will do the right thing and share the global version of react as it is copied. The dependency resolution won't include a second react.
  • If instead we did a symlink, this would not work because React is installed in in A'snode_modules and would not be the same instance as in as B'snode_modules.

Let me know if that example needs more clarification or if I have misunderstood RFC.

@mlucool my understanding of this case is that it would continue to work so long as Module A is anywhere inside the directory structure for B when you link it.

That is:

module-b\ | - module-a |   \ |     node_modules \  node_modules  \    react

module-a will have itsreact hoisted to the top. It's worth being more explicit about how we handle peerDeps in this situation. I hopefully didn't misunderstand the RFC in this case.

@iarna it might be worth specifying the logic forpeerDependencies anddevDependencies for these linked submodules in the RFC, cause I don't think I'm that clear on it rn.

@zkat In my case A is outside of B always. We do not have a monorepo. Stillthis seems to imply as long as it is notetar it'll use symlinks.

My current hack (which works quite well) is to create a symlink like util that does:

1. Uninstall A
2. Install via file:/path/to/a
3. Start a file watcher for all files in A'spackage.json:files
4. Start a file watcher for A's package.json to start from 1 again

Now for any change I make it appears nearly instantly. Clearly this is a hack, but it maintains that I can separate components across repos and not worry about dependency issues.

@mlucool if you're not going to use a monorepo structure, you could literally just runnpm pack andthennpm install file://path/to/module-a/module-a-1.2.3.tgz, which will have the same behavior asnpm i file://path/to/module-a currently does. We're not changing local tarballs. It's a single extra step, but also something the CLI was already doing (it had to, in order to install the package)

and yes, we willalways create symlinks fordirectory specs. The difference is that we hoist deps normallyiff the target package is in a subdir of the current package.

@zkat I could do that to keep my hack working. I am still a little unsure about how this hoisting will play out because you can imagine something like:

examples\  example1    \      package.jsoncomponents\  component1   \     package.json

If I did afile: in example1, this would not work as far as I can tell now (assuming this is still a React example). Ibelieve this paradigm is relatively common also.

Any comments on the node flag required for this to work?

P.S. I am a BIG fan of better support for symlinks, per my commit to node.

@mlucool It's worth noting that it's tremendously unlikely for us at npm to try to change module resolution, including symlink stuff, ourselves. afaik, there's no intent for us to actually go and push for a change on that end in order to land this.

More likely, I think, is discussing the possibility of taking that flag into account when deciding how to build the hoisted tree -- some users will want that symlink preservation, some won't. I'll defer to@iarna about whether she thinks this is something worth exploring, with a note that any current hacks arounddirectory dependencies can continue to exist with thatnpm pack thing I mentioned above.

@iarna does it make sense to you to readprocess.env.NODE_PRESERVE_SYMLINKS from the env and hoisting across symlink boundaries if we find that flag?

IMO if something is outside of a module you should never touch its contents.

@mlucool As@zkat says, changing the default Node.js module loader behavior isn't on the table, which is why 'preserve-symlinks' landed as an option.

Thank you for bringing this up! What you described isn't a use-case we had discussed previously.

I think having it change install behavior based onNODE_PRESERVE_SYMLINKS is actually mandatory.npm as part of installation has to model how Node.js would load the package to correctly determine where to put its dependencies. This is a detail we've thus far not had to deal with becausenpm install has treated symlinks as opaque things it can't know details of.

So yeah, if you haveNODE_PRESERVE_SYMLINKS then npm will install transitive dependencies of linked dependencies in your local project, not in the symlinked module. That is, it will work the same way it works when the symlinked module is in a subdirectory of your project.

Hi@iarna,

We're having an issue with the "file://" specifier not being handled correctly by NPM 5. We have an automated tool that generates apackage.json with references like this:

"dependencies": {"yargs":"~4.6.0","z-schema":"~3.18.3","project1":"file:./projects/project1.tgz","project2":"file:./projects/project2.tgz",

The "project1.tgz" archive contains apackage.json file like this:

{"name":"project1","version":"0.0.0","private":true,

When NPM 4 installs it, thenode_modules/project1/package.json looks like this, as expected:

"version":"0.0.0"

However, NPM 5 for some reason writes it like this:

"version":"file:projects/project1.tgz"

Was this an intentional design change? Is it an NPM 5 bug?

It's causing trouble becauseread-package-tree chokes on this JSON value. We get an error like this:

ERROR: Failed to parse package.json for project1: Invalid version: "file:projects/project1.tgz"

I'm pretty sure this is an NPM bug. I've opened#19006

@iarna what happened to this proposal? has it been rejected?

btw. looks like something similar has beenimplemented in yarn some time ago

@adrian-gierakowskiit was merged and implemented as part ofnpm@5.0.0