pnpm: How to handle "bad" packages that require() things that aren't in their package.json?

pnpm version: `1.23.1`

Code to reproduce the issue:

Create a package.json with the following contents:

{
  "name": "test-app",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "jest-runtime": "20.0.4"
  }
}

Create a sample index.js with the following contents:

require('jest-runtime');

Run node index.js.

Expected behavior:

Everything is able to be resolved.

Actual behavior:

We have a missing dependency.

Error: Cannot find module 'slash'
    at Function.Module._resolveFilename (module.js:469:15)
    at Function.Module._load (module.js:417:25)
    at Module.require (module.js:497:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (Z:\slash\node_modules\.onedrive.pkgs.visualstudio.com\jest-runtime\20.0.4\node_modules\jest-runtime\build\ScriptTransformer.js:25:15)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)

The issue is that jest-runtime has a dependency on slash that it did not declare in its package.json, thus PNPM did not link it.

jest-runtime.js

{ 
  "name": "jest-runtime",
  "dependencies": { 
     "babel-core": "^6.0.0",
     "babel-jest": "^20.0.3",
     "babel-plugin-istanbul": "^4.0.0",
     "chalk": "^1.1.3",
     "convert-source-map": "^1.4.0",
     "graceful-fs": "^4.1.11",
     "jest-config": "^20.0.4",
     "jest-haste-map": "^20.0.4",
     "jest-regex-util": "^20.0.3",
     "jest-resolve": "^20.0.4",
     "jest-util": "^20.0.3",
     "json-stable-stringify": "^1.0.1",
     "micromatch": "^2.3.11",
     "strip-bom": "3.0.0",
     "yargs": "^7.0.2"
  },
  "devDependencies": {
     "jest-environment-jsdom": "^20.0.3",
     "jest-environment-node": "^20.0.3"
  },
}

Many people in the ecosystem are still using npm, and so are likely to continue making mistakes like this in the future. Would it be possible to solve this problem by using some sort of configuration in pnpm which allows a developer to specify certain “bad packages”, and either manually add the missing dependency or have an option where pnpm will behave like npm when linking the bad package?

The newer version of jest fixes this issue, but has a different bug that is preventing us from upgrading to it. We only started noticing this issue as we have been exploring migrating our tooling to use pnpm.

About this issue

Original URL
State: closed
Created 7 years ago
Reactions: 1
Comments: 37 (28 by maintainers)

Most upvoted comments

what can you use pnpmfile.js for?

you can rename the package’s bins
you can substitute packages from your fork
you can force all dependencies to be non-exact
you can add validation. For instance, when you don’t want some package to be used at all, because it is insecure or malware. …

zkochan on Dec 6, 2017

Well, I wouldn’t like to deprecate pnpmfile.js because it can do a lot more than yarn’s resolutions.

We can add a plugin that will convert yarn’s resolutions to pnpm’s readPackage() hook. The plugin would have to be required in the pnpmfile.js.

This solution would keep pnpmfile.js the “the single place of truth”.

On Tue, Dec 5, 2017, 12:17 Vaughan Rouesnel notifications@github.com wrote:

With the pnpmfile.js you can create a hook and publish it as a package

Definitely good to have, but I agree that a config option would be easier. Static pjson config is easier to modify with scripts.

Use case: Say you have multiple projects (Lerna monorepo for example) that each need a custom dep. The easiest thing would be to write a script to go through and add to resolutions key in pjson of each package. Now, the current pnpmfile.js approach of creating and publishing a package provides flexibility, but there is so much more friction involved in publishing a package, than changing a config setting. Also, its not easy to add this config setting to multiple packages with a script.

I’m all for config.

We should support Yarn’s resolutions:

https://github.com/yarnpkg/rfcs/blob/master/implemented/0000-selective-versions-resolutions.md#resolutions

https://yarnpkg.com/en/docs/package-json#toc-resolutions

I’m not sure if it handles “missing” deps, but if not, we could also add additionalResolutions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pnpm/pnpm/issues/949#issuecomment-349259345, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1pm-UliDEwRnZFYjHT4JthOlBUbFjqks5s9Rg_gaJpZM4QxQEP .

zkochan on Jun 9, 2018

yes, we can support something like that but a lot of people asked for a non-executable way of declaring resolutions, so it will have to be something like a pnpmResolutions field in package.json, see #1082

zkochan on Dec 6, 2018

By “global”, do you mean external to the Git repo folder? That would not be a very good approach for us. We have worked very hard to make our developer experience completely deterministic. This avoids problems where a person complains about a build error, but when I try it on my laptop, everything works fine for me. Impossible to investigate without access to the person’s computer, very annoying! Deterministic builds also make it easy to build an old branch from months ago, without running into conflicts with upgraded tools or environmental changes.

Currently we do the following things (via the Rush tool):

Check the NodeJS version and report an error if it’s not in a small range of stable LTS versions
Locally install a specific version of NPM (since different NPM versions have bugs or behavioral differences)
Configure NPM to use a local cache/temp folders, to avoid any influences from other simultaneous builds on the same machine
The NPM registry URL is determined by a local .npmrc committed to Git
All dependencies versions are determined by a shrinkwrap file that is committed to Git
Perform the build without relying on any globally installed packages; the PATH environment variable finds all tools in the local node_modules folder controlled by the shrinkwrap file

This approach also minimizes the amount of setup a person has to do before they can build. In the past, we had to give detailed instructions (install gulp, make sure your NPM is the right version, etc). Today pretty much the only setup step is to add your registry credentials to .npmrc. Everything else is locked down. Even when people use Mac/Linux/Windows OS’s, the results are very consistent.

From this perspective, it wouldn’t make sense to see something like “/home” in a config file. 😃

octogonz on Dec 6, 2017

-1 for global pnpmfile.js. What if it a fix works for one project and not in another project. There is so much craziness involved in resolving that anything is possible with breakages. And it is easy to forget about a global pnpmfile.js. Anything I add to it, I would have to check everytime something goes wrong with resolving.

IMHO the obvious solution is a resolutions field in the package.json for the common-case of hoisted deps that are missing from package.json.

Why can’t we just have both working side-by-side? So anything added to resolutions would be as if you added a hook.

vjpr on Dec 6, 2017