[discussion] JSDoc style convention for documenting packages/plugins

[machikoyasuda]

Kicking off a place to talk about documenting packages/plugins for Reaction, starting with this PR: reactioncommerce/reaction#2808

General documentation is here https://github.com/reactioncommerce/reaction-jsdoc

[machikoyasuda]

@spencern

[machikoyasuda]

Our latest work:

Things we talked about:

• add @Private for anything just used for the plugin or not exported
• @module connectors/shopify/
• @module - name of the package/

[spencern]

Here's an illustration for all of you who don't want to go reading the jsdoc docs yourself.

So here are the conventions that I used to get the above structure.

In every file, right after imports

/**
 * @file Shopify connector webhook methods
 *       contains methods for creating and deleting webhooks providing
 *       synchronization between a Shopify store and a Reaction shop
 * @module connectors/shopify/webhooks
 */

In the method jsdoc

  /**
   * Meteor method for creating a shopify webhook for the active shop
   * See: https://help.shopify.com/api/reference/webhook for list of valid topics
   * @async
   * @method connectors/shopify/webhooks/create
   * @param {Object} options Options object
   * @param {string} options.topic - the shopify topic to subscribe to
   * @param {string} [options.absoluteUrl] - Url to send webhook requests - should only be used in development mode
   * @return {void}
   */

In a private local function jsdoc

/**
 * Transforms a Shopify product into a Reaction product.
 * @private
 * @method createReactionProductFromShopifyProduct
 * @param  {object} options Options object 
 * @param  {object} options.shopifyProduct the Shopify product object
 * @param  {string} options.shopId The shopId we're importing for
 * @param  {[string]} options.hashtags An array of hashtags that should be attached to this product.
 * @return {object} An object that fits the `Product` schema
 *
 * @todo consider abstracting private Shopify import helpers into a helpers file
 */

Include the following:

• @file This becomes the abstract for the file. It's a short block of text that is above all methods, but below the @module name
• @module This is what I used to organize methods into different jsdoc sections. Looking at the navigation on the left, you can see that methods within a file get organized below the module they represent.
• @async - Only include this if your method is an async method.
• @private - Use this for any local functions (not exported) and probably for any functions that are exported as helpers, but should not be used outside of the plugin. Using @private will cause the method not to be listed in jsdoc, but we should still be documenting these methods with jsdoc for consistency. Don't think @Private vs just not documenting. Document Everything. Use @Private to hide from jsdoc output.
• @method - this should be the method name. I think we could have a good debate as to whether we want to use the Meteor.method version of these or a local, scoped version. E.g. if the module is connectors/shopify/webhooks we could jsdoc name the method create even though it will get exported to Meteor as connectors/shopify/webhooks/create
• @param - This must be included for each argument that the method takes
  • properties - using options.topic as above, creates a property for the param. This is how you should document methods that receive options objects.
  • Using square brackets around a param means that it's optional. [options.absoluteUrl] is an optional property and jsdoc will read it that way when the square brackets are present.
• @return essentially the @param for the return. You should provide the return type as well as a description of what gets returned. There is a way to have multiple return types, but I'd also like to see us remove that from our code. Every method should return a single type IMO. For methods that don't return, I see people using @return {void} and also @return {undefined} we should have a single way this is done.
• @todo - You can include this in the jsdoc for a method and the TODOs will get included in the jsdoc files.
• @deprecated - you can include this in the jsdoc for a method to indicate that the method is deprecated.

[zenweasel]

I think @return {void} is a very Java way of doing things and makes no sense in Javascript, @return {undefined} tells you exactly what's happening.

My only comment except for that little ditty is that I would like to see us enabled the JSDoc requirement in ESLint and go back and clean up/doc all the old methods. I don't know if ESLint does this by default but my IDE gives me an error when I have a missing param or the JSDoc params don't match the function params so if there was a way to get that I think that would be really helpful.

[spencern]

@zenweasel good point re void vs undefined - void isn't a type, and so undefined should probably be what we use there.

I use atom as my editor and it also prompts me whenever I have missing or incorrect params, but it doesn't prompt me if I don't have a jsdoc block, so if ESLint could let us know all the places where they are missing, that could be nice too.

[mikemurray]

Your examples have a mixture of @param {object} and @param {Object}. I don't think JSDoc cares about the casing, but we should be consistent. Either Object, String, Number, etc, or object, string, number, etc.

[zenweasel]

Obviously it's a big project to go back and and doc all the existing methods but hopefully we can get to a place where PR's don't go in w/o JSDocs.

[spencern]

@mikemurray Good catch. I'd vote for using lowercase object, string, etc but definitely should be consistent.

[zenweasel]

Also, how do we handle custom types or are we locked into only the javascript primitives?

I would vote for uppercase types so that they match how they are used in code

[spencern]

jsdoc doesn't seem care what you put in for the type, so I don't think we're locked in.

[mikemurray]

@zenweasel The proper way is to define the type in JSDoc: http://usejsdoc.org/tags-typedef.html

Fixed ref to the zen man.

[zenweasel]

Let's not tag poor @brent anymore.

[kieckhafer]

I've come across a couple new @s that aren't mentioned above, which aren't used as widely as the rest.

@example is rarely used, only in 7 files
@summary is a more widely used, in 81 files

Both can be seen in action here: https://github.com/reactioncommerce/reaction/blob/master/lib/collections/schemas/helpers.js

Do we want to use these as well? Strip these out (@example at least, probably not @summary)?

[spencern]

@machikoyasuda Is @summary the same as having text at the top of the jsdoc block?

[machikoyasuda]

Hi all - Updated the jsdoc readme with all the things we decided on: https://github.com/reactioncommerce/reaction-jsdoc/blob/master/README.md

• lowercase type names (not capitalized)
• use return {undefined} (not void)

https://github.com/reactioncommerce/reaction-jsdoc/blob/master/README.md

[spencern]

@machikoyasuda Nice work getting this doc out! Super helpful.

One thing I saw is that the examples don't have all of the required fields - e.g. this example doesn't have a @summary

/**
 * Transforms a Shopify product into a Reaction product.
 * @private
 * @method createReactionProductFromShopifyProduct
 * @param  {object} options Options object 
 * @param  {object} options.shopifyProduct the Shopify product object
 * @param  {string} options.shopId The shopId we're importing for
 * @param  {[string]} options.hashtags An array of hashtags that should be attached to this product.
 * @return {object} An object that fits the `Product` schema
 *
 * @todo consider abstracting private Shopify import helpers into a helpers file
 */