JsDoc improvements #854

janhancic · 2014-07-18T10:36:55Z

When creating a link to a module: {@link module:my/Module}, the default text of the link should be my/Module, not module:my/Module.

Same for return types and argument types.

The text was updated successfully, but these errors were encountered:

janhancic · 2014-07-18T10:37:12Z

Also, move the namespace/package list to the top.

janhancic · 2014-07-18T12:58:38Z

@andyberry88 I don't know if I understood your correctly, so correct me if this is wrong.

If you link to a CommonJS module, you do it like so: module:some/Module, whereas if you link to a namespaced class you link it like so: some.Class. And you can't use the dotted notation to link to a module and vice-versa.

If that is true, then I think we need a workaround that will allow us to do that. Otherwise people will have to constantly update their JsDoc when the thing they are linking to changes from namespaced-js to commonJS (and it is going to be hard to catch when that happens).

andy-berry-dev · 2014-07-29T11:30:33Z

When creating a link to a module: {@ link module:my/Module}, the default text of the link should be my/Module, not module:my/Module.

This has been done and should be in the v0.11 release (dd2b9f5). In order to make sure the change is in the template rather than core JSDoc we now remove any instance of module: in link text. The downside of this is that no link text can start with module: regardless of if you actually want it to.

Also, move the namespace/package list to the top.

Done. Just a template change. (11af58b)

If you link to a CommonJS module, you do it like so: module:some/Module, whereas if you link to a namespaced class you link it like so: some.Class. And you can't use the dotted notation to link to a module and vice-versa.

Correct. I don't think there is any workaround for this and I don't think we should attempt to create one by hacking the JsDoc core. This is simply a side effect of using both commonJS and namespacedJS style code. Ultimately JSDoc needs to be able to identify any symbol, and the module: prefix is part of this unique name.

janhancic · 2014-07-29T12:23:07Z

Correct. I don't think there is any workaround for this and I don't think we should attempt to create one by hacking the JsDoc core. This is simply a side effect of using both commonJS and namespacedJS style code. Ultimately JSDoc needs to be able to identify any symbol, and the module: prefix is part of this unique name.

As per my original comment, I don't think this is acceptable. We had a conversation with @james-shaw-turner and he agreed.

andy-berry-dev · 2014-07-29T12:26:37Z

I think we need to get on a call at some point to discuss this then. IMO, from a BRJS point of view, we shouldn't be changing the core behaviour of tools we rely on. If this is a CT requirement then it can be done as a CT change rather than in the core of BRJS.

janhancic · 2014-07-29T12:30:06Z

This is a user issue. A developer using BRJS or CT that links to documentation in the SDK shouldn't have to care what type it is. And like I said, it will randomly break when libs are updated to CommonJS.

thecapdan · 2014-08-04T16:17:10Z

Following the upgrade to jsdoc:

brjs jsdoc appname

command is taking a very long time. 15 minutes for large apps that previously took around 1 minute.

When testing on a small app in brjs it was difficult to see the issue since it was taking around 20 secs for smaller apps. But when running with bigger apps and more libraries we saw that it was taking a LOT longer.

needs further investigation to see what about the jsdoc toolkit is taking so much longer

thecapdan · 2014-08-05T08:12:20Z

Also, when it fails to produce a jsdoc for a class it doesn't fail gracefully - it just hangs on the command line:


Unicode modifier letter, Unicode other letter, Unicode titlecase letter or Unicode uppercase letter but "'" found.
org.mozilla.javascript.EcmaError: TypeError: Cannot read property "virtual" from undefined
    at org.mozilla.javascript.ScriptRuntime.constructError(ScriptRuntime.java:3689)
    at org.mozilla.javascript.ScriptRuntime.constructError(ScriptRuntime.java:3667)
    at org.mozilla.javascript.ScriptRuntime.typeError(ScriptRuntime.java:3695)
    at org.mozilla.javascript.ScriptRuntime.typeError2(ScriptRuntime.java:3714)
    at org.mozilla.javascript.ScriptRuntime.undefReadError(ScriptRuntime.java:3726)
    at org.mozilla.javascript.ScriptRuntime.getObjectPropNoWarn(ScriptRuntime.java:1509)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_lib_jsdoc_util_templateHelper_js_74._c_anonymous_26(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_lib_jsdoc_util_templateHelper_js_74.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.call1(OptRuntime.java:32)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73._c_anonymous_17(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73.call(Unknown Source)
    at org.mozilla.javascript.NativeArray.iterativeMethod(NativeArray.java:1589)
    at org.mozilla.javascript.NativeArray.execIdCall(NativeArray.java:318)
    at org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:97)
    at org.mozilla.javascript.optimizer.OptRuntime.call1(OptRuntime.java:32)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73._c_addSignatureReturns_16(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.callName(OptRuntime.java:63)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73._c_anonymous_47(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:42)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_node_modules_taffydb_taffy_js_72._c_anonymous_3(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_node_modules_taffydb_taffy_js_72.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.callName(OptRuntime.java:63)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_node_modules_taffydb_taffy_js_72._c_anonymous_82(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_node_modules_taffydb_taffy_js_72.call(Unknown Source)
    at org.mozilla.javascript.ScriptRuntime.applyOrCall(ScriptRuntime.java:2429)
    at org.mozilla.javascript.BaseFunction.execIdCall(BaseFunction.java:269)
    at org.mozilla.javascript.IdFunctionObject.call(IdFunctionObject.java:97)
    at org.mozilla.javascript.optimizer.OptRuntime.call2(OptRuntime.java:42)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_node_modules_taffydb_taffy_js_72._c_anonymous_6(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_node_modules_taffydb_taffy_js_72.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.call1(OptRuntime.java:32)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73._c_anonymous_39(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_brjs_template_publish_js_73.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.callN(OptRuntime.java:52)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18._c_anonymous_26(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.callProp0(OptRuntime.java:85)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18._c_anonymous_23(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.callProp0(OptRuntime.java:85)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18._c_anonymous_16(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.callName(OptRuntime.java:63)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18._c_anonymous_9(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_cli_js_18.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.call1(OptRuntime.java:32)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_jsdoc_js_1._c_anonymous_3(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_jsdoc_js_1.call(Unknown Source)
    at org.mozilla.javascript.optimizer.OptRuntime.call0(OptRuntime.java:23)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_jsdoc_js_1._c_script_0(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_jsdoc_js_1.call(Unknown Source)
    at org.mozilla.javascript.ContextFactory.doTopCall(ContextFactory.java:394)
    at org.mozilla.javascript.ScriptRuntime.doTopCall(ScriptRuntime.java:3090)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_jsdoc_js_1.call(Unknown Source)
    at org.mozilla.javascript.gen.file__D__re_ct_ct_core_sdk_jsdoc_toolkit_resources_jsdoc_toolkit_jsdoc_js_1.exec(Unknown Source)
    at org.mozilla.javascript.commonjs.module.Require.executeModuleScript(Require.java:355)
    at org.mozilla.javascript.commonjs.module.Require.getExportedModuleInterface(Require.java:288)
    at org.mozilla.javascript.commonjs.module.Require.requireMain(Require.java:137)
    at org.mozilla.javascript.tools.shell.Main.processSource(Main.java:525)
    at org.mozilla.javascript.tools.shell.Main.processFiles(Main.java:176)
    at org.mozilla.javascript.tools.shell.Main$IProxy.run(Main.java:100)
    at org.mozilla.javascript.Context.call(Context.java:489)
    at org.mozilla.javascript.ContextFactory.call(ContextFactory.java:504)
    at org.mozilla.javascript.tools.shell.Main.exec(Main.java:158)
    at org.mozilla.javascript.tools.shell.Main.main(Main.java:136)
undefined

dchambers · 2014-08-05T09:30:49Z

I agree with @andyberry88 that the kind of changes @janhancic is suggesting (referring to classes and modules using the same syntax) are not changes we should be making, but should be raised directly against the upstream project.

That being said, I think what @janhancic is saying makes sense since as it happens both CommonJs and NamespacedJs classes are served to the browser as CommonJs modules, and are both modelled internally as modules (they are both instances of SourceModule). Therefore, is the problem actually that we are running the jsdoc command on the source code rather than the transpiled source code?

thecapdan · 2014-08-05T12:49:17Z

Checklist of improvements (in no particular order)

investigate poor performance
fail more gracefully when incorrect syntax is used
improved ability to search the main jsdoc page (now that it's no longer a tree)
more consistency between commonjs/namespacedjs (JanH)

thecapdan · 2014-08-11T08:37:47Z

Some more notes on this one:

Found that one class that was taking more than most was ComponentFixture.js - could look into what in terms of jsdoc it has moreso than other classes - at first glance it looks to have a lot of '@see's.

Also, worth noting that the command requires a working set of well over a gigabyte in memory

jeremyherr · 2014-08-14T11:43:02Z

In brjs API docs as well as CT, for modules, the constructor name is shown as a brackets-enclosed require statement.

The first thing the user sees when looking at modules looks like garbage at first glance. Here is an example: http://apidocs.bladerunnerjs.org/v0.11/js/KnockoutComponent.html

I propose that this should read new KnockoutComponent(sTemplateId, oViewModel) instead of new (require("br/knockout/KnockoutComponent"))(sTemplateId, oViewModel)
We have the same problem in CT for all of our modules. Since all classes in CommonJS are in modules, this affects all of our new code.

jeremyherr · 2014-08-14T11:47:32Z

Also for modules JSDoc, there is a redundant link to source code underneath the constructor. Again as an example: http://apidocs.bladerunnerjs.org/v0.11/js/KnockoutComponent.html.

There are 2 links next to each other, one to line 3, another to line 25.

jeremyherr · 2014-08-15T15:02:26Z

@interface doesn't work. This page seems to indicate that it is something that could be made to work by modifying templates? https://code.google.com/p/jsdoc-toolkit/issues/detail?id=229

hegemonic · 2014-08-21T20:36:34Z

Hi, I'm the maintainer of JSDoc, and I just stumbled across this issue.

I've made some changes to JSDoc over the past couple of weeks that should drastically improve performance for large docsets. I haven't investigated memory usage, though. Please try the current version of JSDoc on master and let me know if you're still seeing performance and/or memory issues. (If you can run JSDoc on Node.js, rather than Mozilla Rhino, that will also give you much better performance.)

Also, the @interface and @implements tags are now available in JSDoc on master. (The link above points to the issue tracker for JSDoc 2, which is extremely out of date and should be turned off. See https://github.com/jsdoc3/jsdoc/issues for a current list of issues.)

I'll work with @dchambers on the issues he filed. I'm about to go out of town, though, so it will be a couple of weeks before I can respond in detail.

dchambers · 2014-08-21T20:52:42Z

Thanks @hegemonic, your assistance is very much appreciated.

leggetter · 2014-08-21T20:53:06Z

👍

dchambers · 2014-08-27T09:01:22Z

IMO, the various issues raised here in this meta issue have either been fixed by #926, or mitigated against.

Things that have been fixed:

All classes (both NamespaceJs and CommonJs) are now defined within modules, so stuff won't break as we migrate.
Interfaces are now correctly supported.
Classes, interfaces, events & modules are now displayed in their own sections of the index.
The redundant extra link to the source code is now fixed, as it's now rendered in the correct section.
I didn't see the hanging issues described by @thecapdan while working with the latest version of jsdoc3.
The time to build very large code-bases like CT should hopefully now be improved.

This leaves the following:

The constructor name is shown as a brackets-enclosed require statement -- this doesn't feel like a problem anymore with the new styling, where there is a 'Constructor' heading, and where the typeface size is smaller.
Fail more gracefully when incorrect syntax is used -- I think this has improved, but any issues we do find should be raised against the upstream project.
Improved ability to search the main jsdoc page (now that it's no longer a tree) -- this is a non-trivial feature that I've raised as a separate bug, and which isn't high enough priority given it's cost to deal with now as for the time being developers can just use the browser's in-built search functionality.

thanhc · 2014-08-29T15:51:54Z

verified all the points Dom's mentioned above, all classes are now displayed as module:my/Module.
Interfaces are displayed in there own sections as other types.

No performance improvements noticed while running jsdoc against live-examples compared to the previous version.

jeremyherr · 2014-09-02T16:06:52Z

Using v0.11.324-ge900841

classes appear twice, both in the classes and modules lists. All classes are modules, so it doesn't make sense to list classes under modules.

constructor name is not valid JavaScript, e.g.

new module:br/component/testing/ComponentFixture(sXml, oModelFixture, oViewFixture)

h1 at top of page for classes looks weird, e.g.

Class: module:br/component/testing/ComponentFixture

thecapdan · 2014-09-03T08:14:35Z

Here's a illustration of the issue seen by @jeremyherr

(moving a couple of issues back to test to discuss during today's standup)

andy-berry-dev · 2014-09-03T09:50:24Z

This is an issue that is likely to be fixed in coming releases of jsdoc. We spoke about this at the standup today and the general consensus was that we wait for it to be fixed upstream rather than hacking it in to BRJS.

Moving this back in to the backlog until its 100% fixed.

dchambers · 2014-09-22T08:37:26Z

@jeremyherr: Sorry, only just seen your comment, but to address your points:

classes appear twice, both in the classes and modules lists. All classes are modules, so it doesn't make sense to list classes under modules.

While it's true that both BladeRunnerJS and CaplinTrader adhere to the one class per module rule, it's also possible to have modules that export multiple classes. Also, the maintainer of jsdoc feels this is how it should work since modules that contain classes are both modules and classes, and so this is deliberate.

constructor name is not valid JavaScript, e.g. new module:br/component/testing/ComponentFixture(sXml, oModelFixture, oViewFixture)

I agree, but suggest it would be better to raise this as single issue item against the upstream project.

h1 at top of page for classes looks weird, e.g. Class: module:br/component/testing/ComponentFixture

I actually think this is broadly a good thing. The logical name of the entity being documented is module:br/component/testing/ComponentFixture, and this is what you would need to use in any jsdoc that wants to refer to this class. Additionally, it is a class.

In the old documentation we used to have different icons for classes, interfaces, singletons, etc, but since jsdoc does't have icons, having some text to tell us what we're looking at is the next best thing IMO. Again, if you disagree I'd recommend creating an issue with the upstream project.

dchambers · 2014-09-22T08:40:39Z

@andyberry88, I'd suggest we close this issue as we've now integrated with jsdoc, ironed out any upgrade issues, and contributed a number of improvements as pull-requests. IMO, any further improvements to jsdoc3 should be issued against that project as a number of single-use issues so they can be more easily managed.

andy-berry-dev · 2014-09-22T08:42:12Z

@dchambers Agreed. I'll move it to 'ReadyForTest' so @thecapdan has a chance to comment first.

thecapdan · 2014-09-26T08:30:07Z

Since we've found a workaround, we can move to done - but keep an eye on jsdoc upgrades that might improve performance

andy-berry-dev modified the milestones: 0.11, v0.12 Jul 22, 2014

andy-berry-dev added enhancement labels Jul 22, 2014

andy-berry-dev modified the milestones: 0.12, 0.13 Aug 4, 2014

andy-berry-dev added 0 - Backlog and removed 2 - Dev labels Aug 4, 2014

andy-berry-dev added size 4 and removed size 1 labels Aug 15, 2014

andy-berry-dev modified the milestones: 0.13, 0.12 Aug 15, 2014

andy-berry-dev added 1 - Planned and removed 0 - Backlog labels Aug 15, 2014

dchambers added 3 - ReadyForTest and removed 1 - Planned labels Aug 27, 2014

thanhc added 5 - Done and removed 3 - ReadyForTest labels Aug 29, 2014

thecapdan added 4 - Test and removed 5 - Done labels Sep 3, 2014

andy-berry-dev removed this from the 0.12 milestone Sep 3, 2014

andy-berry-dev added 0 - Backlog and removed 4 - Test labels Sep 3, 2014

andy-berry-dev modified the milestone: 0.13 Sep 3, 2014

andy-berry-dev added 4 - ReadyForTest and removed 0 - Backlog labels Sep 22, 2014

thecapdan added 6 - Done and removed 4 - ReadyForTest labels Sep 26, 2014

andy-berry-dev added this to the 0.13 milestone Nov 13, 2014

andy-berry-dev removed the 6 - Done label Nov 13, 2014

andy-berry-dev closed this as completed Nov 13, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JsDoc improvements #854

JsDoc improvements #854

janhancic commented Jul 18, 2014

janhancic commented Jul 18, 2014

janhancic commented Jul 18, 2014

andy-berry-dev commented Jul 29, 2014

janhancic commented Jul 29, 2014

andy-berry-dev commented Jul 29, 2014

janhancic commented Jul 29, 2014

thecapdan commented Aug 4, 2014

thecapdan commented Aug 5, 2014

dchambers commented Aug 5, 2014

thecapdan commented Aug 5, 2014

thecapdan commented Aug 11, 2014

jeremyherr commented Aug 14, 2014

jeremyherr commented Aug 14, 2014

jeremyherr commented Aug 15, 2014

hegemonic commented Aug 21, 2014

dchambers commented Aug 21, 2014

leggetter commented Aug 21, 2014

dchambers commented Aug 27, 2014

thanhc commented Aug 29, 2014

jeremyherr commented Sep 2, 2014

thecapdan commented Sep 3, 2014

andy-berry-dev commented Sep 3, 2014

dchambers commented Sep 22, 2014

dchambers commented Sep 22, 2014

andy-berry-dev commented Sep 22, 2014

thecapdan commented Sep 26, 2014

JsDoc improvements #854

JsDoc improvements #854

Comments

janhancic commented Jul 18, 2014

janhancic commented Jul 18, 2014

janhancic commented Jul 18, 2014

andy-berry-dev commented Jul 29, 2014

janhancic commented Jul 29, 2014

andy-berry-dev commented Jul 29, 2014

janhancic commented Jul 29, 2014

thecapdan commented Aug 4, 2014

thecapdan commented Aug 5, 2014

dchambers commented Aug 5, 2014

thecapdan commented Aug 5, 2014

thecapdan commented Aug 11, 2014

jeremyherr commented Aug 14, 2014

jeremyherr commented Aug 14, 2014

jeremyherr commented Aug 15, 2014

hegemonic commented Aug 21, 2014

dchambers commented Aug 21, 2014

leggetter commented Aug 21, 2014

dchambers commented Aug 27, 2014

thanhc commented Aug 29, 2014

jeremyherr commented Sep 2, 2014

thecapdan commented Sep 3, 2014

andy-berry-dev commented Sep 3, 2014

dchambers commented Sep 22, 2014

dchambers commented Sep 22, 2014

andy-berry-dev commented Sep 22, 2014

thecapdan commented Sep 26, 2014