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

Nested parameter not rendered when top parameter is optional #1020

Closed
1 task done
maoueh opened this issue Apr 20, 2019 · 17 comments
Closed
1 task done

Nested parameter not rendered when top parameter is optional #1020

maoueh opened this issue Apr 20, 2019 · 17 comments
Labels
bug Functionality does not match expectation

Comments

@maoueh
Copy link

maoueh commented Apr 20, 2019

Expected Behavior

This snippet should render the nested parameter value correctly in the HTML output:

/**
 * @param options RootParameter
 * @param options.under NestedParameter
 */
export function under(options?: {under: boolean}) {}

More Information

The bug is caused because in strict mode, this snippet get's rendered as an union type of the form { ... } | undefined. It seems typedoc will not render the nested parameters in this case.

This test case fails also even when strict: false:

/**
 * @param options RootParameter
 * @param options.under NestedParameter
 */
export function under(options: {under: boolean} | undefined) {}

Would you accept a PR that cover this cases (I would extend it to null/both cases also ... | null, ... | undefined, ... | null | undefined).

Actual Behavior

The nested parameter should be rendered.

Steps to reproduce the bug

See https://github.com/maoueh/typedoc-bug-1020-repro

  • Define the snippet above in a file.
  • Ensure strict: true (this generates a union type from the parser point of view)
  • Inspect HTML, output is not rendered correctly.

Environment

  • Typedoc version: 0.14.2
@maoueh maoueh added the bug Functionality does not match expectation label Apr 20, 2019
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Apr 20, 2019

That makes a lot of sense why nested keys aren't rendered when an argument is optional.

I definitely think it makes sense to render nested keys when an argument is marked as optional with ?. When the argument is explicitly unioned with null or undefined it might also make sense but I'd like to hear @aciccarello's thoughts on that behavior.

@maoueh
Copy link
Author

maoueh commented Apr 21, 2019

@Gerrit0 Is it even possible to discriminate between options?: {} and options: {} | undefined, seems the former being a sugar syntax for the latter, at least the --json output.json seems to imply that as options?: {} is being seen as this from the parser, which is also the same output when undefined | {}:

type: union,
types: [
  {
    "type": "intrinsic",
    "name": "undefined"
  },
  {
    "type": "reflection",
    "declaration": {
        ...
    }
  }
]

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Apr 21, 2019

Purely from the JSON output, I guess it isn't possible. This seems like an oversight to me. options?: {} isn't quite sugar for options: {} | undefined as the latter requires explicitly passing undefined if the argument is not used.

@maoueh
Copy link
Author

maoueh commented Apr 22, 2019

You are right indeed, did not think about this, they are not the same in this optic.

It's really a blocker for me and I would like to make the fix. For now, I've skim between converter code and default theme doing the final rendering, still looking for the place where an object parameter is being converted to nested field, I guess there might be a recursive call somewhere.

Do you have some hints where I should look for in my hunt for a fix?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Apr 22, 2019

I'm not very familiar with how the theme works, it's been on my list to read through for a while. Parameters are rendered here.

@SagnikPradhan
Copy link

Any updates? Any fixes?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 15, 2019

I suspect #1103 may fix this, but I haven't had the time to dig into it yet.

@SagnikPradhan
Copy link

I suspect #1103 may fix this, but I haven't had the time to dig into it yet.

Yeah, It works now.

@SagnikPradhan
Copy link

But seems like the descriptions of nested parameters are not showing up.
Capture

/**ts
 * @param array Array
 * @param search Search Elem
 * @param opts Options
 * @param opts.start Start Index of Array
 * @param opts.end End Index of Array
 * @param opts.greaterFn Compare if first value is greater than second value
 * @param opts.equalFn Compare if two values are equal
 */
export function binarySearch<T>(
    array: T[],
    search: T,
    {
      start,
      end,
      greaterFn,
      equalFn,
    }: {
      start?: number;
      end?: number;
      greaterFn?: (a: T, b: T) => boolean;
      equalFn?: (a: T, b: T) => boolean;
    } = {
      start: 0,
      end: array.length - 1,
      greaterFn: (a, b): boolean => a > b,
      equalFn: (a, b): boolean => a === b,
    }
): number {
    // ...
}

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 19, 2019

 * @param opts Options

TypeDoc doesn't know what to do with this ^

You don't have any parameters called opts. You should instead name the param according to what is used in the function, so

 * @param start Start Index of Array

@SagnikPradhan
Copy link

But start is inside a Object right?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Oct 21, 2019

Yes, but since you destructed the parameters so that it is available as start (and not opts.start) within the function, that is the parameter name recognized by TypeDoc.

@omahili
Copy link

omahili commented Jan 13, 2020

Unfortunately it seems to happen with classes and interfaces too. Trying with v0.16.1, this code:

/**
 * Lorem ipsum
 */
class Foo {
    /**
     * Lorem ipsum
     */
    public bar?: {
        /**
         * Lorem ipsum
         */
        foobar: string;
    };
}

produces the same bug and foobar is not rendered.

bar

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 13, 2020

That's unfortunate :/ I thought the changes to exports would have fixed that.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 14, 2020

Fixed in [email protected]. If you install without a lockfile it should be picked up, or v0.16.3 (out later today) will pull this in.

Turns out the information to render everything was already there (partially due to the changes done to exports), but the themes weren't updated to support it.

If you are using a custom theme that has changed the type.hbs helper, it will need to pull in changes from [email protected] to get this.


Rendering is tricky. There are cases when dealing with unions/intersections, conditional types, and function types where you have to be very careful to avoid generating an invalid type. I think I've caught all the edge cases, but it's possible I missed one.

@Gerrit0 Gerrit0 closed this as completed Jan 14, 2020
@omahili
Copy link

omahili commented Jan 16, 2020

Thank you @Gerrit0 for addressing this issue.
Although I still find some issues with it, since the type declaration is missing. Is it wanted?

For the same example above now it renders as:

foo_bar

Since foobar is documented, shouldn't "Type declaration" be rendered below? Who looks at the documentation knows that bar can be an object with a foobar property, but doesn't know what it is about.

This happens with nested properties too, for example:

/**
 * Lorem ipsum
 */
class Foo {
    /**
     * Lorem ipsum
     */
    public bar: {
        /**
         * Lorem ipsum
         */
        foobar?: {
            /**
             * Lorem ipsum
             */
            baz: string;
        };
    };

    //...
}

Render as:

baz

So baz isn't documented.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Jan 16, 2020

That could certainly use some improvement, feel free to open a new issue :)

I really want to completely rebuild the default theme. I think there is some major room for improvement in rendering the types in an easily understandable way, the nested lists used right now work, but get unwieldy... Unfortunately I don't think I'll get to this for a while...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Functionality does not match expectation
Projects
None yet
Development

No branches or pull requests

4 participants