Skip to content

feat: add useLink to NuxtLink#26522

Merged
danielroe merged 17 commits intonuxt:mainfrom
userquin:feat-add-use-link-to-nuxtlink
Apr 19, 2024
Merged

feat: add useLink to NuxtLink#26522
danielroe merged 17 commits intonuxt:mainfrom
userquin:feat-add-use-link-to-nuxtlink

Conversation

@userquin
Copy link
Copy Markdown
Member

@userquin userquin commented Mar 27, 2024

🔗 Linked issue

resolves #22169

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Check title: I need to add a test and include useLink type

📝 Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@bolt-new-by-stackblitz
Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@userquin
Copy link
Copy Markdown
Member Author

We need to move the second test to some fixture

@userquin userquin marked this pull request as ready for review March 29, 2024 16:49
@danielroe danielroe requested review from harlan-zw and lihbr April 18, 2024 20:32
required: false,
},
},
useLink: useNuxtLink,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Copy Markdown
Member

@danielroe danielroe Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches RouterLink behaviour (which exposes a 'bound' composable which other libraries, like vuetify, can use to create custom links): https://router.vuejs.org/guide/advanced/composition-api.html#useLink.

return resolvedPath
}

function useNuxtLink (props: NuxtLinkProps) {
Copy link
Copy Markdown
Contributor

@harlan-zw harlan-zw Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI this logic has some legacy issues, see #25532

I wouldn't consider fixing them as part of this PR but it will likely be updated in the future.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. ❤️ Just wanted to check that this implementation would be compatible with the tree-shaking/bundle size improvements you have in mind.

@harlan-zw
Copy link
Copy Markdown
Contributor

Looks good, nice job. Didn't sanity check all of the logic but if tests are passing it should be good 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide RouterLink-compatible useLink method

3 participants