Skip to content
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

Get autodoc working for kentcdodds/genie #39

Open
dtao opened this issue Jan 4, 2014 · 15 comments
Open

Get autodoc working for kentcdodds/genie #39

dtao opened this issue Jan 4, 2014 · 15 comments

Comments

@dtao
Copy link
Owner

dtao commented Jan 4, 2014

See https://gist.github.com/kentcdodds/8260581 to understand the problem. /cc @kentcdodds

@kentcdodds
Copy link
Contributor

Thanks! If there's anything I can do to contribute, let me know :) I'll definitely keep you up to date with how I use it. Hopefully I can help this project move along. I think it's awesome.

dtao added a commit that referenced this issue Jan 4, 2014
@kentcdodds
Copy link
Contributor

That change fixed the example. Now I am having this error:

 'Unable to format type ' + type.type + '!\n\n' + JSON.stringify(
                                                             ^
Unable to format type ArrayType!

{
  "type": "ArrayType",
  "elements": []
}

I'm guessing this is related to one of my @param tags correct? I believe they are all valid, but I could be wrong...

@dtao
Copy link
Owner Author

dtao commented Jan 5, 2014

You are correct that it's almost certainly related to a param tag. You
could very well be correct that all of your tags are fine as well! I'd
wager it's probably autodoc's fault. I can take a look tonight or tomorrow,
or if you want to take a stab with a PR it looks pretty clear where the
issue is.
On Jan 4, 2014 8:37 PM, "Kent C. Dodds" notifications@github.com wrote:

That change fixed the example. Now I am having this error:

'Unable to format type ' + type.type + '!\n\n' + JSON.stringify(
^
Unable to format type ArrayType!

{
"type": "ArrayType",
"elements": []
}

I'm guessing this is related to one of my @param tags correct? I believe
they are all valid, but I could be wrong...


Reply to this email directly or view it on GitHubhttps://github.com//issues/39#issuecomment-31596247
.

kentcdodds pushed a commit to kentcdodds/autodoc that referenced this issue Jan 5, 2014
@kentcdodds
Copy link
Contributor

The pull request I sent you fixed the error. Not sure if it's fixed correctly however...

Now I'm getting an error about one of my functions not being defined. I'm looking into it, but the function it is saying is undefined is in the same scope, just a few lines down I think. And it's not just that one that's getting an undefined error.

@kentcdodds
Copy link
Contributor

Here's the definition/implementation of one of the functions giving the trouble:

  /**
   * Iterates through each own property of obj and calls the fn on it.
   *   If obj is an array: fn(val, index, obj)
   *   If obj is an obj: fn(val, propName, obj)
   * @param obj
   * @param fn
   * @private
   */
  function _each(obj, fn, reverse) {
    if (_isPrimitive(obj)) {
      obj = _arrayify(obj);
    }
    if (_isArray(obj)) {
      return _eachArray(obj, fn, reverse);
    } else {
      return _eachProperty(obj, fn);
    }
  }

It is a private function and I've labeled is as such, however it's not getting into exampleJs here:

var examplesJs = _Autodoc.generate(libraryInfo, options);

in the runTests() method of /bin/autodoc for some reason. Not sure why, I wonder if maybe my definition is incorrect... I have a few other functions which have this same issue...

@kentcdodds
Copy link
Contributor

Made an autodoc branch on the genie repo. If you need to take a look at what I'm doing, check it out here

@dtao
Copy link
Owner Author

dtao commented Jan 6, 2014

I took a look earlier today. The issue is with @param tags without any type. I'm not sure if this is legal in JsDoc or not; either way, I don't see any reason why it shouldn't be allowed in Autodoc (makes sense to just default to the * type in this case). It's messing up doctrine.

I will push a fix tomorrow morning. Probably need to patch doctrine and temporarily depend on my fork before they merge my PR and publish a new version. In the meantime you might try adding types to all of your @param tags, just to see what happens next!

Thanks for sticking with it, by the way. These are the joys of the early adopter ;)

@kentcdodds
Copy link
Contributor

I know how helpful it is to have a user of your library to find issues, so
in happy to help in that way. I'm pretty passionate about this (autodoc)
library anyway. I hope it gets wide spread adoption. I think it will really
simplify things for developers and encourage test coverage. Thanks for
being so responsive.

By the way, I discovered this library when I listened to you talk about
lazy on jsjabber. Pretty cool stuff. In really glad you split autodoc into
a genetic repo.

  • Kent C. Dodds
    Sent from my mobile device, please forgive any errors or brevity. (Chances
    are I used speech to text...)
    On Jan 5, 2014 9:36 PM, "Dan Tao" notifications@github.com wrote:

I took a look earlier today. The issue is with @param tags without any
type. I'm not sure if this is legal in JsDoc or not; either way, I don't
see any reason why it shouldn't be allowed in Autodoc (makes sense to
just default to the * type in this case). It's messing up doctrine.

I will push a fix tomorrow morning. Probably need to patch doctrine and
temporarily depend on my fork before they merge my PR and publish a new
version. In the meantime you might try adding types to all of your @paramtags, just to see what happens next!

Thanks for sticking with it, by the way. These are the joys of the early
adopter ;)


Reply to this email directly or view it on GitHubhttps://github.com//issues/39#issuecomment-31627035
.

@kentcdodds
Copy link
Contributor

So I have a few tests working! Woot! However, when I run autodoc genie.js the webpage that's output is empty... Any idea what that could be about?

Also, I have another function that's causing me some trouble and I'm not certain what's wrong. I added a console.log for templateData before it's rendered by the template engine, as well as another for exampleJs (which, if I'm not mistaken is the result of the template engine). Here's the output for the related function when I run autodoc -t --verbose genie.js > out.js (too long to post everything)

{ namespace: '_contextIsDefault',
   constructorMethod: [Object],
   members: [],
   privateMembers: [],
   allMembers: [Object],
   hasExamples: false,
   hasBenchmarks: false,
   excludeFromDocs: true },

//... removed for brevity

 { name: '_contextIsDefault',
   shortName: '_contextIsDefault',
   longName: '_contextIsDefault',
   identifier: '_contextIsDefault',
   namespace: null,
   description: '<p>Returns true if the given context is the default context.</p>\n',
   params: [],
   returns: null,
   isConstructor: false,
   isGlobal: false,
   isStatic: true,
   isPublic: false,
   isPrivate: false,
   hasSignature: false,
   signature: 'function _contextIsDefault() { /*...*/ }',
   highlightedSignature: '<span class="line" data-line-no="0"><span class="cm-keyword">function</span> <span class="cm-variable">_contextIsDefault</span>() { <span class="cm-comment">/*...*/</span> }</span>',
   examples: [Object],
   hasExamples: false,
   benchmarks: [Object],
   hasBenchmarks: false,
   tags: [],
   source: 'function _contextIsDefault(context) {\n    if (!_isObject(context)) {\n      context = _arrayify(context);\n    }\n    if (_isArray(context) && context.length === 1) {\n      return context[0] === _defaultContext[0];\n    } else if (context.any && context.any.length === 1) {\n      return context.any[0] === _defaultContext[0];\n    } else {\n      return false;\n    }\n  }',
   sectionType: 'method' },

//... removed for brevity

global['[private]'] = require('/Users/kentcdodds/Developer/genie/genie.js');

var jasmine  = require('jasmine-node'),
    env      = jasmine.getEnv();

var reporterType = _commander.verbose ?
  jasmine.TerminalVerboseReporter :
  jasmine.TerminalReporter;

env.addReporter(new reporterType({
  print: process.stdout.write.bind(process.stdout),
  color: true,
  includeStackTrace: true
}));

//... removed for brevity

var _wishCanBeMade = function _wishCanBeMade(wish) {
    return !!wish && !_isNullOrUndefined(wish.action) && _wishInContext(wish);
  };
var _wishInContext = function _wishInContext(wish) {
    return _contextIsDefault(_context) ||
      _contextIsDefault(wish.context) ||
      wish.context === _context ||
      _wishInThisContext(wish, _context);
  };

describe('[private]', function() {

//... removed for brevity

describe('_wishCanBeMade', function() {

  (function() {
    var spec = it('_wishCanBeMade(null) => false', function() {
      var result   = _wishCanBeMade(null);
      var expected = false;
      assertEquality(expected, result);
    });

    spec.suiteId    = '_wishCanBeMade';
    spec.exampleId  = '_wishCanBeMade-1';
    spec.actual     = '_wishCanBeMade(null)';
    spec.expected   = 'false';
    spec.lineNumber = 0;
  }());
  (function() {
    var spec = it('_wishCanBeMade(undefined) => false', function() {
      var result   = _wishCanBeMade(undefined);
      var expected = false;
      assertEquality(expected, result);
    });

    spec.suiteId    = '_wishCanBeMade';
    spec.exampleId  = '_wishCanBeMade-2';
    spec.actual     = '_wishCanBeMade(undefined)';
    spec.expected   = 'false';
    spec.lineNumber = 1;
  }());
  (function() {
    var spec = it('_wishCanBeMade({}) => false', function() {
      var result   = _wishCanBeMade({});
      var expected = false;
      assertEquality(expected, result);
    });

    spec.suiteId    = '_wishCanBeMade';
    spec.exampleId  = '_wishCanBeMade-3';
    spec.actual     = '_wishCanBeMade({})';
    spec.expected   = 'false';
    spec.lineNumber = 2;
  }());
  (function() {
    var spec = it('_wishCanBeMade({action: function(){}}) => false', function() {
      var result   = _wishCanBeMade({action: function(){}});
      var expected = false;
      assertEquality(expected, result);
    });

    spec.suiteId    = '_wishCanBeMade';
    spec.exampleId  = '_wishCanBeMade-4';
    spec.actual     = '_wishCanBeMade({action: function(){}})';
    spec.expected   = 'false';
    spec.lineNumber = 3;
  }());
  (function() {
    var spec = it('_wishCanBeMade({action: function(){}, context: {any:\'universe\'}}) => true', function() {
      var result   = _wishCanBeMade({action: function(){}, context: {any:'universe'}});
      var expected = true;
      assertEquality(expected, result);
    });

    spec.suiteId    = '_wishCanBeMade';
    spec.exampleId  = '_wishCanBeMade-5';
    spec.actual     = '_wishCanBeMade({action: function(){}, context: {any:\'universe\'}})';
    spec.expected   = 'true';
    spec.lineNumber = 4;
  }());
});
});

env.execute();


[private]

    _wishCanBeMade
[32m        _wishCanBeMade(null) => false�[0m
[32m        _wishCanBeMade(undefined) => false�[0m
[32m        _wishCanBeMade({}) => false�[0m
[31m        _wishCanBeMade({action: function(){}}) => false�[0m
[31m        _wishCanBeMade({action: function(){}, context: {any:'universe'}}) => true�[0m

Failures:

  1) [private] _wishCanBeMade _wishCanBeMade({action: function(){}}) => false
   Message:
     [31mReferenceError: _contextIsDefault is not defined�[0m
   Stacktrace:
     ReferenceError: _contextIsDefault is not defined
    at _wishInContext (eval at runTests (/Users/kentcdodds/Developer/genie/node_modules/autodoc/bin/autodoc:202:8), <anonymous>:447:5)
    at _wishCanBeMade (eval at runTests (/Users/kentcdodds/Developer/genie/node_modules/autodoc/bin/autodoc:202:8), <anonymous>:444:58)
    at eval (eval at runTests (/Users/kentcdodds/Developer/genie/node_modules/autodoc/bin/autodoc:202:8), <anonymous>:883:22)
    at jasmine.Block.execute (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:1064:17)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2096:31)
    at jasmine.Queue.start (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2049:8)
    at jasmine.Spec.execute (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2376:14)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2096:31)
    at jasmine.Queue.start (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2049:8)
    at jasmine.Suite.execute (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2521:14)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2096:31)
    at onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2092:18)
    at jasmine.Suite.finish (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2478:5)
    at null.onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2522:10)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2106:14)
    at onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2092:18)
    at jasmine.Spec.finish (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2350:5)
    at null.onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2377:10)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2106:14)
    at null._onTimeout (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2086:18)
    at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

  2) [private] _wishCanBeMade _wishCanBeMade({action: function(){}, context: {any:'universe'}}) => true
   Message:
     [31mReferenceError: _contextIsDefault is not defined�[0m
   Stacktrace:
     ReferenceError: _contextIsDefault is not defined
    at _wishInContext (eval at runTests (/Users/kentcdodds/Developer/genie/node_modules/autodoc/bin/autodoc:202:8), <anonymous>:447:5)
    at _wishCanBeMade (eval at runTests (/Users/kentcdodds/Developer/genie/node_modules/autodoc/bin/autodoc:202:8), <anonymous>:444:58)
    at eval (eval at runTests (/Users/kentcdodds/Developer/genie/node_modules/autodoc/bin/autodoc:202:8), <anonymous>:896:22)
    at jasmine.Block.execute (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:1064:17)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2096:31)
    at jasmine.Queue.start (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2049:8)
    at jasmine.Spec.execute (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2376:14)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2096:31)
    at jasmine.Queue.start (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2049:8)
    at jasmine.Suite.execute (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2521:14)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2096:31)
    at onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2092:18)
    at jasmine.Suite.finish (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2478:5)
    at null.onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2522:10)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2106:14)
    at onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2092:18)
    at jasmine.Spec.finish (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2350:5)
    at null.onComplete (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2377:10)
    at jasmine.Queue.next_ (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2106:14)
    at null._onTimeout (/Users/kentcdodds/Developer/genie/node_modules/autodoc/node_modules/jasmine-node/lib/jasmine-node/jasmine-1.3.1.js:2086:18)
    at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

Finished in 0.012 seconds
�[31m33 tests, 33 assertions, 2 failures, 0 skipped
�[0m

@kentcdodds
Copy link
Contributor

I pushed a bunch of changes to my autodoc branch. I'd love it if you could give me some pointers on what I'm doing wrong to make the tests fail (_contextIsDefault is not defined) and the docs not generate...

@dtao
Copy link
Owner Author

dtao commented Jan 6, 2014

I will take a look!

@dtao
Copy link
Owner Author

dtao commented Jan 6, 2014

OK, couple of things. First one not so major, second one kinda major!

First: doctrine is failing to parse the comment for _contextIsDefault because of the type string[]. For arrays doctrine only likes the format Array.<T>; it doesn't like T[]. (I will look into fixing this later on, and submitting another PR to doctrine. Earlier this morning I submitted a PR for the param w/o type issue; see eslint/doctrine#16.)

Now for the second issue. You've come across a pretty significant limitation to autodoc's handling of @private methods at the moment: as you've probably seen from reading the code, it tests them by magically hoisting their code all the way to the global level in the generated JS. This is a hack, but it works beautifully for self-contained methods.

Unfortunately, for methods that are private and rely on state that's in scope but defined outside the method, this approach doesn't work. Even after making the trivial change string[] => Array.<string> for your _contextIsDefault method, I get an error because _defaultContext isn't defined at the global level. So the magic hoisting trick fails.

I do have a plan to address this, but it will take some work. But I think it's important, because without this Autodoc breaks its promise of allowing you to easily test internal members. Anyway, the plan is to use a tool such as Constellation/escodegen to actually insert tests for private methods in the same scope as the private methods themselves, so that the test has exactly the same scope as the method.

Like I said, this will take some work. So realistically, it will probably be a while until this is working for you :( Regardless, I'll keep you abreast of my progress on this front.

@kentcdodds
Copy link
Contributor

Very interesting. That makes sense. Well, I think that I'll merge the autodoc branch in. The comments are useful anyway. I look forward to when this is working. Let me know if there's anything I can do to help with making this work.

As a side, I'm a padawan developer. Only been doing JavaScript for about a year. I imagine you didn't look into my code a ton, but I would absolutely love feedback from an experienced dev. If you noticed any anti-patterns or anything you thought was a little wonky I would really appreciate it if you let me know. Thanks!

@dtao
Copy link
Owner Author

dtao commented Jan 6, 2014

FYI, w/ the latest versions of Lazy (0.3.1 has a bug 😓) and doctrine (from my fork), the docs are getting generated OK-ish:

geniedocsv1

Also worth noting is that you actually have some legitimate failures that Autodoc spotted :)

geniefails

@kentcdodds
Copy link
Contributor

Sweet! I'll look into bringing the latest in and get those docs up. That'll be really awesome to at least have the docs working and up. Thanks for all your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants