microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.68k stars 12.35k forks source link

JSDoc @import _unintentional?_ passthrough to generated declarations. #58921

Closed typhonrt closed 3 weeks ago

typhonrt commented 2 months ago

Acknowledgement

Comment

With TS 5.5 and the new @import JSDoc tag it appears that TSC will passthrough the comment block containing @import to the generated declarations. The actual import statements are added, but the "unnecessary" comment block remains in the generated declarations. I've already discussed this with the maintainer of TypeDoc and a fix is put in there to ignore any @import comment blocks, but I figured it would be worth raising this as a potential issue with this new functionality. It's not broken, but can possibly be refined in respect to downstream tooling. I think it is fine to remove any JSDoc comment blocks that just contain @import tags from generated declarations.

I do have a full description of the scenario and the small inconvenience this can cause in downstream tooling via this TypeDoc issue, so do refer to that for an example.

I really appreciate @import though as it is a mighty fine feature!

a-tarasyuk commented 1 month ago

@DanielRosenwasser @weswigham Should @param tags be removed because they use types from the @import tag? Or should only the @import tags be removed?

/** @import Foo, { I } from "./a" */
/**
 * @param {Foo} a
 * @param {I} b
 */
export function foo(a: Foo, b: I): void;
import type Foo from "./a";
import type { I } from "./a";
DanielRosenwasser commented 1 month ago

Hm, I'm not sure - I'd have to defer to @weswigham. I would probably just keep them to reuse as much as possible.

typhonrt commented 1 month ago

The @param and anything else is important to keep for generating API docs from type declarations with say TypeDoc. This should be limited to just the @import JSDoc comment block and quite likely only if that comment block contains just @import statements.

a-tarasyuk commented 1 month ago

In this case, the type references will be unavailable. Is it ok? or should we convert the types to import() calls?

typhonrt commented 1 month ago

In this case, the type references will be unavailable. Is it ok? or should we convert the types to import() calls?

@a-tarasyuk Perhaps you can clarify this a little more. Things are working as expected except the passthrough of @import comment block is unnecessary.


In regard to @import it's a directive to generate the the import type ... statements in the declaration and is not semantically relevant beyond that. In the example given above the generated types is all fine and desired:

import type Foo from "./a";
import type { I } from "./a";

The actual JSDoc comment block has no real meaning to downstream tooling beyond TSC and the imports (Foo / I) are available.

Consider the case of bundling type declarations as well where the particular paths referenced from the @import tags are irrelevant in the case of local files in respect to the bundle. If the above file and others where references like ./a, ./some-path/b those references are meaningless in the bundled types. When bundling types import type Foo from "./a" will pull Foo into the bundle, so additional JSDoc comment blocks that reference @import Foo from './a' are not necessary to be in the bundled types.

weswigham commented 3 weeks ago

Hm, I'm not sure - I'd have to defer to @weswigham. I would probably just keep them to reuse as much as possible.

We generally keep the comments because they may contain extra text beyond what is semantically interpretable and generally don't hurt anything (they're comments!). There's a removeComments compiler option that does affect .d.ts emit that should strip all comments if you want that - but I don't think we really wanna get into what comments are and are not important to retain. Generally speaking, if it's a comment with a jsdoc style /** open, we strive to retain it in .d.ts emit since it may contain relevant documentation. If it's a bit duplicative, well... you can swap to TS source to reduce that duplication, or get your auto-doc generator to dedupe the information, as they already have 😆

So, yeah, for both @import and @param tags that end up generating declarations - expect them to stick around in the output most of the time, just because we don't wanna sort and filter the comments text to the semantic and non-semantic parts.