#18002 closed defect (fixed)
dojo doc breaking js-doc-parse
Reported by: | lee | Owned by: | |
---|---|---|---|
Priority: | undecided | Milestone: | 1.10 |
Component: | Documentation | Version: | 1.10.0-beta1 |
Keywords: | Cc: | Sebastien Pereira, Patrick Ruzand, Kris Zyp | |
Blocked By: | Blocking: |
Description
I've tried running js-doc-parse against dojo-release-1.10.0-beta1-src and 4 comment issues cause the doc parser to break.
They are minor fixes, indentation, block comments needed etc but will need to be fixed before release. If I get time I'll submit the PR's myself but if anyone else is currently working on these components, can you make the following changes? :
dojox/mobile/scrollable.js
ERR: dojox/mobile/scrollable.js:1288:49 TypeError?: Cannot read property '1' of null
(line 1281) So I fixed up locally the inline comments to a block comment e.g.
/* #17822: when scrolling on one direction, avoid unnecessarily animating both top and left, because this leads to two transitionend events fired instead of one in some browsers (Safari/iOS7 at least). */
dojox/charting/action2d/TouchIndicator.js
ERR: dojox/charting/action2d/TouchIndicator.js:110:20 TypeError?: Cannot read property '1' of null
(Line 117) I fixed locally indentation to:
// we should not have to do that, but dojo/touch is causing performances issue // we have to workaround here for now
dojox/store/db/IndexedDB.js ERR: dojox/store/db/IndexedDB.js:186:16 TypeError?: Cannot read property '1' of null
(Line 186). Not sure why the object's props are commented out, maybe should be in an example tag instead? I just locally added block comments to get it to parse e.g. :
indices: { /* property: { preference: 1, multiEntry: true } */ },
dojox/mobile/migrationAssist.js
ERR: dojox/mobile/migrationAssist.js:323:27 TypeError?: Cannot call method 'mixinProperties' of undefined
I guess https://github.com/dojo/dojox/commit/65a4f1dd2abdd598f5476e2cf2131771914c5c3d#diff-fefdc9df5f71203b7e0916fc24b1ba1dR316 caused this? I undid this and it worked e.g. changing back to
extendSelectFunction = function(obj) {
I don't think this should be in the API anyway so instead I added an ignore to js-doc-parse/config.js excludes e.g. :
// not to be documented? /dojox\/mobile\/migrationAssist\.js/,
Change History (15)
comment:1 Changed 7 years ago by
Cc: | Sebastien Pereira Patrick Ruzand Kris Zyp added |
---|
comment:2 Changed 7 years ago by
comment:3 Changed 7 years ago by
Note that I don't understand why such multiple single-line comments need to be changed to block commnets:
domStyle.set(node, css3.add({}, { // #17822: when scrolling on one direction, avoid unnecessarily animating // both top and left, because this leads to two transitionend events fired // instead of one in some browsers (Safari/iOS7 at least). transitionProperty: (to.x !== undefined && to.y !== undefined) ?
There are multiple cases in Dojo where successive single line comments is not an issue for js-doc-parse. Why this one ? There's obviously something I am missing...
comment:4 Changed 7 years ago by
I wanted js-doc-parse to run so I didn't look long at it, it's probably just indentation
comment:5 follow-up: 7 Changed 7 years ago by
I would guess that it's looking at your #17822: and trying to parse it because of the : (that's the pattern that is used for things like summary, description, TODO, etc. So I would remove that and see if it parses.
comment:6 Changed 7 years ago by
Plus it's directly after a brace, it looks a lot like:
dojo.declare(..., { // summary: ...
comment:7 Changed 7 years ago by
comment:8 follow-up: 11 Changed 7 years ago by
re: dojo/mobile/migrationAssist, it should indeed be excluded from the api doc.
comment:10 Changed 7 years ago by
The changeset above fixes dojox/charting/action2d/TouchIndicator.js and dojox/mobile/scrollable.js issues.
comment:11 Changed 7 years ago by
Replying to pruzand:
re: dojo/mobile/migrationAssist, it should indeed be excluded from the api doc.
Hmm, I thought we had done that, but it's not listed in https://github.com/wkeese/js-doc-parse/blob/master/config.js.
comment:13 Changed 7 years ago by
Milestone: | tbd → 1.10 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
I excluded migrationAssist.js from scanning in https://github.com/wkeese/js-doc-parse/commit/acfdc99508e20ad7bb16ccdd2f1c5dbe6eff2f73. Parsing seems to work now.
I'll fix the dojox/mobile issues.