search

stefanpenner / hash-for-dep

7 stars 11 forks source link

Compatibility with Yarn PNP #62

Open stephtr opened 4 years ago

stephtr commented 4 years ago

I'm currently using this package via Broccoli. However it fails in my case by calling hashForDep('.yarn\cache\broccoli-babel-transpiler-npm-7.6.0-0caf4090d4-715d71cfe7.zip\node_modules\broccoli-babel-transpiler'):

.yarn\cache\resolve-package-path-npm-1.2.7-dae74e46b0-5467d60f85.zip\node_modules\resolve-package-path\index.js:58
                    throw e;
                    ^

TypeError: Cannot read property 'match' of undefined
    at Object.toPortablePath (.pnp.js:31159:9)
    at Object.resolveToUnqualified (.pnp.js:35642:54)
    at resolvePackagePath (.yarn\cache\resolve-package-path-npm-1.2.7-dae74e46b0-5467d60f85.zip\node_modules\resolve-package-path\index.js:48:23)
    at Function.ModuleEntry.locate (.yarn\cache\hash-for-dep-npm-1.5.1-cd643a5817-e1c64acc43.zip\node_modules\hash-for-dep\lib\module-entry.js:163:18)
    at hashForDep (.yarn\cache\hash-for-dep-npm-1.5.1-cd643a5817-e1c64acc43.zip\node_modules\hash-for-dep\index.js:66:35)

(It fails at pnp.resolveToUnqualified(target + '/package.json', basedir)). Is this command supposed to work for this argument, is this a bug of Yarn, this package or the broccoli plugin?

rwjblue commented 4 years ago

Hmm, can you whip up a demo that we could use to debug?

stephtr commented 4 years ago

I have created a quick repro: hashfordep-yarn-pnp.zip (probably needs Yarn 1.22+).

It basically consists of a default yarn PNP setup and the following test file (which can be run via yarn test):

const hashForDep = require('hash-for-dep');

hashForDep('hash-for-dep'); // resolves to .yarn\cache\hash-for-dep-npm-1.5.1-cd643a5817-e1c64acc43.zip\node_modules\hash-for-dep\...

According to a quick debug the issue is that for hashForDep the dir parameter is optional, but for resolve-package-path or at least for running pnp.resolveToUnqualified it is required.

rwjblue commented 4 years ago

Hmm. OK, I wonder if there is a reasonable default value for dir that we can use 🤔

stephtr commented 4 years ago

Either process.cwd() (even though there are a few scenarios where this wouldn't work, for example yarn workspaces) or using __dirname and traversing up until one reaches the .yarn or node_modules parent folder, which also doesn't feel great 🤔.