Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

intersection type: T & { … } not fully documented #2276

Closed
bugwelle opened this issue May 10, 2023 · 7 comments
Closed

intersection type: T & { … } not fully documented #2276

bugwelle opened this issue May 10, 2023 · 7 comments
Labels
enhancement Improved functionality
Milestone

Comments

@bugwelle
Copy link

bugwelle commented May 10, 2023

Search terms

intersection type

( I searched for "intersection type" before opening this issue; I found #1519 but that was closed as completed )

Expected Behavior

I would expect intersection types to be documented as well (their fields), i.e. the right-hand-side of typical intersections such as A & { … }.

Actual Behavior

Intersection type is not fully documented.

Steps to reproduce the bug

  • Install typedoc v0.24.7
  • Generate documentation for following snippet
export = project;

declare namespace project {
  /**
   * Options abc.
   */
  export type Options = {
    /**
     * My Flag
     */
    flag?: boolean
  }

  export type OtherOptions = Options & {
    /**
     * other flag
     */
    flag2: string | 'f' | 'g'
  }
}

Open documentation. You will see:

Options:
Screenshot 2023-05-10 at 10 11 23

OtherOptions:

Screenshot 2023-05-10 at 10 11 31

Environment

  • Typedoc version: 0.24.7
  • TypeScript version: Version 5.0.4
  • Node.js version: v18.16.0
  • OS: macOS 13.3.1
@bugwelle bugwelle added the bug Functionality does not match expectation label May 10, 2023
@Gerrit0 Gerrit0 added enhancement Improved functionality and removed bug Functionality does not match expectation labels May 15, 2023
@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 15, 2023

This is working as designed right now -- TypeDoc only recurses to show properties if the root type is an object type. The relevant code is in member.declaration.tsx.

I'm not sure what the right solution for this is yet. It certainly doesn't make sense to recursively show properties for every object type within an arbitrary type e.g. type Foo<T> = T extends { a: string } ? Something<{ b: string }> : never, but this case as well as a recent SO question are both simple enough that they probably ought to be recursed into and rendered.

It seems like it would be confusing to me to recurse if:

  • There are more than one object types to display
  • The object type isn't "roughly top level" Record<X, {}> yes, {}[] yes, T & {} yes, T | {} maybe? T extends {} no, SomeRandomGeneric<{}> maybe?

@bugwelle
Copy link
Author

Hi,

from my TS-noob perspective, I thought that even for something like { a: String } | { b: String } both sides would be rendered, i.e. all anonymous structures would be documented. And for Type | { … }, Type would be a link and the right-hand-side would be documented inline as well.

Of course, I don't know all the edge cases and I'm by far not experienced enough in TypeScript to say which way would be better.

But from a user's perspective (i.e. the one reading the documentation), I think it's strange to see a type whose properties are not documented in the rendered HTML. Would it be feasible to render the anonymous structure's properties if there is only one such occurrence? Or to render all, but add headings such as 1. Block to separate different properties of the same name? (e.g. { a: String } | { a: Integer })

My use case is:

type Config = {}
type SubConfig = Config | {}

Regards,
Andre

@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 17, 2023

Generally, if that much description of a type is necessary, I'd look at splitting it up - it's probably too complex of a type. TypeDoc's CommentDisplayPart type is an example of this, before it was split up it looked like this:

export type CommentDisplayPart =
    | { kind: "text"; text: string }
    | { kind: "code"; text: string }
    | { kind: "inline-tag";  tag: `@${string}`; text: string; target?: string | number | ReflectionSymbolId; };

Rendering all of the properties for this type twice would get really messy.. (potentially more than twice since the heading would probably have to render the type to indicate what object it's associated with)

There might be a good way to do this that I'm not seeing, UI design isn't my strong point, I tend to prefer just looking at declaration files for library documentation rather than looking at HTML rendered docs (at least for TypeScript, browsing header files is much worse than cppreference)

@bugwelle
Copy link
Author

I see. Personally, I'd rather see it all duplicated. After all, it's what the developer wrote. If developers want fewer details on a page, they should split it up (as proposed by you). In my case it's only a few properties that are disjunct. At the moment, I use interfaces for documentation, because there all properties are rendered with their description. I only use TypeDoc for documentation, so it's not an issue for me, but I thought types would fit better. :-)

@Oblarg
Copy link

Oblarg commented May 19, 2023

The whole type should be rendered, and there should be an inline tag for disabling rendering if there is a need to de-duplicate in the rendered docs.

Typically, though, rendered repetition is fine (even beneficial!) so long as it comes from a normalized canonical source. It's often useful not to have to "click through" to component types to know what's in a large composite.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 29, 2023

If someone wants to give implementing a implementing deeply nested rendering for object literals a shot, I'd be happy to take a look at it, but given my expertise lies far outside UI design, and I'm not seeing an obvious way to do this in a non-terrible way, for now I'm just going to go with:

{props.type?.visit<JSX.Children>({
    reflection: renderTypeDeclaration,
    array: (arr) => arr.elementType.visit(visitor),
    intersection: (int) => int.types.map((t) => t.visit(visitor)),
    union: (union) => union.types.map((t) => t.visit(visitor)),
    reference: (ref) => ref.typeArguments?.map((t) => t.visit(visitor)),
    tuple: (ref) => ref.elements.map((t) => t.visit(visitor)),
})}

(that is, render the properties of objects at the top level, or one level deep for arrays/intersections/unions/references/tuples)

@Oblarg
Copy link

Oblarg commented May 29, 2023

@Gerrit0 you may want to investigate piggybacking off of however vscode does its tooltip rendering, if that's possible. It deeply renders the type (often when you don't want it to!). Also, the current TypeDoc renderer seems to do deep rendering by default in type parameter constraint blocks, of all things...

In general I think exposing some level of user control over this is one of the main ways TypeDoc could be improved. I may start looking at PRing some improvements of my own in this direction - I think @expand or @noexpand tags could be very helpful here. Otherwise, you can end up with this...

image

@Gerrit0 Gerrit0 added this to the v0.24.8 milestone Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improved functionality
Projects
None yet
Development

No branches or pull requests

3 participants