tmcw / dx-spec

Issue (and spec) repository for the development of a spec that would succeed JSDoc.
27 stars 0 forks source link

Params #22

Open jamiebuilds opened 6 years ago

jamiebuilds commented 6 years ago

The more I look at it the more I don't like the syntax for documenting params:

// - param: `a` `number` - description
// - param: `b` `string` - description
function foo(a, b) {
  // ...
}

Especially once you are using Flow/TypeScript:

// - param: `a` - description
// - param: `b` - description
function foo(a: number, b: string) {
  // ...
}

I think I would prefer doing this:

function foo(
  a, // `number` - description
  b, // `string` - description
) {
  // ...
}
function foo(
  a: number, // description
  b: string, // description
) {
  // ...
}
jamiebuilds commented 6 years ago

Other examples:

function foo(
  opts: { // Options
    a: number, // description
    b: string, // description
  },
) {
  // ...
}
function foo(
  opts /*: { // Options
    a: number, // description
    b: string, // description
  } */,
) {
  // ...
}
tmcw commented 6 years ago

I'm into it - some of the intent for the JSDoc style of params-as-comments-above-the-thing is that you could document parameters that weren't really real, like

// @param {numbers...} a buncha numbers
function add() {
  var sum = 0;
  for (var i = 0; i < arguments.length; i++) sum += arguments[i];
  return sum;
}

From what I've seen, people did take advantage of this flexibility to have documentation document the intended API, not what the code really says literally. But we're taking a very different approach, and one that seems more sustainable. Inline docs make sense.

jamiebuilds commented 6 years ago

We should probably have some amount of consistency about how comments are attached to nodes.

function foo(
  opts /*: { // Options
    a: number, // description
    b: string, // description
  } */,
) {
  // ...
}

With this syntax the // description in a: number, // description is actually attached to the object property b.

Basing it off of location rather than node attachment makes enough sense to me.

anandthakker commented 6 years ago

@thejameskyle can you say more about what you don't like about being able to document params in the outer function comment?

Asking because I definitely dig the more 'inline' styles you're proposing here, with description comments colocated with the parameter, for many use cases. But something in me balks at the notion that the only way to document individual params would be to break up a function signature into multiple lines -- especially when one of the params requires a long explanation.

Here's an example from Mapbox GL JS:

    /**
     * Returns an array of [GeoJSON](http://geojson.org/)
     * [Feature objects](http://geojson.org/geojson-spec.html#feature-objects)
     * representing visible features that satisfy the query parameters.
     *
     * @param {PointLike|Array<PointLike>} [geometry] - The geometry of the query region:
     * either a single point or southwest and northeast points describing a bounding box.
     * Omitting this parameter (i.e. calling {@link Map#queryRenderedFeatures} with zero arguments,
     * or with only a `options` argument) is equivalent to passing a bounding box encompassing the entire
     * map viewport.
     * @param {Object} [options]
     * @param {Array<string>} [options.layers] An array of style layer IDs for the query to inspect.
     *   Only features within these layers will be returned. If this parameter is undefined, all layers will be checked.
     * @param {Array} [options.filter] A [filter](https://www.mapbox.com/mapbox-gl-style-spec/#types-filter)
     *   to limit query results.
     *
     * @returns {Array<Object>} An array of [GeoJSON](http://geojson.org/)
     * [feature objects](http://geojson.org/geojson-spec.html#feature-objects).
     * ... (more explanation, examples, etc.)
     */
    queryRenderedFeatures(geometry?: PointLike | [PointLike, PointLike], options?: Object) {

If param docs had to be inline, this would be something like:

     /* Returns an array of [GeoJSON](http://geojson.org/)[Feature objects](http://geojson.org/geojson-spec.html#feature-objects) representing
visible features that satisfy the query parameters. */
   queryRenderedFeatures(
      geometry?: PointLike | [PointLike, PointLike], /* The geometry of the query region: either
                a single point or southwest and northeast points describing a bounding box.
                Omitting this parameter (i.e. calling {@link Map#queryRenderedFeatures} with
                zero arguments, or with only a `options` argument) is equivalent to passing a
                bounding box encompassing the entire map viewport. */
     options: {
         filter?: Array, /* A [filter](https://www.mapbox.com/mapbox-gl-style-spec/#types-filter) to
                limit query results. */
         layers?: Array<string> /* An array of style layer IDs for the query to inspect. Only features 
                within these layers will be returned. If this parameter is undefined,
                all layers will be checked. */
     }
   ) {

This actually looks more palatable than I expected when I started typing, but I still think that in cases like this, it could be more readable to have the long, detailed parameter documentation up in the main function comment. (Admittedly, I'm not sure there's a good way to document the members of options in the main function comment -- maybe https://github.com/tmcw/dx-spec/issues/21#issuecomment-346910796.)