microsoft / tsdoc

A doc comment standard for TypeScript
https://tsdoc.org/
MIT License
4.71k stars 131 forks source link

Discussion: How to document objects/interfaces in function arguments #246

Closed neefrehman closed 4 years ago

neefrehman commented 4 years ago

I'm currently trying to add TSDoc comments to a custom React hook that I'd like to release on npm. The only argument passed to the function is an optional config object which I'm destructuring. I can add a summary to the function fine, which works with IntelliSense, but when trying to document the properties of the config object I'm running into issues. I can't see the description when I mouseover them.

I can't see anything in the docs or online about doing this apart from this stackoverflow answer for JSdoc, which doesn't seem to work.

Below is my function signature. Paste it into the TSDoc playground to see what I mean. I've tried abstracting the object types to a separate interface with their own TSDoc comments, as well as using inline TSDoc comments above each line, and they still don't work.

I'd like users of my package to be able to see the descriptions of the config's properties as they type or mouseover them. What is the proper way to do this? This is a problem for any functions that have object parameters including React components

/**
 * A custom hook to use `requestAnimationFrame` inside a React component
 *
 * @param config - An optional configuration object for the hook
 * @param config.onStart - A callback that will be run once when the animation starts
 * @param config.onFrame - A callback that will be run on every frame of the animation
 * @param config.onEnd - A callback that will be run on once when the animation ends
 * @param config.delay - A delay in ms after which the animation will start
 * @param config.endAfter - A time in ms after which the animation will be stopped
 * @param config.fps - The desired fps that the animation will be throttled to
 *
 * @returns An object containing the current elapsedTime, frameCount and fps of the animation, as well as a function to stop the animation
 */
const useAnimationFrame = ({
    onStart,
    onFrame,
    onEnd,
    delay,
    endAfter,
    fps: throttledFps
}: {
    onStart?: () => void;
    onFrame?: () => void;
    onEnd?: () => void;
    delay?: number;
    endAfter?: number;
    fps?: number;
} = {}) => {
    let elapsedTime, frameCount, fps, endAnimation;

    // Do stuff

    return { elapsedTime, frameCount, fps, endAnimation };
};
octogonz commented 4 years ago

I would write it like this:

interface IUseAnimationFrameOptions {
    onStart?: () => void;
    onFrame?: () => void;
    onEnd?: () => void;
    delay?: number;
    endAfter?: number;
    fps?: number;
}

interface IUseAnimationFrameResult {
    elapsedTime: number;
    frameCount: number;
    fps: number;
    endAnimation: number;
};

function useAnimationFrame(options: IUseAnimationFrameOptions = {}): IUseAnimationFrameResult {
  // This is an implementation detail, so it seems strange to make it part of
  // your published API signature.
  const {
    onStart,
    onFrame,
    onEnd,
    delay,
    endAfter,
    fps: throttledFps
  } = options;

  return { elapsedTime, frameCount, fps, endAnimation };
}

For a published API contract, the top priority is to make it easy for other people to read. Sometimes this makes it a little more verbose to write.

That said the @param config.onStart notation is common enough that we should support it. I'll look into that when I get some time.

neefrehman commented 4 years ago

@octogonz thanks, you're right that's a better way to document the API. There's still a DX issue when it comes to descriptions however. When typing out the properties in the options parameter, I'm getting an autocomplete and type signature for each property, but no description of what that property does (just the description of IUseAnimationFrameOptions).

I'm wondering if it's possible to have the description of an object's properties also presented to the user, as if it were a regular function parameter? Initially I thought the @param config.onStart syntax was what would achieve this, which is why I didn't use interfaces.

Given the below code (or similar, as I don't think @param is right for interfaces) I'm looking to achieve something like this image when the user starts typing an options parameter or mouseovers them (which for the purposes of the image I've made onEnd the second parameter of the function).

edit: I can achieve this for autocomplete only with single-line TSDoc comments above each property in the interface.

image

/**
 * An optional configuration object for `useAnimationFrame`
 *
 * @param onStart - A callback that will be run once when the animation starts
 * @param onFrame - A callback that will be run on every frame of the animation
 * @param onEnd - A callback that will be run on once when the animation ends
 * @param delay - A delay in ms after which the animation will start
 * @param endAfter - A time in ms after which the animation will be stopped
 * @param fps - The desired fps that the animation will be throttled to
 */
interface IUseAnimationFrameOptions {
    onStart?: () => void;
    onFrame?: () => void;
    onEnd?: () => void;
    delay?: number;
    endAfter?: number;
    fps?: number;
}

/**
 * The returned object from `useAnimationFrame`
 *
 * @param elapsedTime - A callback that will be run once when the animation starts
 * @param frameCount - A callback that will be run on every frame of the animation
 * @param fps - A callback that will be run on once when the animation ends
 * @param endAnimation - A delay in ms after which the animation will start
 */
interface IUseAnimationFrameResult {
    elapsedTime: number;
    frameCount: number;
    fps: number;
    endAnimation: () => void;
};

/**
 * A custom hook to use `requestAnimationFrame` inside a React component
 *
 * @param options - An optional configuration object for the hook
 * @returns An object containing the current elapsedTime, frameCount and fps of the animation, as well as a function to stop the animation
 */
function useAnimationFrame(options: IUseAnimationFrameOptions = {}): IUseAnimationFrameResult {
  const {
    onStart,
    onFrame,
    onEnd,
    delay,
    endAfter,
    fps: throttledFps
  } = options;

  let elapsedTime, frameCount, fps, endAnimation;

  return { elapsedTime, frameCount, fps, endAnimation };
}
octogonz commented 4 years ago

I'm wondering if it's possible to have the description of an object's properties also presented to the user, as if it were a regular function parameter?

@neefrehman I had trouble understanding what you are asking for here.

The right way to document these parameters is like this:

/**
 * An optional configuration object for `useAnimationFrame`
 */
interface IUseAnimationFrameOptions {
    /**
     * A callback that will be run once when the animation starts
     */
    onStart?: () => void;
    /**
     * A callback that will be run on every frame of the animation
     */
    onFrame?: () => void;
    /**
     * A callback that will be run on once when the animation ends
     */
    onEnd?: () => void;
    /**
     * A delay in ms after which the animation will start
     */
    delay?: number;
    /**
     * A time in ms after which the animation will be stopped
     */
    endAfter?: number;
    /**
     * The desired fps that the animation will be throttled to
     */
    fps?: number;
}

If the IntelliSense does not show these descriptions as you're writing the code, that might rather be a problem with VS Code (or the TypeScript language service). TSDoc's responsibility is to document them, which seems to be accomplished by my example above (if I understand correctly).

neefrehman commented 4 years ago

Hi @octogonz, sorry I should have been clearer, I was looking for the correct way to document the parameters, yes. I'm now doing it like you say which gets me most of what I need.

In VSCode I still don't get the descriptions when destructuring documented parameters from an argument, however. Below is the tooltip I see when hovering over a parameter that has a description in its interface, and below that is the tooltip I see in the file where it's documented. I've resigned myself to the fact that this a VSCode issue instead.

image

image

octogonz commented 4 years ago

@neefrehman Without seeing a full code sample, I'm guessing that { width, height } in this expression is not actually typed as Canvas2DDrawProps. You seem to be relying heavily on type inference, so there may be some type algebra reason why the analyzer cannot safely apply the docs to width. (Or maybe it just isn't smart enough and will get improved in some future compiler release?)

I can repro something similar with this code sample:

interface IOptions {
    /** The width */
    width: number;
    /** The height */
    height: number;
}

type DistanceFunction = (options: IOptions) => number;

const getDistance: DistanceFunction = ({width, height}) => {
    return width + height;
};

I was able to fix it by eliminating the destructuring parameter:

const getDistance: DistanceFunction = (options) => {
    return options.width + options.height;
};

If you are targeting ES5 for web browsers, the transpiled output will look like this. It feels old fashioned to say this, but life often is easier if we avoid fancy syntaxes and write code using simple, explicit patterns. This philosophy applies especially to public APIs that need to be documented and consumed by an unfamiliar audience.

octogonz commented 4 years ago

I'm guessing that { width, height } in this expression is not actually typed as Canvas2DDrawProps.

I found a proof:

const getDistance: DistanceFunction = ({width}) => {
    return width;
};

The above code compiles without error, even though {width} does not fulfill the interface contract of IOptions, because it is missing the height member.

neefrehman commented 4 years ago

@octogonz Thanks for the insight! You're right, without destructuring the docs are picked up correctly by VSCode.

width and height are part of the Canvas2DDrawProps interface, which contains a bunch of other QoL variables and callbacks that can be passed down for drawing to the canvas. My current version to test on my own projects is here. Its the first library I'm hoping to release, so your notes on simple patterns are useful.