go-ipfs is required from the top-level module

[mappum]

When the ipfsd-ctl package is installed and required by another module, the following error is given:

/Users/mappum/Projects/inpm/node_modules/ipfsd-ctl/index.js:13
var IPFS_EXEC = path.join(requireResolve('go-ipfs').pkg.root, '/bin/ipfs')
                                                   ^

TypeError: Cannot read property 'pkg' of null
    at Object.<anonymous> (/Users/mappum/Projects/inpm/node_modules/ipfsd-ctl/index.js:13:52)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/mappum/Projects/inpm/bin/main.js:11:16)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)

This happens because the go-ipfs dependency gets loaded as a direct dependency of the top-level module, not as a dependency of ipfsd-ctl. So even though npm install installs go-ipfs in node_modules/ipfsd-ctl/node_modules/go-ipfs, the ipfsd-ctl code tries to require it from node_modules/go-ipfs.

If this is actually the intended behavior, the README should tell the user to also install go-ipfs, and go-ipfs should be a peer dependency rather than a direct dependency.

[bcomnes]

What happens when we swap out requireResolve for https://github.com/substack/node-resolve ?

[mappum]

What happens when we swap out requireResolve for https://github.com/substack/node-resolve ?

It seems require-resolve is being used because it returns an object containing the root path. resolve seems to only return the path of the index script (the same as Node's built-in require.resolve().

[bcomnes]

Hrmm...

My only concern is that peerDependencies no longer install with npm3.

Could go-ipfs solve this issue by exporting a path.resolve(__dirname + '/bin/ipfs') ?

[bcomnes]

It looks like this was already done, but kinda broken maybe? ipfs/npm-kubo#2 I could be totally off though!

[jbenet]

maybe try now? published new: go-ipfs@0.3.7-3 nvm, i think this problem is something else?

[bcomnes]

Which module is this occurring in? (and with what version of npm?)

[mappum]

I think this is just a problem with require-resolve. It is probably better to just get the go-ipfs path by doing something like path.resolve(module.paths[0], 'go-ipfs').

[bcomnes]

@mappum did the upstream npm-go-ipfs changes I put in fix this issue?

[bcomnes]

It looks like this is still grabbing the older version of npm-go-ipfs. @jbenet How come you didn't release that update as a patch? (also what is with the dash in the version of npm-go-ipfs? It seems to be breaking range prefixes e.g. ^)

[bcomnes]

I think once the upstream changes are pulled it we should be good to go. Tested on npm2 and 3: #21

[mappum]

Woo! Thanks @bcomnes

[jbenet]

@bcomnes i want the version of the "go-ipfs npm module" to match the exact version of go-ipfs, so what i'm doing is using a hack like <go-ipfs-version>-<npm-module-patch> and this works because the compare should work, and because (to make sure) i just unpublish the older one, so when a user asks for npm install go-ipfs@0.3.8 they get the latest of those.

ideally we would never have to patch it, but it's there because of updates to the "go-ipfs module" that come without a go-ipfs update. make sense?

[mappum]

@jbenet We could automate that using the preversion script in package.json.

That is actually pretty useful, I've had that problem when maintaining forks of modules and not wanting the versions to diverge. We could make a module that contains the script and that automatically adds itself to package.json, then you can just bump with npm version 0.1.2 (0.1.2) ... npm version 0.1.2 (0.1.2-1).

[bcomnes]

I figured there was a reason, just wasn't sure. Thanks for the explanation! Also just realized modules below 1.0.0 (eg 0.5.0) disable npm semver range updates as well: http://semver.npmjs.com/

[jbenet]

@mappum SGTM. want to PR it up to npm-go-ipfs ?