[feat] Link Component

[haworku]

Make a basic Link component that wraps a link in the classes we need for an internal link usa-link versus and external link usa-link usa-link--external.

This is another way to encapsulate styles. See Typography > Links.

By default renders an anchor tag, but has an optional prop to accept a different component.

[suzubara]

I like this idea, but I think the rendered component should be customizable somehow in case the user is using client-side routing and needs more than a basic <a> tag.

[haworku]

We could use createElement similar to the work being done for #161

[haworku]

We need further discussion of what kind of user/developer experience we want for handling the customizable component.

Requirements

• We want to be able to pass in a different base element to the link and apply styles to it.
• We want a clear prop interface (if props are conflicting we should have an agreed upon approach that we communicate through warnings or type/prop errors).
• We don't want to lose type safety. Anchor tag links should still warn if the user doesn't pass in required attributes such as href (required for <a/>) or children (required by jsx-a11y for anchor tags).

Do these requirements seem correct??

Also, here are two approaches for consideration:

1. createElement feat: Link Component #161 #309
   Use asCustom component prop to pass a custom component definition.
   

   react-uswds/src/components/Link/Link.stories.tsx

   Lines 75 to 80 in 12e857f

   	<Link<MockLinkProps>
   	className="abc"
   	asCustom={CustomLink}
   	to="http://www.truss.works">
   	This
   	</Link>

2. cloneElement [WIP] Link component with cloneElement #285
   Pass custom component as a React.Element child to Link. Use asCustomComponent boolean prop.
   

   react-uswds/src/components/Link/Link.stories.tsx

   Lines 58 to 63 in 0adac0a

   	<Link asCustomComponent>
   	<a data-testid="customComponent" href="http://www.truss.works">
   	This
   	</a>
   	</Link>
   	&nbsp;is a custom component link.

[ahobson]

I should be clear that after seeing the solution proposed by @haworku I like it way better than the one I worked on.

[suzubara]

okay I’m looking at this a little bit and I have some thoughts/questions:

1. I prefer the syntax of the first option (<Link<MockLinkProps asCustom={CustomLink}...). Having to pass in the custom component as a child feels a little confusing, since the children end up as the returned node instead of its contents.

2. I don't see why in the first option, we aren't adding the USWDS classes at all if it's a custom component. In that case there doesn't seem to be any reason to use it. I saw this comment:

   The custom FCProps might have a variant that has a different meaning in the custom component, so we have to leave styling up to the custom component

   and actually I disagree, I think our styling should still take precedent because that is really the only value our Link component provides at all. If someone is using it but has their own variant prop or wants to overwrite with their own styling then I think they should just not use this component.

3. It seems like there are a few different questions being asked and it's not clear to me that any of these decisions are bound together (but please correct me if I'm wrong...):

   • syntax to use for passing in custom component (asCustom prop vs. asCustomComponent and children)
   • whether to use createElement or cloneElement -- I will add there's a third option, which is:

     const Component = asCustom
     return <Component {...linkProps}>{children}</Component>
     

   • how to handle conflicting props if passing in a custom component

Ultimately I would keep in mind two things:

• I see the primary use case of this feature as if someone's using a routing library like react-router and needs to be rendering their Link component instead of plain ol' a tags. I would focus on that and make use of the example app, which already has react-router set up, to demonstrate test cases and see how the different options feel in a more real life scenario than Storybook can provide.
• Just because someone is using this library for USWDS styles doesn't mean they are forced to use our Link component. So I wouldn't worry so much about making it bulletproof, and instead focus on the convenience it provides over making it work for every single scenario.

[haworku]

Thanks for the response and pointing out the separate concerns! Very helpful.

Syntax - syntax could change for passing in the custom component. We could use cloneElement with this syntax(<Link component={<CustomLink />} /> instead. I reverted a commit that used children to instead show what a component prop would look like on the original PR.

Conflicting Props - yes this this the main question as well as the the prop interface on Link. createElement (or the const Component = asCustom example) require expanding the acceptable props for <Link />. Maybe this isn't a big deal but the are potential conflicts with that (now and in future) and losing type safety concern me. variant isn't that rare of a prop name. I saw some libraries added warnings to make sure people use the props accordingly and address conflicting types. That gets to issue 2.

I hear you saying that person shouldn't use our component.

cloneElement versus createElement In general I don't see the advantage of using createElement for our use case (just adding styles to an element). I'm assuming its because people find it familiar? To me it seems like an approach that seems more risky because of the unknowns with expanding props and having to (maybe) check what they are passing down. But it seems like I might be alone in that. Likely its because I'm not familiar with this pattern.

[ahobson]

After futzing for a bit, I really don't think it's possible for us to determine if their custom component also uses variant and so I don't think we could warn them. This would only come into play if their custom component property variant is a string type. We can document the limitation and live with it, I guess.

[haworku]

@ahobson do you want to rework #309 to handle styles also?? We can rely on typescript to warn about invalid props and just add a comment in the component file of what it supported.

It should be quick - I suggest looking at the earlier version of my PR last week - it handles everything with styles and also uses createElement. It just had messed up typescript because I couldn't figure out typing the way that you have 😄

[ahobson]

@haworku I just pushed up a change to #309 that I believe does what we want.

[haworku]

This shipped in #320