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
When using pnpm
, a module that uses Buffer.isBuffer
fails to bundle
#2063
Comments
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. |
Hi @ljharb, thanks for mentioning it! About opening it on Also, we had a similar issue in the past which was open there and solved in pnpm/pnpm#795 I ended up deciding to open it here because I saw an issue that involved the same package ( #1825. Also, since it has something to do with patching One more thing to add, if I change the call in 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. |
Context
Hi browserify friends! I'm migrating a monorepo to use
pnpm
instead ofyarn
. The monorepo has afrontend
package which is bundled withbrowserify
. After migrating fromyarn
, the application started to throw an error when bundling.The error happened when attempting to bundle
filestack-js
:Investigation
The error seems to be related to the
is-buffer
package, however, it wasn't even a dependency forfilestack-js
. After some time debugging, it seemed to be related with a call that it does toBuffer.isBuffer
. This led me to theinsert-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 tois-buffer
.To prove that, if I go to the
filestack-js
directory: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
withinsertGlobalVars
.I'm not sure if this should be a
browserify/browserify
issue or abrowserify/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.
The text was updated successfully, but these errors were encountered: