When using pnpm, a module that uses Buffer.isBuffer fails to bundle

[wmartins]

Context

Hi browserify friends! I'm migrating a monorepo to use pnpm instead of yarn. The monorepo has a frontend package which is bundled with browserify. After migrating from yarn, the application started to throw an error when bundling.

The error happened when attempting to bundle filestack-js:

Error: Can't walk dependency graph: Cannot find module '../../../../../../node_modules/.pnpm/is-buffer@1.1.6/node_modules/is-buffer/index.js' from '../../node_modules/.pnpm/filestack-js@3.26.1_typescript@5.1.3/node_modules/filestack-js/build/browser/filestack.esm.js'

Investigation

The error seems to be related to the is-buffer package, however, it wasn't even a dependency for filestack-js. After some time debugging, it seemed to be related with a call that it does to Buffer.isBuffer. This led me to the insert-module-globals package:

https://github.com/browserify/insert-module-globals/blob/0549e4372523ec2c967058b0bfce51fad9038b0f/index.js#L38-L41

I believe somehow it is not resolving the path correctly. It may be related to the fact that pnpm symlinks the packages. Maybe there's something incorrect when trying resolve the path to is-buffer.

To prove that, if I go to the filestack-js directory:

cd node_modules/.pnpm/filestack-js@3.26.1_typescript@5.1.3/node_modules/filestack-js/build/browser/

And then, if I try requiring that path on the error, I can confirm the same problem occurs:

node -e "console.log(require('../../../../../../node_modules/.pnpm/is-buffer@1.1.6/node_modules/is-buffer/index.js'))"

However, the actual path should have been one level lower, meaning this works:

node -e "console.log(require('../../../../../../../node_modules/.pnpm/is-buffer@1.1.6/node_modules/is-buffer/index.js'))"

Reproducing

I've put up a repo that can be used to reproduce it: https://github.com/wmartins/browserify-pnpm-investigation.

Current workaround

My current workaround for this is to provide my own implementation of Buffer.isBuffer with insertGlobalVars.

---

I'm not sure if this should be a browserify/browserify issue or a browserify/insert-module-globals issue, but I've decided to add it here. If this is not the right place, I'm happy to move it.

Do you have any idea how to solve this problem? Is there anything else that I can do?

In case there's something missing or any extra information is needed, I'm happy to provide it!

I can also provide a PR, but honestly I don't have much context on how to navigate it and to point exactly where the issue is.

[ljharb]

If something works with npm, and not with an alternative package manager, then it seems likely to be a bug in that package manager. If pnpm, or yarn, or anyone else is placing things in a different location than npm does, then it's responsible for also ensuring that standard module resolution APIs act as if things are in an npm-installed location.

Have you filed an issue on pnpm to investigate? If there's something in browserify then a fix ofc can be investigated, but it seems worth checking with pnpm first.

[wmartins]

Hi @ljharb, thanks for mentioning it! About opening it on pnpm, from what I read yesterday, when an issue is tooling related, it's something that pnpm wont solve, according to this:

pnpm/pnpm#801

Also, we had a similar issue in the past which was open there and solved in browserify/module-deps:

pnpm/pnpm#795
browserify/module-deps#133

I ended up deciding to open it here because I saw an issue that involved the same package (is-buffer), which was solved in browserify/insert-module-globals:

#1825.
browserify/insert-module-globals#75.

Also, since it has something to do with patching Buffer.isBuffer (which seems to be browserify's responsibility) I thought this would be the right place.

One more thing to add, if I change the call in browserify/insert-module-globals from path.relative to path.resolve, it ends up bundling things correctly:

diff --git a/index.js b/index.js
index dfc2664..3c35f03 100644
--- a/index.js
+++ b/index.js
@@ -10,7 +10,7 @@ var isbufferPath = require.resolve('is-buffer')
 var combineSourceMap = require('combine-source-map');
 
 function getRelativeRequirePath(fullPath, fromPath) {
-  var relpath = path.relative(path.dirname(fromPath), fullPath);
+  var relpath = path.resolve(path.dirname(fromPath), fullPath);
   // If fullPath is in the same directory or a subdirectory of fromPath,
   // relpath will result in something like "index.js", "src/abc.js".
   // require() needs "./" prepended to these paths.

However, there's one test that ends up failing because of this: https://github.com/browserify/insert-module-globals/blob/0549e4372523ec2c967058b0bfce51fad9038b0f/test/isbuffer.js#L22.