vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.85k stars 26.96k forks source link

⚠ Constent tab crashes when using browser's navigation buttons in soft navigation in app dir - reproduces across projects, envs, browsers and devices #50382

Open FearlessSlug opened 1 year ago

FearlessSlug commented 1 year ago

Verify canary release

Provide environment information

---

Which area(s) of Next.js are affected? (leave empty if unsure)

Entire app, it's a navigation bug

Link to the code that reproduces this issue or a replay of the bug

https://github.com/FearlessSlug/nextjs-app-dir-router-bugs

To Reproduce

TLDR: Soft navigate between the pages and then try to slightly spam the browser's back button (it has to be the broswer's navigation buttons!) you will see that the tab freezes and then crashes

I published this nextjs blank project that just has a bunch of different ways to navigate in nextjs app dir, if you soft navigate between the pages (don't go to the 404 pages because they'll do full refresh) and then try to slightly spam the browser's back button you will see that the tab freezes and then crashes, this kind of tab freeze and crash is especialy frequent when using path params hight up in the url, for example when using localization and you need to add /[locale]... to the root of your app directory.. I have tested it on multiple browsers and multiple PCs both locally as well as Vercel hosted, it's consistently crashing and freezing tabs, and keep in mind this is a blank project that has no extra code... Github repo: https://github.com/FearlessSlug/nextjs-app-dir-router-bugs Vercel hosted: https://nextjs-app-dir-router-bugs.vercel.app/test1

2023-05-25-19-23-19-Trim

Describe the Bug

Tab freeze and crash

Expected Behavior

I expect the tab to not freeze and crash

Which browser are you using? (if relevant)

Brave, Firefox, Safari (iOS), Edge

How are you deploying your application? (if relevant)

Vercel

niallobrien commented 1 year ago

Can confirm, happens for me too when testing the above repo/deployment.

imaksp commented 1 year ago

I can also confirm (checked on latest chrome on MBP), It seems they made app dir stable early in hurry.

FearlessSlug commented 1 year ago

This bug seems like it's rooted in the client navigation system of NextJS if that's correct it means it effects any app that uses the app directory and client side navigation, this issue should be very high priority... IDK how people going to prod with Next13.4.3/4, are they not seeing this bugs while developing or do they don't even care..

FearlessSlug commented 1 year ago

This bug has huge impact on DX I literally cannot develop my app, I get constant tab crashes after every single fast refresh during development, how is this considered stable? The more my code base grows the more unbearable the DX become this is actually insane...

niallobrien commented 1 year ago

So what's the process here? Cuz I see lots of issues, but no comments from the team and then the issues just automatically get closed if there's been no activity.

FearlessSlug commented 1 year ago

@niallobrien I have no idea, this is very strange, it effects so many projects yet no action is taken, it feels like an abandoned project 😅

niallobrien commented 1 year ago

I mean, I understand that the team are very busy and all, but I'm just wondering what the actual processes are in place for triaging issues etc.?

FearlessSlug commented 1 year ago

This is horrible, the tabs keep on crashing non-stop in development even for code that's not related to navigation...

niallobrien commented 1 year ago

I haven't seen this issue in other repos though, which is weird...

FearlessSlug commented 1 year ago

@niallobrien But what can cause this though? it's an empty project that just uses the router and navigation just like the documentations show..

niallobrien commented 1 year ago

No idea, have you managed to narrow it down to just RSC or client components?

imaksp commented 1 year ago

Hi @FearlessSlug after wasting almost 1 hour I found that you have used async in page component function (may be by mistake), due to this it is hanging on browser back/forward click. remove async from here: https://github.com/FearlessSlug/nextjs-app-dir-router-bugs/blob/main/src/app/test1/page.tsx#L5

FearlessSlug commented 1 year ago

@imaksp Thank you so much I just pushed the changes to the repo and to vercel and it seems like it's fixed it! That's the stupidest bullshit of a bug if I've ever seen one, why wouldn't it work like a redundant async on a regular function, I'm honestly shocked, I nearly dropped nextjs altogether because of this BS bug which blocked me for almost a week...

FearlessSlug commented 1 year ago

I closed this issue but I think it's still an unexpected behavior that should be taken care of either by a warning/error in the terminal or by making redundant async in components not block the thread..

FearlessSlug commented 1 year ago

On second thought I'll keep it opened..

I just implemented similar fixes to my main project that originally experienced the bug and for some reason the bug isn't completely fixed by removing the redundant asyncs, the situation did get better and now the tab doesn't freeze immediately when I navigate, but on a specific page it does, the page only has one client component that just returns a div, it has no extra logic whatsoever, so I'm still experiencing this bug... 🤢 I'll try to reproduce it in the project I linked and post an update later

imaksp commented 1 year ago

Yes you can keep it open & I think this is good candidate for next lint. but if you are keeping it open you will need to update the reproduction repo as without reproduction this might get closed by team.

FearlessSlug commented 1 year ago

@imaksp So I managed to reproduce the new bug, I pushed the changes to the repo above, I just moved the pages to a (test-pages) directory for convenience, and the problems occurs for some reason because of the client component page /test1/page.tsx is using the /test1/components/SomeComp.tsx client component, here's how to reproduce:

  1. do some soft navigation (only between test1 and test2 pages, not to the 404 pages)
  2. after some soft navigation go to the 404 pages
  3. click the back button of the browser a few times
  4. You will notice that after a couple of times of successfully navigating away from the 404 page which sometimes make the tab do a full refresh the app will go back to the soft navigation when navigating back and after a couple more back navigation you will see that the app will no longer respect the back button navigation and the URL changes, and the page content doesn't change, so even though you are on URL /test1 you will still see the content of the /test2 page
imaksp commented 1 year ago

Hi, checked quickly & I can confirm that it messes up after you visit any 404 page from async server component & then go back by using browser back button. May be some bug with async server component. But in actual app user won't visit 404 page by clicking some site button, so I think its less serious than before, but still a bug. will try to find out more later.

Also by looking at docs it seems this is still at RFC stage in React, but Next team has implemented it in advance.

JasonA-work commented 1 year ago

I can verify that this is happening both in 13.4.4 and canary. For me, this happens randomly whenever there are errors shown while developing too (errors like TS type errors and sass errors like missing a bracket).

I also think that app dir is flagged as stable too early :(

cgpro commented 1 year ago

In my use case, I have a fetch function in page.tsx and in some components, it has to be async in my case. Prop drilling is out of question, since I have various component combinations with Storyblok (Headless-CMS) for a relatively large website with multiple pages in two languages.

Is there any “temporary” workaround?

I think this is not such a rare case, that several components have to be async.

cgpro commented 1 year ago

@FearlessSlug, does this also happen, if you wrap your components with a <Suspense>? This fixed my "crashing/hanging back button". (a loading.tsx with Loading() in it, fixes it in my case too)

thucxuong commented 1 year ago

I just copy the data fetching pattern NextJS docs for app dir feature, and the back button does freeze my chrome tab. Still does not know when Vercel team can fix this since the back button is quite common for user to use.

imaksp commented 1 year ago

I just copy the data fetching pattern NextJS docs for app dir feature, and the back button does freeze my chrome tab. Still does not know when Vercel team can fix this since the back button is quite common for user to use.

First make sure that you are not using async with client components, I think this occurs for some specific cases, if you think this happens for simple navigations, please create new reproduction (with specific reproduction steps) or even a new issue because it looks like no one from team is paying attention to this.

curiouscod3 commented 1 year ago

For me, It was because "async" keyword with "use client". So. I want to know why client components shouldn't be used with "Async". I think Async client components should be no problem because async/await is a super common pattern in Javascript world.

jangir-ritik commented 1 year ago

Similar problem occurred to me.

Setup

  1. I had a client component (page.js of /portfolio route)
  2. A child component (PortfolioList) (An async server component that uses the new way of data fetching introduced in app router)
  3. PortfolioList had portfolioCards (async server components) that navigated to respective portfolio detail page using next/Link
  4. Portfolio detail page (async server component) fetched portfolio detail API and rendered the content.

What I am trying to point out is that I had a client component as the parent and its children were all async server components.

The problem

On soft navigation to the portfolio page (using next/link or router/navigation), multiple fetch requests to portfoliolist API were fired in race condition (Sometimes crashing the browser window, sometimes working as expected) In cases when it worked as expected, I'd further navigate to portfolio detail page by clicking on the portfolioCard, the detail page would render and the url in the browser would change (next would prefetch the data successfully), now when I navigate back using the back button of the browser, the browser window would definitely crash ( I tried putting up debuggers, checking for memory leaks, network usage but all in vain). Then I came across this issue and a RFC on data fetching and why clients can't be async functions , I don't entirely understand the underlying workings so, I created a new next app and tried to see if it breaks here as well or not. This time, I didn't use the parent as a client component and it worked as expected.

Why did I flag the parent as a client component?

The requirement was to have a button that would conditionally render portfolios or case studies, and to achieve this I created a flag that I stored in a react state variable, hence, a client component

Conclusion

Do not use client side functions with async server side children as of now. I still can't wrap my head around how can a child be server component when the parent is a client sided component