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

Link with forwardRef component not working #9784

Closed
rahul3103 opened this issue Dec 19, 2019 · 14 comments
Closed

Link with forwardRef component not working #9784

rahul3103 opened this issue Dec 19, 2019 · 14 comments

Comments

@rahul3103
Copy link

rahul3103 commented Dec 19, 2019

Bug report

Describe the bug

Link with forward ref doesn't work anymore. If I pass passHref than it passes the href but the page gets reloaded.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

import React from "react";
import Link from "next/link";

const MyButton = React.forwardRef(({ onClick, href }, ref) => (
  <a href={href} onClick={console.log} ref={ref}>
    Click Me
  </a>
))

const Home = () => (
  <div>
      <Link href="/another">
        <MyButton />
      </Link>
  </div>
);

export default Home;

https://codesandbox.io/s/nextjs-wtnzg
https://github.com/rahul3103/testLink

Expected behavior

I expected url to be seen in the bottom left on hover but it was not there, than I inspected a and there was no href. But, I was getting the onClick event. Than, I added passHref to Link. After that href was there but onClick the whole page was getting re rendered.

System information

  • OS: [macOS]
  • Browser [chrome]
  • Version of Next.js: [9.1.6]

Additional context

Add any other context about the problem here.

@jamesmosier
Copy link
Contributor

I think you'll want to try to reproduce this while using a Next.js codesandbox and by running your app with npm run dev (which codesandbox will do for you automatically). It looks like your codesandbox example isn't using Next.js at all (aside from the next/link component).

Here is an empty Next.js codesandbox you could use.

@rahul3103
Copy link
Author

Next.js codesandbox

https://codesandbox.io/s/objective-poincare-2rblu

@jamesmosier
Copy link
Contributor

jamesmosier commented Dec 19, 2019

This appears to be working.

Code taken from this issue (#8425)

@rahul3103
Copy link
Author

rahul3103 commented Dec 19, 2019

@jamesmosier can you check this: https://github.com/rahul3103/testLink I am using create next app. And link is also not working here.

@jamesmosier
Copy link
Contributor

jamesmosier commented Dec 19, 2019

View the codesandbox code i sent.

const MyButton = React.forwardRef(({ href, onClick }, ref) => (
  <a href={href} ref={ref} onClick={onClick}>
    Click Me
  </a>
));

you have to pass onClick as a prop, not console.log.

And you have to have a page inside pages/ folder in order for the navigation to happen. Right now there is no page for the link to navigate to in your repo.

@rahul3103
Copy link
Author

rahul3103 commented Dec 19, 2019

I know I didnt created any route file but have you seen the difference in both the links. That was the major thing.
Screenshot 2019-12-19 at 9 14 46 PM

See the blue link is created with passHref

@jamesmosier
Copy link
Contributor

There is no way around this. You have to use passHref in this case so the child gets the href. If there is no href on the rendered <a> then the link won't be styled properly by browsers.

@rahul3103
Copy link
Author

rahul3103 commented Dec 19, 2019

Yeah I know, I have to use passHref but have you seen the events get consoled and the page reloads and console goes away. I have added about page also kindly check

@jamesmosier
Copy link
Contributor

Sorry, i'm not understanding. From my perspective, this code should work perfectly:

// need to add `onClick={onClick}` NOT onClick={console.log} as you have
const MyButton = React.forwardRef(({ onClick, href }, ref) => (
  <a href={href} onClick={onClick} ref={ref}>
    Click Me
  </a>
));

const Home = () => (
  <Link passHref href="/about">
    <MyButton />
  </Link>
)

export default Home

@rahul3103
Copy link
Author

Yeah, the above code works perfectly but I don't want my page to reload on the click of link. That's why I added console to show you how events are getting consoled but the page is also getting reloaded on client side on change of URL. console is just logging the events which is getting triggered onclick of link. But, onclicking the link the console comes and go. Which means the page reloaded

@jamesmosier
Copy link
Contributor

In order for the page to NOT reload, you CANNOT have onClick={console.log}. This will never work.

It NEEDS to be onClick={onClick} for it to work, which in your repo you do not have

Aside from that, try asking on spectrum or stackoverflow as this is not an issue with Next.js itself.

@rahul3103
Copy link
Author

rahul3103 commented Dec 19, 2019

const selectedNav = ((state) => state.nav.selectedNav)

const handleClick = nav = (e) => {
    dispatch(setSelectedNav(nav));
 
  };

  useEffect(() => {
    console.log(selectedNav);
  }, [selectedNav]);

 const Room = forwardRef(({ href, onClick }, ref) => (
    <a href={href} onClick={onClick} ref={ref}>
      <Item data-id="room">Room</Item>
    </a>
  ));

  const Home = forwardRef(({ href, onClick }, ref) => {
    return (
      <a href={href} ref={ref} onClick={onClick}>
        <MainItem data-id="home">Home</MainItem>
      </a>
    );
  });

<NavItems>
        <Link href="/" passHref>
          <Home onClick={handleClick('home')} />
        </Link>
        <Link passHref href="/room">
          <Room onClick={handleClick('room')} />
        </Link>
      </NavItems>

This I am using but my page rerenders

@ijjk
Copy link
Member

ijjk commented Dec 19, 2019

As mentioned by @jamesmosier the props like onClick added by next/link needs to be passed or else Next.js can't handle it for you. We recommend having the a tag directly as the child of a Link component since it provides the best experience and makes sure you don't have to worry about passing props like this.

I'm going to close this as it doesn't seem to be an issue with Next.js and we also already have tests covering this here

@ijjk ijjk closed this as completed Dec 19, 2019
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants