Make pre-commit compatible with pnpm

[etamponi]

Hello :)

Lately I am contributing a lot to pnpm which is an alternative package manager for Node with tons of interesting features (and it's also very fast).

One of its peculiarity is the node_modules structure it creates. Unfortunately, pre-commit expects node_modules to be handled by npm, so it calculates the root variable in install.js as just __dirname + '/../..'.

Would it be possible to replace this logic with another one that is compatible with pnpm?

For example: root might be the first directory up the tree in which there is a package.json file.

Thanks in advance!

[jsumners]

As a reference, I do this sort of thing in nixconfig -- https://github.com/jsumners/nixconfig/blob/master/lib/findpkg.js

[3rd-Eden]

Why wouldn't it be compatible tho? Because I don't recall node_modules being a npm specific thing. It's what the node module system assumes to exists. Both yarn and npm use this. What makes pnpm different here?

[jsumners]

@3rd-Eden pnpm actually installs modules to a global store, ~/.pnpm-store. It then uses links, hard links if possible, to build the node_modules directory within the target project. This reduces disk usage across multiple projects. So, when pre-commit gets installed it resolves the path __dirname + '/../..' to ~/.pnpm-store/2/ since the real current working directory is ~/.pnpm-store/2/registry.npmjs.org/pre-commit.

[etamponi]

@3rd-Eden, it's more or less how @jsumners explained but not quite. pnpm uses a global store, but it still hardlinks all the dependencies inside node_modules. The problem is that it doesn't copy them directly to their final destination. Example: pre-commit would end up in node_modules/.registry.npmjs.org/pre-commit/1.2.2/node_modules/pre-commit. Then pnpm makes a symlink from that location to node_modules/pre-commit.

This means that ../../ is node_modules/.registry.npmjs.org/pre-commit/1.2.2.

Going up the tree you can still get to the project directory.

[jsumners]

That makes sense. That you for the clarification.

[jsumners]

Wouldn't changing:

https://github.com/observing/pre-commit/blob/f25888ffc67d21beaa1572b53ab6704a67266b64/install.js#L10

To:

, root = process.cwd()

Solve this problem?

[zkochan]

I suspect it is better to use process.env.INIT_CWD because process.cwd() will have the pre-commit's package location in node_modules, not the dependent project's location.

EDIT:

there was a similar issue with husky. I contributed this fix: https://github.com/typicode/husky/pull/185

but they fixed it by then in the "next" version. I don't know what was the fix but the "next" version of husky works perfectly with pnpm

[3rd-Eden]

@zkochan You're confusing process.cwd() with __dirname where __dirname would be the directory of pre-commit

[zkochan]

@3rd-Eden no, I know it for sure because I was configuring lifecycle scripts in pnpm and I learned how they work with npm

here's the lib that npm and pnpm use for running lifecycle script: https://github.com/npm/lifecycle

wd is what is set as cwd for the scripts when they are executed. wd is the location of the dependency, not the project. process.cwd() is assigned to INIT_CWD https://github.com/npm/lifecycle/blob/latest/index.js#L67

[jsumners]

It is possible to work around this issue with pnpm >= 1.39.0 -- https://github.com/pnpm/pnpm/issues/1055#issuecomment-386803939