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

pnpm link status message #615

Closed
vjpr opened this issue Feb 9, 2017 · 9 comments · Fixed by #6956
Closed

pnpm link status message #615

vjpr opened this issue Feb 9, 2017 · 9 comments · Fixed by #6956

Comments

@vjpr
Copy link
Contributor

vjpr commented Feb 9, 2017

There should be a message after a pnpm link X or pnpm link command to show what has been linked where.

It should also show a warning that the linked module will not be able to use peer dependencies from the current module (as discussed in #604). It should search the linkee's package.json file to check for peer deps.

pnpm version: 0.53.0

@vjpr vjpr changed the title pnpm link status message pnpm link status message Feb 9, 2017
@zkochan zkochan added this to Priority in Status Feb 16, 2017
@zkochan
Copy link
Member

zkochan commented Feb 16, 2017

Message regarding what linked where added here 9bab641

Does npm print messages about peer deps in this situation?

@vjpr
Copy link
Contributor Author

vjpr commented Feb 17, 2017

npm@2

➜  foo npm link
npm WARN peerDependencies The peer dependency bar included from foo will no
npm WARN peerDependencies longer be automatically installed to fulfill the peerDependency
npm WARN peerDependencies in npm 3+. Your application will need to depend on it explicitly.
/Users/Vaughan/nvm/versions/node/v6.5.0/lib/node_modules/foo -> /Users/Vaughan/dev-scratch/foo

npm@4

➜  foo npm link
npm WARN foo@1.0.0 No description
npm WARN foo@1.0.0 No repository field.
/Users/Vaughan/nvm/versions/node/v7.5.0/lib/node_modules/foo -> /Users/Vaughan/dev-scratch/foo
{
  "name": "foo",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "peerDependencies": {
    "bar": "*"
  }
}

@vjpr
Copy link
Contributor Author

vjpr commented Feb 17, 2017

So npm doesn't, but I think we should show a message because with npm you can use --preserve-symlinks to ensure the same cached module is used for a npm linked peer dependency, but with pnpm it is impossible to use the same cached module of a npm linked peer dep, amongst dependencies.

@vjpr
Copy link
Contributor Author

vjpr commented Feb 17, 2017

Thes message should be something like this:

pnpm link foo

Prints:


The package foo, which you have just pnpm linked has the following peerDependencies specified in its package.json:

  • bar@*

Warning: Whilst foo is linked, when a peer dependency is required inside package foo, this peer dependency will refer to foo's local version, not your project's version. This can cause issues if there should be only one instance of the peer dependency during runtime (e.g. react). It may also increase your project's startup time because you would have multiple versions of a package and all its dependencies being evaulated.

To workaround this issue, in Node.js you can modify the internal require resolver by monkey-patching Module._resolveFilename. Or when using webpack you can use resolve.alias or resolve.modules config settings to acheive the same behaviour. See <TODO: Some FAQ entry on the wiki>.


Its a long message, but pnpm link X is not used that often, and if users don't understand this limitation they will encounter crazy bugs.

If we can create a good FAQ article about this, and maybe a blessed require-hack module to ensure the correct peer dep is resolved (I have already written a POC), then there would be no one who can say that pnpm's non-standard hardlinking breaks framework development.

zkochan added a commit that referenced this issue Jul 7, 2018
BREAKING CHANGE:

Linked packages are logged via the added log object.

ref #818
ref #615
@chpio
Copy link

chpio commented Jan 24, 2019

there is still no message when linking to packages with a peerDep. It's not even installing a local instance of the peerDep at the moment.

@vjpr
Copy link
Contributor Author

vjpr commented Feb 13, 2020

Bump. I always think its a bug.

@zkochan zkochan added this to the v4.x milestone Feb 13, 2020
@zkochan zkochan modified the milestones: v5.x, v6.x Mar 23, 2021
@DylanCulfogienis
Copy link

Bump.

@rickstaa
Copy link

This is a reasonable feature request since the current behaviour is confusing.

@zkochan
Copy link
Member

zkochan commented Dec 19, 2021

It is possible to link dependencies so that they reuse the peer dependencies of the project.

Use injected: https://pnpm.io/package_json#dependenciesmetainjected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Status
Priority
Development

Successfully merging a pull request may close this issue.

5 participants