refactor: remove path and ref from module args also findBin (#458)

Since we're not passing modules to `.spawn()` there's no danger of
passing module refs over http, so we can remove the `.path` and
`.ref` arguments when specifying which `ipfs-http-client` and `ipfs`
modules to use, instead setting them up when you configure the
daemon factory, either in-proc or as a server in an `.aegir` file.

Also removes the `findBin` command magic in favour of being explicit
about which binary you want the factory to use.

If you need to support env vars like `IPFS_GO_EXEC`, do that when
you set up the factory.

BREAKING CHANGES:

- `.path` and `.ref` args removed from `ipfsModule` and `ipfsHttpModule`
- `findBin` function removed

* chore: update dep versions

* chore: increase test timeouts

* fix: only get go-ipfs-dep path in node

* fix: linting

Co-authored-by: Alex Potsides <alex@achingbrain.net>

.aegir.js

@@ -1,23 +1,16 @@
 'use strict'
 
 const createServer = require('./src').createServer
-const { findBin } = require('./src/utils')
 
 const server = createServer(null, {
-  ipfsModule: {
-    path: require.resolve('ipfs'),
-    ref: require('ipfs')
-  },
-  ipfsHttpModule: {
-    path: require.resolve('ipfs-http-client'),
-    ref: require('ipfs-http-client')
-  }
+  ipfsModule: require('ipfs'),
+  ipfsHttpModule: require('ipfs-http-client')
 }, {
   go: {
-    ipfsBin: findBin('go', true)
+    ipfsBin: require('go-ipfs-dep').path()
   },
   js: {
-    ipfsBin: findBin('js', true)
+    ipfsBin: require.resolve('ipfs/src/cli/bin.js')
   }
 }) // using defaults
 module.exports = {

examples/electron-asar/app.js

@@ -6,7 +6,7 @@ const app = electron.app
 const ipcMain = electron.ipcMain
 const BrowserWindow = electron.BrowserWindow
 
-const { createNode, createServer } = require('ipfsd-ctl')
+const { createController, createServer } = require('ipfsd-ctl')
 
 app.on('ready', () => {
   const win = new BrowserWindow({
@@ -24,7 +24,10 @@ ipcMain.on('start', async ({ sender }) => {
   try {
     const s = createServer()
     await s.start()
-    const node = await createNode({ type: 'go' })
+    const node = await createController({
+      type: 'go',
+      ipfsBin: require('go-ipfs-dep').path()
+    })
     console.log('get id')
     sender.send('message', 'get id')
 

examples/electron-asar/package.json

@@ -3,7 +3,7 @@
   "private": true,
   "main": "./app.js",
   "dependencies": {
-    "go-ipfs-dep": "~0.4.22",
+    "go-ipfs-dep": "github:ipfs/npm-go-ipfs-dep#add-path-function-to-detect-binary",
     "ipfs": "^0.39.0-rc.2",
     "ipfsd-ctl": "file:../.."
   },

examples/id/id.js

@@ -1,20 +1,32 @@
 /* eslint no-console: 0 */
 'use strict'
 
-const { createNode } = require('../../src')
+const { createController } = require('../../src')
 
 async function run () {
-  const node = await createNode({ type: 'go' })
+  const node = await createController({
+    type: 'go',
+    ipfsBin: require('go-ipfs-dep').path(),
+    ipfsHttpModule: require('ipfs-http-client')
+  })
   console.log('alice')
   console.log(await node.api.id())
   await node.stop()
 
-  const nodeJs = await createNode({ type: 'js' })
+  const nodeJs = await createController({
+    type: 'js',
+    ipfsBin: require.resolve('ipfs/src/cli/bin.js'),
+    ipfsHttpModule: require('ipfs-http-client')
+  })
   console.log('alice')
   console.log(await nodeJs.api.id())
   await nodeJs.stop()
 
-  const nodeProc = await createNode({ type: 'proc' })
+  const nodeProc = await createController({
+    type: 'proc',
+    ipfsModule: require('ipfs'),
+    ipfsHttpModule: require('ipfs-http-client')
+  })
   console.log('bob')
   console.log(await nodeProc.api.id())
   await nodeProc.stop()

examples/remote-disposable/remote-disposable.js

@@ -4,12 +4,16 @@
 // Start a remote disposable node, and get access to the api
 // print the node id, and stop the temporary daemon
 
-const { createNode, createServer } = require('../../src')
+const { createController, createServer } = require('../../src')
 const server = createServer()
 
 async function run () {
   await server.start()
-  const node = await createNode({ remote: true })
+  const node = await createController({
+    remote: true,
+    type: 'go',
+    ipfsBin: require('go-ipfs-dep').path()
+  })
 
   console.log(await node.api.id())
   await node.stop()

package.json

@@ -62,7 +62,6 @@
     "merge-options": "^2.0.0",
     "multiaddr": "^7.2.1",
     "nanoid": "^2.1.9",
-    "resolve-cwd": "^3.0.0",
     "temp-write": "^4.0.0"
   },
   "devDependencies": {
@@ -71,10 +70,10 @@
     "chai": "^4.2.0",
     "chai-as-promised": "^7.1.1",
     "dirty-chai": "^2.0.1",
-    "go-ipfs-dep": "^0.4.22",
+    "go-ipfs-dep": "github:ipfs/npm-go-ipfs-dep#master",
     "husky": "^4.0.10",
     "ipfs": "^0.40.0",
-    "ipfs-http-client": "^42.0.0-pre.0",
+    "ipfs-http-client": "^42.0.0",
     "lint-staged": "^10.0.2"
   },
   "peerDependencies": {

src/index.js

@@ -82,13 +82,9 @@ module.exports = {
  * - proc - spawn in-process js-ipfs node
  * @property {Object} [env] - Additional environment variables, passed to executing shell. Only applies for Daemon controllers.
  * @property {Array} [args] - Custom cli args.
- * @property {Object} [ipfsHttpModule] - Setup IPFS HTTP client to be used by ctl.
- * @property {Object} [ipfsHttpModule.ref] - Reference to a IPFS HTTP Client object. (defaults to the local require(`ipfs-http-client`))
- * @property {string} [ipfsHttpModule.path] - Path to a IPFS HTTP Client to be required. (defaults to the local require.resolve('ipfs-http-client'))
- * @property {Object} [ipfsModule] - Setup IPFS API to be used by ctl.
- * @property {Object} [ipfsModule.ref] - Reference to a IPFS API object. (defaults to the local require(`ipfs`))
- * @property {string} [ipfsModule.path] - Path to a IPFS API implementation to be required. (defaults to the local require.resolve('ipfs'))
- * @property {String} [ipfsBin] - Path to a IPFS exectutable . (defaults to the local 'js-ipfs/src/bin/cli.js')
+ * @property {Object} [ipfsHttpModule] - Reference to a IPFS HTTP Client object. (defaults to the local require(`ipfs-http-client`))
+ * @property {Object} [ipfsModule] - Reference to a IPFS API object. (defaults to the local require(`ipfs`))
+ * @property {String} [ipfsBin] - Path to a IPFS exectutable
  * @property {IpfsOptions} [ipfsOptions] - Options for the IPFS node.
  * @property {boolean} [forceKill] - Whether to use SIGKILL to quit a daemon that does not stop after `.stop()` is called. (default true)
  * @property {Number} [forceKillTimeout] - How long to wait before force killing a daemon in ms. (default 5000)

src/ipfsd-client.js

@@ -46,7 +46,7 @@ class Client {
   _setApi (addr) {
     if (addr) {
       this.apiAddr = multiaddr(addr)
-      this.api = (this.opts.ipfsHttpModule.ref)(addr)
+      this.api = this.opts.ipfsHttpModule(addr)
       this.api.apiHost = this.apiAddr.nodeAddress().address
       this.api.apiPort = this.apiAddr.nodeAddress().port
     }

src/ipfsd-daemon.js

@@ -38,10 +38,6 @@ class Daemon {
   constructor (opts) {
     /** @type ControllerOptions */
     this.opts = opts
-    // make sure we have real paths
-    this.opts.ipfsHttpModule.path = fs.realpathSync(this.opts.ipfsHttpModule.path)
-    this.opts.ipfsBin = fs.realpathSync(this.opts.ipfsBin)
-
     this.path = this.opts.ipfsOptions.repo || (opts.disposable ? tmpDir(opts.type) : defaultRepo(opts.type))
     this.exec = this.opts.ipfsBin
     this.env = merge({ IPFS_PATH: this.path }, this.opts.env)
@@ -61,7 +57,7 @@ class Daemon {
    */
   _setApi (addr) {
     this.apiAddr = multiaddr(addr)
-    this.api = require(this.opts.ipfsHttpModule.path)(addr)
+    this.api = this.opts.ipfsHttpModule(addr)
     this.api.apiHost = this.apiAddr.nodeAddress().address
     this.api.apiPort = this.apiAddr.nodeAddress().port
   }
@@ -254,7 +250,9 @@ class Daemon {
     try {
       await this.api.stop()
     } catch (err) {
-      if (!killed) throw err // if was killed then ignore error
+      if (!killed) {
+        throw err // if was killed then ignore error
+      }
 
       daemonLog.info('Daemon was force killed')
     }

src/ipfsd-in-proc.js

@@ -36,7 +36,8 @@ class InProc {
       return
     }
 
-    const IPFS = this.opts.ipfsModule.ref
+    const IPFS = this.opts.ipfsModule
+
     this.api = await IPFS.create(merge({
       silent: true,
       repo: this.path
@@ -49,7 +50,7 @@ class InProc {
    */
   _setApi (addr) {
     this.apiAddr = multiaddr(addr)
-    this.api = (this.opts.ipfsHttpModule.ref)(addr)
+    this.api = this.opts.ipfsHttpModule(addr)
     this.api.apiHost = this.apiAddr.nodeAddress().address
     this.api.apiPort = this.apiAddr.nodeAddress().port
   }
@@ -90,6 +91,7 @@ class InProc {
 
     await this.setExec()
     await this.api.init(opts)
+
     this.clean = false
     this.initialized = true
     return this

src/utils.browser.js

@@ -45,9 +45,6 @@ const checkForRunningApi = (path) => {
   return null
 }
 
-const findBin = (type) => {
-}
-
 const tmpDir = (type = '') => {
   return `${type}_ipfs_${nanoid()}`
 }
@@ -57,6 +54,5 @@ module.exports = {
   repoExists,
   defaultRepo,
   checkForRunningApi,
-  findBin,
   tmpDir
 }

src/utils.js

@@ -5,8 +5,6 @@ const path = require('path')
 const fs = require('fs-extra')
 const debug = require('debug')
 const nanoid = require('nanoid')
-const resolveCwd = require('resolve-cwd')
-const isWindows = os.platform() === 'win32'
 
 const log = debug('ipfsd-ctl:utils')
 
@@ -26,7 +24,7 @@ const repoExists = async (repoPath) => {
 const defaultRepo = (type) => {
   return path.join(
     os.homedir(),
-    type === 'js' ? '.jsipfs' : '.ipfs'
+    type === 'js' || type === 'proc' ? '.jsipfs' : '.ipfs'
   )
 }
 
@@ -41,16 +39,6 @@ const checkForRunningApi = (repoPath) => {
   return api ? api.toString() : null
 }
 
-const findBin = (type, required) => {
-  const resolve = required ? resolveCwd : resolveCwd.silent
-
-  if (type === 'js') {
-    return process.env.IPFS_JS_EXEC || resolve('ipfs/src/cli/bin.js')
-  }
-
-  return process.env.IPFS_GO_EXEC || resolve(`go-ipfs-dep/go-ipfs/${isWindows ? 'ipfs.exe' : 'ipfs'}`)
-}
-
 const tmpDir = (type = '') => {
   return path.join(os.tmpdir(), `${type}_ipfs_${nanoid()}`)
 }
@@ -60,6 +48,5 @@ module.exports = {
   repoExists,
   defaultRepo,
   checkForRunningApi,
-  findBin,
   tmpDir
 }

test/browser.utils.js

@@ -8,15 +8,11 @@ const expect = chai.expect
 chai.use(dirtyChai)
 
 const { isEnvWithDom } = require('ipfs-utils/src/env')
-const { findBin, tmpDir, checkForRunningApi, defaultRepo, repoExists, removeRepo } = require('../src/utils')
+const { tmpDir, checkForRunningApi, defaultRepo, repoExists, removeRepo } = require('../src/utils')
 const { createFactory, createController } = require('../src')
 
 describe('utils browser version', function () {
   if (isEnvWithDom) {
-    it('findBin should return undefined', () => {
-      expect(findBin()).to.be.undefined()
-    })
-
     it('tmpDir should return correct path', () => {
       expect(tmpDir('js')).to.be.contain('js_ipfs_')
       expect(tmpDir('go')).to.be.contain('go_ipfs_')
@@ -37,10 +33,7 @@ describe('utils browser version', function () {
         type: 'proc',
         disposable: false,
         ipfsOptions: { repo: 'ipfs_test_remove' },
-        ipfsModule: {
-          path: require.resolve('ipfs'),
-          ref: require('ipfs')
-        }
+        ipfsModule: require('ipfs')
       })
       await ctl.init()
       await ctl.start()
@@ -60,10 +53,7 @@ describe('utils browser version', function () {
           ipfsOptions: {
             repo: 'ipfs_test'
           },
-          ipfsModule: {
-            path: require.resolve('ipfs'),
-            ref: require('ipfs')
-          }
+          ipfsModule: require('ipfs')
         })
         expect(await repoExists('ipfs_test')).to.be.true()
         await node.stop()