this.visit is not a function (latest 0.8.4)

[tunnckoCore]

Strange but L24 is empty, but yea, it is L29 currently.

/home/charlike/dev/useware/node_modules/base-options/index.js:24
    this.visit('define', Options.prototype);
         ^

TypeError: this.visit is not a function
    at Object.fn (/home/charlike/dev/useware/node_modules/base-options/index.js:24:10)
    at Object.use (/home/charlike/dev/useware/node_modules/base-plugins/index.js:95:18)
    at Docks.<anonymous> (/home/charlike/dev/useware/node_modules/base-plugins/index.js:70:13)
    at Object.<anonymous> (/home/charlike/dev/useware/test.js:88:7)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)

That's strange, cuz there's visit method on base.

[jonschlinkert]

Might not be strange, it sounds like a plugin is being called on an object that isn't an instance of base. can you share more of your config?

[tunnckoCore]

Sounds, but not or I can't see it here

Docks.prototype.initDocks = function initApp () {
  this.use(require('base-plugins')())
  this.use(function extractComments (app) {
    this.define('parse', function parse (input, options) {
      if (typeof input === 'object') {
        this.options = extend(this.options, input)
        input = null
      }

      this.cache = extend({}, this.cache)
      this.cache.input = input || this.input
      this.options = extend({
        preserve: false,
        block: true,
        line: false
      }, this.options, options, {
        locations: true,
        unwrap: true
      })

      this.cache.comments = extract(this.cache.input, this.options).comments
      var len = this.cache.comments.length
      var idx = -1

      while (++idx < len) {
        // not so cool workaround,
        // but we can't pass different than object currently
        // waiting `use` PR
        this.run({
          current: this.cache.comments[idx],
          index: idx,
          next: this.cache.comments[idx + 1]
        })
      }

      return this.cache.comments
    })
  })
  return this
}

[jonschlinkert]

try doing this:

Docks.prototype.initDocks = function initApp () {
  console.log(this.isApp)
  this.use(require('base-plugins')())
  this.use(function extractComments (app) {
    console.log(this.isApp)

[tunnckoCore]

It is undefined two times.

[jonschlinkert]

Try doing the same with isBase. See if you can figure out what the object is exactly. base-options doesn't have any special requirements for the "app" (like isApp). Just to see if it works, inside the Docks ctor try doing this.isApp = true

[tunnckoCore]

isBase both are true, adding isApp to Docks not work.

Actually, what's the case of these isApp, isVerb and the is-valid-app and is-registered, in short. Because currently, testing that plugin (without the base-options, b/c realized that i don't need it), but can't get it work if i set !isValid(app, 'plugin-name') return or isRegistered(app, 'plugin-name') return - without that check it works and adds the given method.

[jonschlinkert]

what's the case of these isApp, isVerb and the is-valid-app and is-registered, in short

• is-registered: prevents memory leaks. We don't want the plugin to be registered more than once on the same object, so isRegistered keeps things fast and prevents humans from accidentally re-adding plugins to the stack, potentially in a cyclically-redundant way
• isApp is an arbitrary property that I like to use in certain plugins as a way of ensuring that plugins are only run against "whitelisted" instances. For example, base-fs should not be registered on an actual view object, but in theory it could be if you thought it might be useful to do app.run(view), to "do something" to the view. If the view happens to have a .use method (which it does in our ecosystem), it would try to run it's own view.use() method on the function. edit: which means that the plugin would keep going down the rabbit hole getting applied against instances it wasn't intended to run against. But more importantly, these objects won't have other plugins that might be necessary for your plugin to work. (I can show you tricks to make this easier too)

If this doesn't make sense, it's not you. it's hard to explain in words. But trust me it's worth the effort, I'd be happy to show other examples if it would help.

Also, in your code, you should be able to move the .parse method out of the plugin. That shouldn't be needed. I would consider shallow cloning those options inside .parse as well (meaning extend({}, this.options, ...) instead of extend(this.options, ...)), or you might end up with a stack overflow.

Docks.prototype.initDocks = function initApp () {
  this.use(require('base-plugins')())
  this.define('parse', function parse (input, options) {
    if (typeof input === 'object') {
      this.options = extend(this.options, input)
      input = null
    }

    this.cache = extend({}, this.cache)
    this.cache.input = input || this.input
    this.options = extend({
      preserve: false,
      block: true,
      line: false
    }, this.options, options, {
      locations: true,
      unwrap: true
    })

    this.cache.comments = extract(this.cache.input, this.options).comments
    var len = this.cache.comments.length
    var idx = -1

    while (++idx < len) {
      // not so cool workaround,
      // but we can't pass different than object currently
      // waiting `use` PR
      this.run({
        current: this.cache.comments[idx],
        index: idx,
        next: this.cache.comments[idx + 1]
      })
    }
    return this.cache.comments
  })
  return this
}

Hmm, I also see why you want to use .run() there, but there might be a better way to approach that. .run() might not be the best choice b/c of what I explained.

If the .isApp stuff didn't make complete sense, let me know. I think once I show examples you will agree that it's a good approach. If not, then that's ok too, you might have an idea of how to do it better.

[jonschlinkert]

ok so here is an example that might explain better in visuals (I can add to docs if you think this helps):

function appPlugin(app) {
  if (!isValid(app, 'plugin-name', ['app', 'collection'])) return;
  //                      ^             ^
  //               isRegistered?  |  checks `isApp` and `isCollection`, e.g. is "this" a 
  //                              |  valid "app" or "collection" instance?

}

function viewPlugin(view) {
  if (!isValid(view, 'plugin-name', ['view'])) return;
  //                      ^             ^
  //               isRegistered?  |  checks `isView`, e.g. is "this" a valid "view" instance?

}

[tunnckoCore]

So, it seems i understand it correctly and it make sense, yea, definitely.

Yea, I can define it directly with .define but I need to be a plugin. Thinking to create 3-4 plugins (one for base-extract-comments - the above, one for doctrine modifying, other for source code, and etc) and I need to pass comment to each plugin. I don't see how this can be done without .run.

I can try to push something to repo in github this night.
If i can't that's the base-extract-comments plugin

index.js

var utils = require('./utils')

module.exports = function baseExtractComments (opts) {
  return function extractComments (app) {
    // i tried both is-valid-app and is-registered here as `isValid`
    if (!utils.isValid(app, 'base-extract-comments')) return

    this.use(utils.plugins())
    this.define('extractComments', function parse (input, options) {
      if (typeof input === 'object') {
        this.options = utils.extend({}, this.options, input)
        input = null
      }

      this.cache = utils.extend({}, this.cache)
      this.cache.input = input || this.cache.input
      this.options = utils.extend({
        preserve: false,
        block: true,
        line: false
      }, opts, this.options, options, {
        locations: true,
        unwrap: true
      })

      this.cache.comments = utils.extract(this.cache.input, this.options).comments
      var len = this.cache.comments.length
      var idx = -1

      while (++idx < len) {
        // not so cool workaround,
        // but we can't pass different than object currently
        // waiting `use` PR
        this.run({
          current: this.cache.comments[idx],
          index: idx,
          next: this.cache.comments[idx + 1]
        })
      }

      return this.cache.comments
    })
  }
}

and the test.js

var  Base = require('base')
var plugin = require('./index')
var app = new Base()

app.use(plugin())
app.use(function (app) {
  return function (comment) {
    comment = comment.current // workaround
    console.log('comment:', comment)
  }
})
// throws error if use `isValid` (no matter if it is `is-valid-app` or `is-registered`)
// if comment out the first if in plugin it works
app.extractComments(fs.readFileSync('./fixture.js', 'utf8'))

would consider shallow cloning those options inside .parse as well

Yea make sense.