next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Duplicate props with `@next/codemod new-link`

Open andrewmartin opened this issue 1 year ago • 4 comments

Verify canary release

  • [X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

I'm not sure this is super relevant as it's related to the @next/codemod package, but here you go:

    Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 22.1.0: Sun Oct  9 20:14:54 PDT 2022; root:xnu-8792.41.9~2/RELEASE_X86_64
    Binaries:
      Node: 14.20.0
      npm: 6.14.17
      Yarn: 1.22.5
      pnpm: N/A
    Relevant packages:
      next: 13.0.0
      eslint-config-next: 12.1.6
      react: 18.2.0
      react-dom: 18.2.0

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

I think I may have used the <Link /> API incorrectly, so I'm not sure this falls into the path of something that should be handled by the mod, but I ran:

npx @next/codemod new-link .

And it worked, but I had to manually go thru and fix No duplicate props allowed react/jsx-no-duplicate-props errors because all of my links had two href attributes after the mod.

Easy enough to fix, but I thought I'd throw it out there as something maybe the codemod could account for?

<Link href={href} href={href} />

Expected Behavior

I should only see one href attribute.

Link to reproduction

NA

To Reproduce

This should happen on any next repo with a <Link href={href} /><a href={href}></a></Link> element structure.

andrewmartin avatar Oct 27 '22 02:10 andrewmartin

@balazsorban44 I would like to help w/ this one; if it's not assigned to anyone already :)

PS: I think this issue has something to do w/ following lines of code ? https://github.com/vercel/next.js/blob/e2a98cdf3d6ebd03696c53d60739d2506e7d6e9a/packages/next-codemod/transforms/new-link.ts#L68-L75 https://github.com/vercel/next.js/blob/e2a98cdf3d6ebd03696c53d60739d2506e7d6e9a/packages/next-codemod/transforms/new-link.ts#L85-L93

Pranav-yadav avatar Oct 28 '22 19:10 Pranav-yadav

Hey @balazsorban44!

I've submitted a PR that fixes this issue. Would be great if you could review it and if satisfactory, let me know of any due diligence I might need to do to get this merged. I'm fairly new to OS contributions and this project as well, so would appreciate any guidance about steps I might've missed. Thanks!

ishaqibrahimbot avatar Oct 29 '22 20:10 ishaqibrahimbot

Added a comment, great addition and I think it'd be cool to see this in a shared helper or util somewhere. Seems like a common use case to avoid spreading and adding duplicate props.

andrewmartin avatar Oct 29 '22 20:10 andrewmartin

Yeah seems to be a handy pattern. I'll look into where it would be best to put this and how it would be interfaced with.

ishaqibrahimbot avatar Oct 30 '22 09:10 ishaqibrahimbot