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

react forwardRef shows up as namespace *and* function. #2461

Closed
RobMayer opened this issue Dec 13, 2023 · 6 comments
Closed

react forwardRef shows up as namespace *and* function. #2461

RobMayer opened this issue Dec 13, 2023 · 6 comments
Labels
bug Functionality does not match expectation no bug This is expected behavior

Comments

@RobMayer
Copy link

Search terms

react forwardref namespace function kind

Expected Behavior

exporting a forwardRef component should show up once in the generated docs, as a function, consistent with any other functional component.

Actual Behavior

typedoc has any forwardRef component showing up as both a namespace and a function

Steps to reproduce the bug

/**
* @function
* @group Component
*/
export const MyComp = forwardRef((props: { children?: ReactNode }, ref: ForwardedRef<any>) => {
   return <>{children}</>
});

Environment

  • Typedoc version: 0.25.4
  • TypeScript version: 5.0.4
  • Node.js version: 20.6.1
  • OS: win10
@RobMayer RobMayer added the bug Functionality does not match expectation label Dec 13, 2023
@RobMayer
Copy link
Author

for what it's worth, I was able to subvert this by forcing the type on export with an 'as' .

I think I understand why it happens: React's additional properties like displayName, propTypes, defaultProps, etc, makes a function into an object, and thus a namespace according to typedoc. things like React.memo and React.forwardRef explicitly export that object-function instead of a simple component-shaped-function that one usually exports.

would this classify as a "working as intended" or is there a more elegant way to handle this particular react side-effect?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Dec 18, 2023

It does count as working as intended right now, though looking back at #2436, I'm not entirely sure why I decided to make a namespace in this case rather than putting the property declarations on the function. Consistency is nice, but in this case, and the Object.assign case from that issue, I think it makes more sense to treat these as function properties than creating a separate namespace.

@pjeby
Copy link

pjeby commented Dec 20, 2023

Would function properties appear in the index, then?

I ask because I'm working on a project where certain functions have properties and methods; e.g. there is an effect() function with effect.root() and effect.scheduler(). This results in both a namespace and a function, as described by this issue. I would actually like to unify the namespace and the function, but would at least somewhat prefer to still be able to see the properties in the index. (But maybe it would be fine as long as the properties are sufficiently prominent on the function's page?)

Slightly more complex: I have a callable const that actually implements an interface with overloads. This is currently expanded into a function and namespace in the docs, but if I try to document the const directly in any way, I lose all the overload-specific documentation from the interface. (And if I don't document the const, I can't put it in a category.) Instead of documented properties + overloads, I get the constant's documentation repeated for each overload, and then the rest of the interface is represented by a namespace.

Playing around with the fix for #2436, it looks like directing the convertSymbols() to apply to the current reflection instead of a new namespace declaration, causes Object.assign-style properties to be added to an "index" section on the function page, but not to the on-page table of contents. It also works for interfaces, in that the interface's methods get added to the index, but it doesn't fix the inability to keep the overloads' documentation if you document the export.

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Dec 27, 2023

What do you mean by "in the index"? The change that you made is what I was considering, with a few other adjustments to handle some more edge cases. This results in both properties being shown in the "On this page" section, and the "index", but not under the full site navigation tree as functions are not expanded.

image

This makes me want to add more options for configuring the navigation tree...


The const is a totally different issue -- and a way worse one. Overloads are awful, overloads without overload syntax (e.g. variables rather than multiple function declarations) are even worse. There is only one declaration, so unless you repeat the type, there's only one place to add the comment, so TypeDoc applies it to all signatures.

const x: {
  /** Docs for signature 1 */
   (): string;
  /** Docs for signature 2 */
   (x: number): number;
} = ...

In this case, since you're just adding a category, we're in luck, as the inline form of @inheritDoc can point to the interface, which will copy the descriptions from the signatures while allowing us to set the category.

export interface Int {
    /** Docs for string returning signature */
    (): string;
    /** Docs for number returning signature */
    (x: string): number;
}

/**
 * {@inheritDoc Int}
 * @category Foo
 */
export const foo: Int = () => {
    throw 1;
};

@Gerrit0 Gerrit0 added the no bug This is expected behavior label Dec 27, 2023
@pjeby
Copy link

pjeby commented Dec 28, 2023

By "in the index" I meant that on the main index (modules.html) there would be entries for the effect.root and effect.scheduler functions, right after effect itself.

In practice the issue has largely been mooted for me at this point; I gave up on getting Typedoc to match the project and instead made the project match Typedoc, i.e. I got rid of the namespaces/function "methods".

(Of course, after doing that, I ended up spending enough time hacking on the Typedoc default theme figuring out how to get the markdown pages plugin working with the new navigation system, that if I really needed to I'm pretty sure I could make the navigation work whatever way I wanted now, but I like the namespace-free version of my API better so I'm probably not going back. 😁)

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Dec 28, 2023

Ah, that index doesn't include anything nested. I tend to also prefer namespace-free APIs

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 no bug This is expected behavior
Projects
None yet
Development

No branches or pull requests

3 participants