Standardize JSDoc for TypeScript function parameters

[victorlin]

continuing conversation from #1124 (comment)

There are two approaches to add JSDoc descriptions for TypeScript function parameters.

1. In the parameter type definition's JSDoc block. Example:

   nextstrain.org/static-site/app/blog/parseMarkdown.ts

   Lines 6 to 18 in 2998b80

   	export default async function parseMarkdown({
   	mdString,
   	addHeadingAnchors = false,
   	headingAnchorClass = undefined,
   	}: {
   	mdString: string
   	/** Should `<a>` tags be added to headings? */
   	addHeadingAnchors?: boolean
   	/** Class name for heading anchors (from page-specific CSS Module) */
   	headingAnchorClass?: string
   	}): Promise<string> {

   Benefits:

   • Description appears on hover of the parameter
   • Keeps the variable name, type, and description all in one place with no duplication

2. In the function's JSDoc block using @param. Example:

   nextstrain.org/static-site/components/list-resources/index.tsx

   Lines 131 to 138 in 2998b80

   	* @param versioned - boolean for whether the resources are versioned
   	* @param elWidth - width of the element
   	* @param quickLinks - list of `QuickLink` objects for the resources
   	* @param defaultGroupLinks - boolean for if the group name is a url
   	* @param groupDisplayNames - apping from group name -> display name
   	* @param tileData - the tiles for the resources
   	*/
   	function ListResourcesContent({

   Benefits:

   • Description appears on hover of the function

Tasks

• Settle on a single approach as the standard
• Replace existing usage of the other approach
• Document this standard (maybe as part of Standardize TypeScript usage auspice#1890, which should apply not just to Auspice but all TypeScript usage under @nextstrain)

[victorlin]

My vote is for (1) but I introduced that pattern and am biased. Please add any other benefits or different approaches that I'm not aware of.

[jameshadfield]

I vote for (1). I don't remember if I read official advice or just followed my instincts, but (2) is going to result in duplication of parameter descriptions cf. (1). Note that the way (2) is written is wrong, since it's a single destructured parameter, and I find JSDoc quite clunky in this respect. Put another way, I wouldn't push for actually using fully-complete JSDoc anywhere we have types, but I'd use the /** ... */ JSDoc-style comments throughout so that LSPs can pick them up.

[genehack]

Victor and I discussed this in our 1:1 yesterday (19 March), and we are settled on version 1.

• I am going to update Convert staging to app router [#1067] #1131 to convert the JSDoc in that PR to this format
• I will go back and make a cleanup PR that converts all the existing JSDoc comment blocks under /static-site/app/ and static-site/components/ over to this format; I will also add JSDoc comments to any function/method/param/class that doesn't already have them. (This PR will also be where the behavioral change for /pathogens discussed in this comment will be done, I imagine.)
• I will NOT correct or add any JSDoc to the current Page Router-based pages and components, because when Convert static-site from Pages Router to App Router #1052 is completed, none of those files will be present

I will be applying the corrections to #1131 pretty soon (ideally later today), and will plan on rolling on into the second and third items above immediately afterward, on top of the #1131 PR. If anybody thinks this is wrong, or disagrees, please holler quickly. 😁

[victorlin]

In practice, the recommendation to add JSDoc "at the source" of function parameters can take on many different forms. Here's the common ones:

1. Multiple parameters (example)

   function area(
     /** width */
     x: number,
   
     /** height */
     y: number,
   ): number

2. Single parameter, inlined type (example)

   function area({
     x,
     y,
   }: {
     /** width */
     x: number
   
     /** height */
     y: number
   }): number

3. Single parameter, external type definition (example)

   interface Dimensions {
     /** width */
     x: number
   
     /** height */
     y: number
   }
   
   function area({
     x,
     y,
   }: Dimensions): number

[genehack]

With this merge, I think the work described in this issue is complete, so I'm closing it out.