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

[v6] Link no longer supports component prop #7598

Closed
hugmanrique opened this issue Sep 4, 2020 · 6 comments
Closed

[v6] Link no longer supports component prop #7598

hugmanrique opened this issue Sep 4, 2020 · 6 comments

Comments

@hugmanrique
Copy link

hugmanrique commented Sep 4, 2020

Version

6.0.0-beta.0

Test Case

On RR v5, the Link component supported a component prop that specified the element to render the link with. It defaulted to a.
Many use component libraries that provide styled anchor links with their own styles, but now there's no way to replace the a element.

See https://github.com/ReactTraining/react-router/blob/81a5079bf90da0741a61001297926113b112b1d4/packages/react-router-dom/index.tsx#L256.

The migration guide suggests component was renamed to as, but that doesn't work either.

Alternatives include making the styled components wrap an anchor child, but:

  1. It's often not an option when a UI library is external or unmaintaned;
  2. It unnecessarily polutes the DOM and our code.

Steps to reproduce

import { Link } from 'react-router-dom';

function Foo(props) {
    return <a {...props} className="foo" />;
}

function App() {
    // TODO Setup BrowserRouter & Routes
    return <Link to="bar" component={Foo}>Something</a>;
}

Expected Behavior

<a href="bar" class="foo">Something</a> is rendered to the DOM.

Actual Behavior

<a href="bar" component="[object Object]">Something</a> is rendered instead.

TS now errors because LinkProps doesn't contain a component prop.

@timdorr
Copy link
Member

timdorr commented Sep 6, 2020

#7409 was working on this.

@hugmanrique
Copy link
Author

hugmanrique commented Sep 6, 2020

Thanks for the link! This issue gives a more convincing argument for re-adding the as prop. I'm still rendering a totally accessible <a> link to the DOM, my wrapper component just adds some extra classes to style it. The same applies to the style, target="_blank", etc. props.

In my case, I cannot directly modify the Link component I'm passing to the as prop as it's provided by an external library. Yes, I can create yet another Link wrapper that uses useHref and useNavigate, but imo 1. it's not scalable to large codebases with multiple link styles and 2. relies on react-router users making the Link component accessible when react-router already does a great job at this.

@stale stale bot added the stale label Nov 6, 2020
@hugmanrique

This comment has been minimized.

@stale stale bot removed the stale label Nov 6, 2020
@remix-run remix-run deleted a comment from stale bot Nov 6, 2020
@timdorr timdorr added the fresh label Nov 6, 2020
@timdorr
Copy link
Member

timdorr commented Nov 6, 2020

Note that there is some concern over the utility of this prop, which I tend to agree with.

I think if we end up implementing this again, we pass both the navigate and an onClick prop, so that the example in the docs works as-is. For those that want to fully customize the click behavior, they can continue to do so. But for those wanting to just substitute an <a> for another component, they can do so more easily.

@hugmanrique
Copy link
Author

hugmanrique commented Nov 6, 2020

I agree that adding that boilerplate to every custom Link implementation isn't a clean solution. However, while inspecting https://github.com/ReactTraining/react-router/blob/dev-experimental/packages/react-router-dom/index.tsx#L235 I noticed the handleClick function. Is passing that function as the onClick handler to the built component from component/as an option? I think that would hide the implementation details (the CustomLink doesn't know what the onClick function does, only ensures it's passed to the final DOM rendered component). This approach doesn't require passing the navigate prop (which may be a downside to some? if so, how?). My rationale is that a CustomLink passed as the component prop to a Link should only modify its aesthetics, not its behavior.

@chaance
Copy link
Collaborator

chaance commented Sep 3, 2021

Addressed in #7998

chaance added a commit that referenced this issue Sep 3, 2021
* Add support for `useLinkClickHandler` and `useLinkPressHandler` hooks
* Add tests
* Resolves #7598
@chaance chaance closed this as completed Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants