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

perf: wrap link component in NextLink #17

Merged
merged 1 commit into from
Nov 1, 2024
Merged

perf: wrap link component in NextLink #17

merged 1 commit into from
Nov 1, 2024

Conversation

ishaan000
Copy link
Collaborator

@ishaan000 ishaan000 commented Nov 1, 2024

  • Refactor the Link component to wrap it with Next.js’ Link component for improved performance and native support.
  • Remove conditional logic to allow Link to act as a button if href is not provided, enforcing href as required for all navigation components.
  • Update tests to align with the refactored implementation, removing button-specific checks and focusing on navigation behavior.

Why This Improves Performance:

  • Client-Side Navigation: Using Next.js’ Link component enables client-side transitions, which keep route changes within the app without triggering a full-page reload. This results in faster navigation and smoother user experience. (See Next.js Link documentation)

  • Prefetching for Faster Loading: Next.js prefetches pages linked with its Link component, loading content in the background while users browse. This allows for near-instant transitions to linked pages, improving perceived performance. (See Next.js Performance Optimization Guide)

  • Enforced Consistency and Simplified Logic: By requiring href on all Link components, this update removes ambiguity in component behavior, aligns with Next.js best practices, and simplifies navigation logic by focusing on links as navigation elements.

@ishaan000 ishaan000 added components Related to React components feature Related to new feature dev Related to development environment labels Nov 1, 2024
@ishaan000 ishaan000 self-assigned this Nov 1, 2024
Copy link
Member

@hiyaryan hiyaryan left a comment

Choose a reason for hiding this comment

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

@ishaan000 Looks good! The only thing I'd update is the commit message. So this would be perf instead of feat!, for a performance improvement rather than a breaking change. We also want to be a little more specific in the commit message, so I'd write perf: wrap link component in NextLink with the body of the commit explaining the changes and how it improves the component. Then for the PR itself, it helps to elaborate more in the first comment block in the conversation. Having a good PR write up is really useful. So a PR write up you should provide links to resources you used, images, and thoroughly explain what this PR does. Use your commit body as an outline for your PR write up instead of using it as the write up itself. Here's an example of a good PR write up cognovi-ai/the-cdj#91

@ishaan000 ishaan000 changed the title feat!: update link component perf: wrap link component in NextLink Nov 1, 2024
Copy link
Member

@hiyaryan hiyaryan left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for updating the PR write up and commit message. 😄

Wrap the Link component with Next.js Link for performance and native support.
Remove logic allowing Link to act as a button without href, enforcing href as required.
Update tests to reflect these changes, removing button-specific checks.
@ishaan000 ishaan000 merged commit 8c77dfc into main Nov 1, 2024
1 check passed
@ishaan000 ishaan000 deleted the link-update-1 branch November 1, 2024 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
components Related to React components dev Related to development environment feature Related to new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants