endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

@endo/eslint-plugin: something should be done w/ jsdoc/check-types #1755

Open boneskull opened 1 year ago

boneskull commented 1 year ago

I know someone put a lot of time into the issue templates, so I apologize that I'm not using them. It wasn't clear which one I should use.

TL;DR: plz disable jsdoc/check-types

I see warnings coming out of eslint-plugin-jsdoc of this variety:

/**
 * @typedef {Object} MyTypedef
 * @property {string} whatever
 */

causing:

Invalid JSDoc @typedef "MyTypedef" type "Object"; prefer "object"

I'm sure there's a reason for this (it appears to be the default behavior), but the plugin doesn't have enough context to make a helpful recommendation. This assumes that Endo even wants to let this plugin tell us there's something wrong with our types (instead of e.g., TypeScript itself).

I do think this is reasonable in JSDoc proper or closure-compiler-land, perhaps, but those are not applicable here. Or if they are, then I am just Wrong.

I kind of implicitly understood this before, but I have now explicitly tested my assumption. There is no difference between these typedefs, as far as TypeScript is concerned:

/**
 * @typedef {Object} MyTypedef
 * @property {string} whatever
 */

/**
 * @typedef MyTypedef
 * @property {string} whatever
 */

// this last one triggers a warning:

/**
 * @typedef {object} MyTypedef
 * @property {string} whatever
 */

Generally, it appears TS will (internally or otherwise) turn typedefs into interfaces when they "extend" object, Object, or nothing; everything else is a types. The above all compile to:

interface MyTypedef {
  whatever: string;
}

So in the above context, any warning from eslint-plugin-jsdoc is just hand-wringing, because it doesn't matter.

There's a distinction between object and Object in TypeScript, of course, which can be seen when using typedefs as type aliases:

// jsdoc won't complain about this

/**
 * @typedef {object} MyNotPrimitive
 */

// jsdoc will complain about this 

/**
 * @typedef {Object} MyActualObject
 */

The above compiles to:

type MyNotPrimitive = object;

type MyActualObject = Object;

...which is expected (object can be thought of as "not a primitive" and Object is more akin to the intrinsic--but that's about the limit of my understanding).

This distinction is also respected in function signatures; @param {Object} foo is not @param {object} foo; @returns {Object} is not @returns {object}.


All that above is to write that I don't trust the jsdoc/check-types rule and think it should be disabled. Failing that, it should not warn about Object since perhaps it's helpful otherwise.

boneskull commented 1 year ago

Also worth noting is that the plugin does not recognize tsdoc syntax. If Endo ever hopes to automate doc generation of its API, support would be highly desirable.