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

Function components look weird in rustdoc #2144

Closed
hamza1311 opened this issue Nov 13, 2021 · 3 comments · Fixed by #2478
Closed

Function components look weird in rustdoc #2144

hamza1311 opened this issue Nov 13, 2021 · 3 comments · Fixed by #2478
Labels
Milestone

Comments

@hamza1311
Copy link
Member

hamza1311 commented Nov 13, 2021

The Problem

The #[function_component(_)] macro expands into two items:

  1. a struct which implements FunctionProvider
  2. a type definition of FunctionComponent<the_function_struct>

The struct is marked as #[doc(hidden)] and for a good reasons:

  • It gets no documentation
  • It's name doesn't follow conventions
  • It isn't supposed to be used directly

But in rustdoc, the type definition looks weird. For reference, this is how BrowserRouter looks:
image

Solution

Make the #[function_component] macro add a #[doc = "..."] attribute which adds a note about the type of props (and links them)

// other expanded code

#[...attributes from code expansion]
#[doc = "# Props"]
#[doc = "This component takes [`ThePropsIdent`] as props."]
type TheComponent = ...;

Originally proposed solution (here for history)

Only generate one struct which implements FunctionProvider and provide a blanket implement of Component for any type that implements FunctionProvider. This would be easy to implement but right now, FunctionComponent<T: FunctionProvider> keeps it's own state which isn't possible to hold without our own struct.

pub struct FunctionComponent<T: FunctionProvider + 'static> {
_never: std::marker::PhantomData<T>,
hook_state: RefCell<HookState>,
message_queue: MsgQueue,
}

hook_state and message_queue need to somewhere else but where?
A thread_local might be enough with anymap2 but I'm not sure if it's the best solution. But I also don't know of any other solutions.

@hamza1311 hamza1311 added the A-yew Area: The main yew crate label Nov 20, 2021
@WorldSEnder
Copy link
Member

While I agree that the docs look a bit weird, I don't find this issue bad enough to introduce even more weirdness in the implementation details. Tying the hook_state into a thread_local anymap, or even a blanket impl are not what I'd call "light weight" solutions.

For any consumer of the docs, I think it sufficiently documents the behaviour of the alias. Who really looks at the definition anway, to discover the details of the hidden browser_router? More likely jumping to FunctionComponent and the defined component itself is sufficient. Maybe in a future version of rustdoc the right hand side can be partially hidden.

In any case, as long as clicking [src] jumps to the function definition, everything looks okay to me.

@voidpumpkin
Copy link
Member

voidpumpkin commented Nov 21, 2021

I actually don't think this documentation is all that bad.
The main thing i want from function component docs is what props does it accept and that can be found on the left:
image

Its sad that we cant know what props are optional, but i would find that out via ctrl + click 'ing until i get to props definition in code.
image

@hamza1311 I would dare to argue that we can close this issue

@hamza1311
Copy link
Member Author

Since props are the only thing missing, we could make the function_component add the props type in docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants