JSDoc book module

Brilliant.

Hm. So I've now made this comment on 2 other patches...

I've noticed that Drupal~behavior items have attach: functions in them. The lines look like this:

  Drupal.behaviors.bookDetailsSummaries = {
    attach: function (context) {
 

So it looks to me like this is a function called Drupal.behaviors.bookDetailsSummaries.attach right? If so, shouldn't it (and all the similar ones) have docs for the attach() function (which maybe is just an @inheritdoc)?

The rest of this patch looks good!

I defined the Drupal~behavior type to avoid copy pasting. Basically tagging something as Drupal~behavior means the thing is an object with an attach and detach property. Both properties are functions with their own parameters.

I don't feel it's needed to document both because detach should be undoing whatever attach function does (in reality it's the case when a detach function exist) and only that so describing what is done in attach is sufficient. If we where to document both function individually we would need to do :

/**
 * <description>
 *
 * @namespace
 */
Drupal.behaviors.activeLinks = {

  /**
   * <description>
   *
   * @param {HTMLDocument|HTMLElement} context
   */
  attach: function (context) {
    // …
  },

  /**
   * <description>
   *
   * @param {HTMLElement} context
   * @param {object} settings
   * @param {string} trigger
   */
  detach: function (context, settings, trigger) {
    // …
  }
};

One downside of this form is that we would need to always document context, settings and trigger parameters because we can't use @inherit since there is no Constructor/Class involved with behaviors (nothing to inherit from in the sense of javacript objects). And if we were to use something like @borrows (which didn't work when i tried, don't know if jsdoc bug or me), we would lose the ability to describe individual settings keys needed in the settings object like it's done in the proposed patch #1835016-38: Polyfill date input type.

The downside of using Drupal~behavior is that we can't document what's in settings for attach function like it's done in the issue above.

So our PHP docs standards basically say "Everything needs a doc block". Do you think that should not be the case here? I mean, it's a member function, so it seems like it should have docs of some sort, and also isn't it the case that a function without JSDocs does not show up on the JSDoc output site?

I'm worried about people being too lazy to document all the parameters properly and having rubbish docs in their projects. Adding just one @type tag to the behavior seems easier.

The page with the list of behaviors is easy to follow: http://read.theodoreb.net/drupal-jsapi/Drupal.behaviors.html we know what's there and the description is easy to access. For the type information clicking on the type beside the name redirect to http://read.theodoreb.net/drupal-jsapi/Drupal.html#~behavior which will show attach/detach and all the types and params available.

I tried several things to make it easy on people to document this but unfortunately there is nothing we can bend to have an @inheritdoc type of thing. This is due to the way our code is architected and I didn't find a way to make it easier. Also if we document attach/detach, unless the parent object is declared as @namespace, anything written in the docblocks will not show up in the docs.

I'll let you choose what you prefer between the 3 possibilities. For me:
1. is the easiest,
2. is the most explicit and readable (talking about the doc output), it would be my pick,
3. is the most correct but error prone.

1. /**
    * <description>
    *
    * @type {Drupal~behavior}
    */
   Drupal.behaviors.activeLinks = {
     attach: function (context) {
       // …
     },
     detach: function (context, settings, trigger) {
       // …
     }
   };
   

2. New idea I got earlier:
   

   /**
    * <description>
    *
    * @type {Drupal~behavior}
    *
    * @prop {Drupal~behaviorAttach} attach
    *   What does the attach function does.
    * @prop {Drupal~behaviorDetach} detach
    *   What does the detach function does.
    */
   Drupal.behaviors.activeLinks = {
     attach: function (context) {
       // …
     },
     detach: function (context, settings, trigger) {
       // …
     }
   };

3. /**
    * <description>
    *
    * @namespace
    */
   Drupal.behaviors.activeLinks = {
     /**
      * <description>
      *
      * @param {HTMLDocument|HTMLElement} context
      */
     attach: function (context) {
       // …
     },
   
   
     /**
      * <description>
      *
      * @param {HTMLElement} context
      * @param {object} settings
      * @param {string} trigger
      */
     detach: function (context, settings, trigger) {
       // …
     }
   };

Thanks for the explanation. I'm OK with any of those options you presented.

How about if you pick one of your choice and update the JS docs standards page to explain when this type of documentation should be used instead of the usual "everything needs documentation" rul?

All right, number #2 it is, will make is easy to spot behavior with no detach function (a todo to solve in core at some point).

Thanks! I'll get the doc updated.

Updated: https://www.drupal.org/node/2183405#behavior

Standard was decided and patch updated accordingly (and all the JSDoc patches in the queue were updated by eiriksm too).

Committed and pushed to 8.0.x. Thanks!