JSDoc correction for namespaces

[herregroen]

Currently the generated JSDoc will look as follows: ​https://atimmer.github.io/wordpress-jsdoc/.

This has the following issues:

• Everything is in the global namespace.
• Many functions are incorrectly seen as global functions due to JSDoc not recognising extend.
• The default JSDoc template will duplicate any namespaces that are defined multiple times.

To fix this the following should happen:

• Define all used namespaces using @namespace.
• Correctly specify in which namespace each class is using @memberOf.
• Define each usage of the extend function as a prototype assignment using @lends.

[herregroen]

Patch

[herregroen]

This patch will generate JSDoc that looks as follows: ​https://herregroen.github.io/wordpress-jsdoc/.

It offers the following improvements:

• There's a clear overview of namespaces.
• All classes are children of their respective namespaces.
• Only actually global functions are part of the global namespace.
• All class functions are correctly listed under their class.

The following could still be of interest:

• Add a readme.md to display on the homepage.
• Change the theme. The template used is ​https://github.com/docstrap/docstrap which allows for several themes, currently the united theme is chosen. This template was chosen for it's vastly superior namespace support over the default template.

[herregroen]

Updated patch

[atimmer]

Just for context: I created ​https://atimmer.github.io/wordpress-jsdoc/ to be an auto-updating representation of the documentation that could be generated from the JavaScript core.

@herregroen has gone through all the documentation in wp-includes and made the namespace structure parsable by jsdoc. This will also make it much easier to parse the output of JSDoc and put it in a WordPress installation.

[adamsilverstein]

Great work, this is looking really good!

Where would we would include this resource in the handbook? Let's adjust the theme to match the styles there.

[netweb]

Replying to adamsilverstein:

Great work, this is looking really good!

That it is +1

Where would we would include this resource in the handbook? Let's adjust the theme to match the styles there.

Eventually we'd want this on the ​https://developer.wordpress.org/ site wouldn't we?

There's also an option to use and adpapt if needed this ​GitHub to w.org Handbook sync plugin using ​one ​of the the JSDoc to Markdown theme templates.

We could also write our own JSDocs templates (not that that I've looked into what this would entail)

[herregroen]

I've just updated the preview ( ​https://herregroen.github.io/wordpress-jsdoc/ ) using an updated copy of DocStrap ​https://github.com/herregroen/docstrap.

This was simply updating Docstrap to use the latest version of Bootswatch ​https://bootswatch.com/. The most recent version of Cosmo seemed somewhat WordPress Codex like.

Given that the template does do quite a bit in structuring the menus and adding search functionality I expect the easiest way at least to get close to the codex look would be to base it on our own fork of DocStrap and adapting the styles/main.less file there as needed.

[atimmer]

I think the eventual goal should be to have this on developer.wordpress.org just like the parsed PHP code is on there. This will make it possible to have user contributed notes and extra information for functions and namespaces.

The way to get there is by building a convert step from JSDoc to WordPress just like we have for PHP. The PHP work on this can be found on ​https://github.com/wordpress/phpdoc-parser. I started on a JavaScript version at ​https://github.com/yoast/jsdoc-parser. This could use all the help it can get. The current idea is to use something like ​https://www.npmjs.com/package/jsdoc-json to convert the internal JSDoc representation to JSON and use that to create posts in a WordPress site.

[netweb]

Related: ​#3063-meta Show JavaScript documentation on developer.wordpress.org

[herregroen]

Fix typo in class name.

[adamsilverstein]

Hey @herregroen - thanks for your work here... I was trying to see what you fixed in jsdoc-namespaces.3.patch​ however it seems to be exactly the same as jsdoc-namespaces.2.patch​ - are you sure you got that final change saved in this diff?

[adamsilverstein]

@herregroen I reviewed your patch and have a few questions:

• do the @lends statements need to be inserted inline like that (and not in the doc block above)? Makes for really long lines and more difficult to read
• In a few spots you have removed existing inline comments. Are these changes related? can we remove them to keep this patch focused solely on the namespace changes?
• are parse errors expected when run locally? i did npm install and grunt jsdoc and although many files parsed fine, I also got a bunch of: ERROR: Unable to parse a tag's type expression for source file

[herregroen]

@adamsilverstein

• The third patch contains a single additional character on a line that's changed in both version ( @class wp.media.collectio -> @class wp.media.collection ).
• The @lends statements need to be right before the object passed to extend starts. An alternative to reduce line lengths could be:

    Class.extend(
        /** @lends Class.prototype */
        {
            method: function () {
                // Do stuff.
            }
            ... 

• The comments I've removed were either incorrectly used @global tags ( these do not document a global being used, they document that the function/variable being documented is a global and thus override @memberOf statements ) or comments I felt added no additional information when the correct tags were added ( for example: utility function namespace to @namespace wp.customize.utils )
• The parse errors are expected. The @global tags I've removed were not the only tags being incorrectly used and JSDoc throws errors for each incorrect usage. The official WordPress javascript documentation standards ( ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/javascript/ ) are not compatible with JSDoc and using them will result in errors. My suggestion here would be to change the javascript documentation standards to match the standards of JSDoc, but that's probably an issue for another ticket.

[SergeyBiryukov]

Replying to herregroen:

The comments I've removed were either incorrectly used @global tags ( these do not document a global being used, they document that the function/variable being documented is a global and thus override @memberOf statements ) or comments I felt added no additional information when the correct tags were added

I've noticed the patch removes one of the comments added in [33709] for customize-loader.js:

/* 
 * Expose a public API that allows the customizer to be 
 * loaded on any page. 
 */ 

But keeps a similar comment in customize-preview.js:

/* 
 * Script run inside a Customizer preview frame. 
 */ 

Is there a reason for removing the first comment?

[netweb]

Are the patches here the same as that of this pull request?

• ​https://github.com/Yoast/wordpress-develop-mirror/pull/77

It looks like it is, I've added a comment per @SergeyBiryukov comment above in comment:14 inline in that PR

• ​https://github.com/Yoast/wordpress-develop-mirror/pull/77#pullrequestreview-58550931

It might be easier to code review this on GitHub, changes between patches are a little easier to view such as what @adamsilverstein mentioned above in comment:11

[netweb]

Why are variables being moved to the top of the file? Does JSDoc require this for formatting?

There's quite a few instances of this, for example:

src/wp-includes/js/media/controllers/customize-image-cropper.js

@@ +1 @@
+var Controller = wp.media.controller,
+        CustomizeImageCropper;
+
 /**
  * wp.media.controller.CustomizeImageCropper
  *
+ * @memberOf wp.media.controller
+ *
  * A state for cropping an image.
  *
  * @class
@@ -8 +13 @@
  * @augments wp.media.controller.State
  * @augments Backbone.Model
  */
-var Controller = wp.media.controller,
-        CustomizeImageCropper;
-
-CustomizeImageCropper = Controller.Cropper.extend({
+CustomizeImageCropper = Controller.Cropper.extend(/** @lends wp.media.controller.CustomizeImageCropper.prototype */{
         doCrop: function( attachment ) {
                 var cropDetails = attachment.get( 'cropDetails' ),
                         control = this.get( 'control' ),

Leaving the var declarations where they originally were doesn't cause any issue with linting via JSHint or the proposed ESLint changes we're in the progress of implementing

[herregroen]

Replying to netweb:

Why are variables being moved to the top of the file? Does JSDoc require this for formatting?

There's quite a few instances of this, for example:

src/wp-includes/js/media/controllers/customize-image-cropper.js

@@ +1 @@
+var Controller = wp.media.controller,
+        CustomizeImageCropper;
+
 /**
  * wp.media.controller.CustomizeImageCropper
  *
+ * @memberOf wp.media.controller
+ *
  * A state for cropping an image.
  *
  * @class
@@ -8 +13 @@
  * @augments wp.media.controller.State
  * @augments Backbone.Model
  */
-var Controller = wp.media.controller,
-        CustomizeImageCropper;
-
-CustomizeImageCropper = Controller.Cropper.extend({
+CustomizeImageCropper = Controller.Cropper.extend(/** @lends wp.media.controller.CustomizeImageCropper.prototype */{
         doCrop: function( attachment ) {
                 var cropDetails = attachment.get( 'cropDetails' ),
                         control = this.get( 'control' ),

Leaving the var declarations where they originally were doesn't cause any issue with linting via JSHint or the proposed ESLint changes we're in the progress of implementing

JSDoc matches documentation with the line exactly below it.

The comment blocks at the top of files were documenting the var declarations, not the class declaration. This means that the comment block was documenting a class called Controller which has no methods or members of it's own. That's clearly not the intent.

This ties in with the Wordpress Javascript documentation standards leading to errors. A documentation block containing @class should be directly above the class declaration. If the intent is to add a comment block to the top of a file to document what the file contains then this should be done with using @file.

If JSHint or ESLint want documentation blocks at the top of files then these can be added but they should only contain the @file tag and possible the @author tag, there should be no other tags.

[herregroen]

Replying to SergeyBiryukov:

Replying to herregroen:

The comments I've removed were either incorrectly used @global tags ( these do not document a global being used, they document that the function/variable being documented is a global and thus override @memberOf statements ) or comments I felt added no additional information when the correct tags were added

I've noticed the patch removes one of the comments added in [33709] for customize-loader.js:

/* 
 * Expose a public API that allows the customizer to be 
 * loaded on any page. 
 */ 

But keeps a similar comment in customize-preview.js:

/* 
 * Script run inside a Customizer preview frame. 
 */ 

Is there a reason for removing the first comment?

In the places I've added @namespace tags where there were existing comments mentioning exposing the variable, declaring a namespace or anything to that extent I've removed those comments as I personally felt the @namespace tag conveys this information already.

If people feel these comments still add value then I will add them back in.

[herregroen]

Use single quotes in Gruntfile and tabs in jsdoc.conf.json

[herregroen]

I've uploaded a new patch responding to some reviews from @netweb on GitHub ( see: ​https://github.com/Yoast/wordpress-develop-mirror/pull/77 ).

[SergeyBiryukov]

Replying to herregroen:

If people feel these comments still add value then I will add them back in.

I second @adamsilverstein's ​comment:

I would prefer all the comments be left in unless they effect the namespace parsing. That way, this patch remains smaller and more focused. We can address these other changes in a follow up ticket if needed.

[netweb]

Replying to herregroen:

JSDoc matches documentation with the line exactly below it.

The comment blocks at the top of files were documenting the var declarations, not the class declaration. This means that the comment block was documenting a class called Controller which has no methods or members of it's own. That's clearly not the intent.

This ties in with the Wordpress Javascript documentation standards leading to errors. A documentation block containing @class should be directly above the class declaration. If the intent is to add a comment block to the top of a file to document what the file contains then this should be done with using @file.

Perfect, thanks for the detailed reply +1

If JSHint or ESLint want documentation blocks at the top of files then these can be added but they should only contain the @file tag and possible the @author tag, there should be no other tags.

+1 Let's look at doing this after this has been merged in a new ticket

[herregroen]

Restore removed comments.

[herregroen]

@adamsilverstein

I've just uploaded a patch that restores the few comments that were removed.

[netweb]

Replying to herregroen:

@adamsilverstein

I've just uploaded a patch that restores the few comments that were removed.

Thanks, here's that change/commit on GitHub ​https://github.com/Yoast/wordpress-develop-mirror/pull/77/commits/9cac1b99dd7371249f9fb6a9bdcf06fda4c6697c

[adamsilverstein]

41682.diff​ is a refresh of jsdoc-namespaces.5.patch​ against trunk.

[adamsilverstein]

In 41351:

Docs: JSDoc improvements for namespaces.

Improve JS parsing of our inline JSDocs by introducing @namespace, @lends and @memberOf. Helps set the way for showing our JavaScript documentation on developer.wordpress.org, see ​https://meta.trac.wordpress.org/ticket/3063.

• Define all used namespaces using @namespace.
• Correctly specify in which namespace each class is using @memberOf.
• Define each usage of the extend function as a prototype assignment using @lends.
• Some comment blocks were moved to correct the parsing of certain definitions.

Props herregroen, atimmer, netweb, SergeyBiryukov.  
Fixes #41682.

[netweb]

Reopening to add the new file jsdoc.conf.json that was missed in [41351]

@adamsilverstein SVN being SVN ;) -> svn add jsdoc.conf.json

[adamsilverstein]

@netweb ha, so many files in my status list I missed it - thanks for catching, will commit soon.

[adamsilverstein]

In 41370:

Docs: Add jsdoc.conf.json JSDOC configuration file.

Left this file off by mistake in [41351].

Fixes #41682.

[netweb]

Patch 41682.3.diff​ adds the jsdoc folder to .gitignore and svn:ignore

The svn:ignore change is not visible here on Trac but is shown in the diff once downloaded.

Alternatively running svn propedit svn:ignore . and adding jsdoc to the end of the file will do the trick

cc @adamsilverstein

[adamsilverstein]

Thanks @netweb, I'll get this committed.

[adamsilverstein]

In 41381:

JSDocs: add the jsdoc folder to .gitignore and svn:ignore.

Props netweb.
Fixes #41682.