typedoc2md / typedoc-plugin-markdown

A plugin for TypeDoc that enables TypeScript API documentation to be generated in Markdown.
https://typedoc-plugin-markdown.org
MIT License
689 stars 172 forks source link

A class file is generated twice #603

Closed HarelM closed 2 months ago

HarelM commented 2 months ago

What package is the bug related to?

typedoc-plugin-markdown

Describe the issue

Using version 4, two of my classes are generated twice, I'm not sure why.

TypeDoc configuration

{
    "$schema": "https://typedoc.org/schema.json",
    "plugin": ["typedoc-plugin-missing-exports", "typedoc-plugin-markdown"],
    "groupOrder": [
        "Main",
        "Markers and Controls",
        "Geography and Geometry",
        "Handlers",
        "Sources",
        "Event Related"
    ],
    "out": "./docs/API",
    "excludeExternals": true,
    "excludeInternal": true,
    "placeInternalsInOwningModule": true,
    "excludeNotDocumented": true,
    "treatWarningsAsErrors": true,
    "entryPoints": ["./src/index.ts"],
    "intentionallyNotExported": ["CollisionBoxArray"],
    "navigation": {
        "includeCategories": false,
        "includeGroups": true
    },
    "name": "MapLibre GL JS",
    "githubPages": false,
    "hideBreadcrumbs": true,
    "hidePageHeader": true,
    "expandParameters": true
  }

Expected behavior

The class should be generated only once. The following are the classes that are generated twice: Marker, Popup.

My repo can be found here: https://github.com/maplibre/maplibre-gl-js

I've just tried to migrate to version 4.

HarelM commented 2 months ago

Not sure if this is related to this issue or not, let me know if you prefer that I open a different issue, but I found a return value that is also duplicated in one of those files:

image

The following is the code if it helps:

    /**
     * Adds the popup to a map.
     *
     * @param map - The MapLibre GL JS map to add the popup to.
     * @returns `this`
     * @example
     * ```ts
     * new Popup()
     *   .setLngLat([0, 0])
     *   .setHTML("<h1>Null Island</h1>")
     *   .addTo(map);
     * ```
     * @see [Display a popup](https://maplibre.org/maplibre-gl-js/docs/examples/popup/)
     * @see [Display a popup on hover](https://maplibre.org/maplibre-gl-js/docs/examples/popup-on-hover/)
     * @see [Display a popup on click](https://maplibre.org/maplibre-gl-js/docs/examples/popup-on-click/)
     * @see [Show polygon information on click](https://maplibre.org/maplibre-gl-js/docs/examples/polygon-popup-on-click/)
     */
    addTo(map: Map): this {
        if (this._map) this.remove();

        this._map = map;
HarelM commented 2 months ago

Actually, I now see the issue with the duplicate return type in the existing docs which uses an older version, so this apparently is not a new bug.

The duplicate file is a new bug though.

tgreyuk commented 2 months ago

@HarelM Thank you for the detailed description.

The duplicate return type i think is something else (its also happening on the HTML theme) and i'll look at that next.

Here are my findings on the duplicate Class files.

Issue

The root cause of the duplicate file appears to be a misuse of the @event tag on the Marker Class.

Screenshot 2024-05-03 at 22 52 23

What this appears to be doing is marking the Marker Class itself as an Event, which TypeDoc is then duplicating in an Events group.

Here is a screenshot (its using the HTML theme as it is easier for me to demonstrate):

Screenshot 2024-05-03 at 22 51 30

Fix

If you remove the @event tags then Marker is no longer duplicated into the Events group and the file is not duplicated.

Screenshot 2024-05-03 at 22 53 55 Screenshot 2024-05-03 at 22 53 45

The issue was hidden previously because the duplicate file was saving on top of the original.

tgreyuk commented 2 months ago

So the duplicate returns is also a comment issue. The @returns tag is not required here as the type is inferred. What is happening is that this is being printed as the return type description:

Screenshot 2024-05-03 at 23 30 18
HarelM commented 2 months ago

Thanks for the detailed responses! Should I move this issue to the typedoc repo? How should I document a class that raises events if not with the @event tag?

Also there are other return type that are inferred, is this issue relevant only for "this"? Or is this because the description for the return is "this" and I should write something better like "this - for chaining" it something?

Thanks again for the prompt response!

tgreyuk commented 2 months ago

Should I move this issue to the typedoc repo?

I don't think it is a bug because the @event tag needs to be used on the reflection that is being marked as the event. The interpretation of the tag is pretty basic at the moment in the sense that it simply groups the marked reflection in an 'Events' group. Arguably it should probably be ignored in this scenario though.

How should I document a class that raises events if not with the @event tag?

I think something like this (not tested) - you could create static properties on the Marker class.

Screenshot 2024-05-04 at 10 25 12 Screenshot 2024-05-04 at 10 25 28

You could even encourage use of the static property by consumers:

Screenshot 2024-05-04 at 10 27 28

This will correctly assign the events as grouped within the Marker class as follows (using HTML theme):

Screenshot 2024-05-04 at 10 42 10

If you don't want to do this then i think you would simply not use the @event tag as per my first example.

Also there are other return type that are inferred, is this issue relevant only for "this"? Or is this because the description for the return is "this" and I should write something better like "this - for chaining" it something?

No all types will be inferred from code and you only need to write a return description in the @return tag. Something like this:

Screenshot 2024-05-04 at 09 34 23 Screenshot 2024-05-04 at 09 36 53
HarelM commented 2 months ago

I see. Thanks a lot for all the suggestions! Feel free to close this issue.

tgreyuk commented 2 months ago

No probs at all! If you find anything else please do raise another issue. Closing this one.

HarelM commented 2 months ago

Thanks! I made the following PR to resolve all the issues: https://github.com/maplibre/maplibre-gl-js/pull/4075

As a side note, I couldn't find the "Example" text in the strings that one can replace in the options, it is intentional? https://typedoc-plugin-markdown.org/docs/options#textcontentmappings

Also the previous version set the example title with code style and not as a header. When I have two examples one after the other it creates this not-so-great table of content when using mkdocs:

See the right side table of content where it has example twice: image

Is there a way for me to improve that?

tgreyuk commented 2 months ago

As a side note, I couldn't find the "Example" text in the strings that one can replace in the options, it is intentional?

No - that was overlooked as it is generated from a tag - this can be added though. As an FYI I am not sure if the upcoming localization support will affect the recommended implementation for updating text string. https://github.com/TypeStrong/typedoc/blob/beta/internal-docs/internationalization.md.

Also the previous version set the example title with code style and not as a header. When I have two examples one after the other it creates this not-so-great table of content when using mkdocs:

Because the examples are already in code blocks you don't actually need to use the @example tag. You could just write @example1 and @example2.

However if you don't want the tags to be headers at all (and not in the toc) maybe we can introduce an option. Something like commentTagStyle: "heading" | "code" which would default to "heading" but can be set to "code" for same behaviour as v3. Let me know if you think that sounds useful?

HarelM commented 1 month ago

I'm not sure I want to use "custom" tsdoc tags such as "example1" as I would like to have the docs pass some linting, which might look for unknown tags. Also the IDE presents these comments, so I do want to keep it as standard as possible.

For me, it would be great if the examples would be grouped together under a "Examples" header in case of multiple examples in the same comment and "Example in case of a single comment.

I'm not sure how possible this is using this plugin, but I think it will achieve the best user experience.

Another option is to keep the same level of header for the method example (currently ####) as the class example (currently ##), this will keep things looking the same.

You can see the different values in the following output of marker.ts class here:

Class:

# Marker

Creates a marker component

## Example

```ts
let marker = new Marker()
  .setLngLat([30.5, 50.5])
  .addTo(map);
``

## Example

Set options
``ts
let marker = new Marker({
    color: "#FFFFFF",
    draggable: true
  }).setLngLat([30.5, 50.5])
  .addTo(map);
``

## See

 - [Add custom icons with Markers](https://maplibre.org/maplibre-gl-js/docs/examples/custom-marker-icons/)
 - [Create a draggable Marker](https://maplibre.org/maplibre-gl-js/docs/examples/drag-a-marker/)

## Events

**Event** `dragstart` of type [Event](Event.md) will be fired when dragging starts.

**Event** `drag` of type [Event](Event.md) will be fired while dragging.

**Event** `dragend` of type [Event](Event.md) will be fired when the marker is finished being dragged.

## Extends

- [`Evented`](Evented.md)

Method:

## Methods

### addClassName()

> **addClassName**(`className`: `string`): `void`

Adds a CSS class to the marker element.

#### Parameters

| Parameter | Type | Description |
| :------ | :------ | :------ |
| `className` | `string` | on-empty string with CSS class name to add to marker element |

#### Returns

`void`

#### Example

``
let marker = new Marker()
marker.addClassName('some-class')
``

#### Source

[src/ui/marker.ts:641](https://github.com/maplibre/maplibre-gl-js/blob/96781701ea7763b2ef6a0d26526463fbfad020e2/src/ui/marker.ts#L641)

***
tgreyuk commented 1 month ago

For me, it would be great if the examples would be grouped together under a "Examples" header in case of multiple examples in the same comment and "Example in case of a single comment.

Thats not a bad idea. We could achieve the following with multiple @example tags:

Screenshot 2024-05-06 at 13 11 29 Screenshot 2024-05-06 at 13 11 21
HarelM commented 1 month ago

Looks good!