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

Test out Support scroll: false for Link component for app router #51869 #146

Closed

Conversation

steve-marmalade
Copy link

I continue to find this repo to be a helpful tool in evaluating NextJS features/fixes. Recently, vercel/next.js#51869 landed in canary, which adds support for scroll={false} in the app router. I was curious whether that was enough to get this demo working, and it seems like it is (no longer getting scroll position reset to the top of the page, browser back button is returning to the correct spot in the feed). I figured I would share the code in case you or anyone else was curious.

  • Upgrade to 13.4.9-canary.1
  • Do some light cleanups (migrate to next/font and remove unused experimental config entries)
  • Use router.replace(href, {scroll: false} and remove the router.replace (not sure if we still need this for anything)
  • Comment out the the use of the Code component as this was causing hydration errors that I didn't know how to fix.

Of course, these changes still don't address the performance issue you noted earlier (that each page is re-fetching all prior pages). I guess we are waiting on a new RSC primitive to help with that.

@alvarlagerlof
Copy link
Owner

@steve-marmalade Looks like a type error on the searchParams. If you commit again will allow the build.

Thank you for the PR!

@vercel
Copy link

vercel bot commented Jul 4, 2023

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

@alvarlagerlof
Copy link
Owner

I think you need to push again or something, Vercel didn't want to build this time.

@alvarlagerlof
Copy link
Owner

Next is now at 13.4.10 in this repo. Please rebase.

@alvarlagerlof
Copy link
Owner

It seems like scroll={false} is no longer needed to have a working browser back button that takes you back to the right scroll position. It seems like prod is behaving correctly already.

Thank you for your PR anyways!

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

Successfully merging this pull request may close these issues.

None yet

2 participants