App router: Content flickering with React.createContext and next/dynamic for SSG/SSR about next.js HOT 21 CLOSED

One more repro case to pile on here. When the initial fix PR was reverted, I dug into the history of next/dynamic and saw others reporting similar issues as us on the PR which introduced its current implementation. The suggested fix was to wrap state changes in startTransition. One response to that in the thread suggests a problem when the state updates are within a 3rd party hook out of your control. We're now running into exactly this, so I've set up an isolated repro here to illustrate the problem.

from next.js.

@huozhi Removing of extra suspense boundary fixed the loading state to flash before rendering the next/dynamic - #64687
Without this fix we are completely blocked to proceed with our app migration. For the initial launch we ended up statically importing the components which are above the pageFold to mask CWV degradation, but we cannot continue doing this as we cannot treeshake this and it abundantly increases the app size.

If you want to mark this behaviour as "intended", I would like to ask you to help us find any alternatives to "next/dynamic" as we weren't able to achieve the same level of tree shaking with static imports.

💯 💯 💯
I also wanted to add, in my sandbox example I have created a minimal code to reproduce the bug. The setState call is not in any React contextProvider which you have mentioned in the comment above. But instead in a simple react component which renders a next/dynamic imports. I have more details in added in the issue if it helps to debug it better. Thanks.

from next.js.

@huozhi are there any updates on this ? We are completely blocked to launch our Nextjs project due to the lack of treeshaking support coupled with these unresolved issues. Any updates would be helpful to plan next steps at our end. Thanks

from next.js.

@huozhi quick proposal, thoughts if we want to add the extra suspense as a conditional element ? We can probably use the suspense prop (which I see was deprecated, or we can introduce a new prop to handle this logic)
The default behavior can be to render with the Suspense, but the consumers can opt out of it if required. Thoughts ?

from next.js.

Hi! The real-world use case is loading some initial data from localStorage and setting it as a state on mount. We need it, unfortunately.

Please take a look at my previous comment as well. The fix you had initially pushed, fixed both of our issues (I tested it on a canary version). Both flickering on mount and flickering on redirect. I know you had the reason to revert it, I just want to stress out that we can't finish our migration to the App router in a number of JAMStack apps. Using "registry with dynamic imports" approach proved well on Pages router and we want to keep it in the App router.
If you want to mark this behaviour as "intended", I would like to ask you to help us find any alternatives to "next/dynamic" as we weren't able to achieve the same level of tree shaking with static imports.
I hope my previous message describes our use cases well. Thanks!

from next.js.

@KirillEvtushenko could you share the codebase of this video to let us dig into it? 🙏

from next.js.

I'm looking into this now with previous approach, but there's still errors could throw on client that breaks the dom insertion. We're still investigating

from next.js.

Can you try switching dynamic() call to React.lazy() if you're not using ssr: false before we get it fixed? So far there's a suspense boundary warpped there, it probably can be removed in the future if there's no loading specified.

from next.js.

Hey @huozhi ! Thanks for the answer.
I've tried to switch dynamic() calls to lazy():

'use client';

import { lazy } from 'react';

export const ClientComponent = lazy(() => import('./ClientComponent'));
export const ClientComponent2 = lazy(() => import('./ClientComponent2'));
// Other client components imports

However, I get this error (have never seen this kind of error before lol):
Error: Element type is invalid. Received a promise that resolves to: [object Object]. Lazy element type must resolve to a class or function. Did you wrap a component in React.lazy() more than once?

I found the only way to fix it: remove "use client" directive from the registry. But this breaks code-splitting and puts everything to the First load JS

Am I missing something?

from next.js.

Hi @huozhi!

I'm running into this issue as well, and also thought to try React.lazy(). This does indeed solve the flickering issue, but it causes a regression around the CSS being included on SSR; the awesome work you did to close #62940 isn't applied with React.lazy().

But I'm glad to hear that there's a potential fix, with removing the Suspense boundary when no loading state is provided! I believe this would solve the issue 👍.

from next.js.

I can confirm the next/dynamic doesn't work even if done in a "use client" file.
The solution we did for now was using React.lazy where we could. But yeah quite the regression compared to Pages Router ^^'

from next.js.

Moving aside the issue with React.ContextAPI breaking next/dynamic, there is one more “issue” with next/dynamic I want to show. 



We have a number of JAMStack apps built with Next.js, that we are currently moving from Pages router to App router. In CMS we have a content that describes a page -> sections, their order, props etc. Nothing fancy there, quite common approach. To map CMS content to actual components, we use this kind of registry (simplified):


// registry.ts
const ClientComponentLvl1 = dynamic(() => import('./atoms/ClientComponentLvl1'));
const ClientComponentLvl2 = dynamic(() => import('./molecules/ClientComponentLvl2'));
const ClientComponentLvl3 = dynamic(() => import('./organisms/ClientComponentLvl3'));

const components = {
  ClientComponentLvl1,
  ClientComponentLvl2,
  ClientComponentLvl3
}
export const getComponent = (name: keyof typeof components) => components[name]


// page.tsx
import {getComponent}  from "@/components/registry";

const getPageProps = async () => {
  const props = await fetch(`content from CMS`);
  return props;
}

export default async function Page() {
  const props = await getPageProps();

  const Section = getComponent(props.sectionName);

  return (
    <main>
      <Section />
    </main>
  );
}

All pages are built as SSG. If we open some page, HTML loads as it supposed to, we immediately see the content, no CLS. The problem appears if we NAVIGATE to this page using built-in <Link /> component. We don’t see any HTML for "use client" components then and we suffer from noticeable CLS as dynamic components start actually to load dynamically.

Opened directly (looks good): 


Internal navigation (CLS):

So again: we can directly open the SSG generated page and see the content, but we CAN’T navigate to this page through the app without seeing how it dynamically loads. And this applies only to the ‘use client’ components. RSC is good.
“funny” thing is that the workaround would be to change <Link /> to <a /> tag. Full redirect with reloading of html makes it look much better and smoother, ha.
This behaviour seems wrong. Or do we miss some other way of utilizing JAMStack in Next.js.

from next.js.

In the repro seems there's a setState call in React context Provider when on mount, which will trigger the re-render of Suspense boundary in next/dynamic that lead to the flicking you saw. We recommend to remove the setState in the context provider.
The reproduction demostrates the issue well but I wonder what's the real-world case as trigger re-render in context Provider will trigger all the re-render in the Suspense boundaries below.

from next.js.

Hi @snehakhobragade90 the setState in provider was the original reproduction of this issue. In your repro there's a setState call above the next/dynamic component which will also trigger the Suspense re-rendering

useEffect(() => {
    setsomeState("bar");
  });

from next.js.

@KirillEvtushenko I patched an improvement in #65486 that will preload the chunks earlier, and seemed to work for me with the initial reproduction I downloaded before. Could you help verify with latest canary again to see if it helps since I don't have the source code of the demo from the video?

Also regarding to the read of localStorage, is that possible to finish in callback of useState like useState(() => { .. }) so you don't need to set it again to trigger the re-render?

from next.js.

Hi @snehakhobragade90 the setState in provider was the original reproduction of this issue. In your repro there's a setState call above the next/dynamic component which will also trigger the Suspense re-rendering

@huozhi, that's correct. Calling useState directly in the parent OR setState in a provider will trigger next/dynamic's baked-in suspense boundary. I don't know if it's realistic to recommend that parents and siblings to a dynamic component should not call useState or setState. The baked in suspense boundary does have me looking at using React.lazy() instead to avoid the unwanted suspense boundary but as @joshwcomeau mentioned this reintroduces the style flashing issue #62940 that was patched for next/dynamic. Is a potential path forward to apply a similar fix to React.lazy so CSS is collected before rendering client side?

I have a sandbox that demonstrates the React.lazy() + CSS Module flashing issue.

from next.js.

@KirillEvtushenko I patched an improvement in #65486 that will preload the chunks earlier, and seemed to work for me with the initial reproduction I downloaded before. Could you help verify with latest canary again to see if it helps since I don't have the source code of the demo from the video?

Also regarding to the read of localStorage, is that possible to finish in callback of useState like useState(() => { .. }) so you don't need to set it again to trigger the re-render?

@huozhi, tested on 14.3.0-canary.57. Both issues (on direct opening and on redirect) are still there, unfortunately. I clearly see that waterfall improved and components appear faster, but it's still visually visible. And thats for the direct opening process.

As for redirect to the page using Link component, dynamic loading is still consequent:

from next.js.

Of course! Updated the repo and readme.
https://github.com/KirillEvtushenko/app-router-dynamic-imports-react-context-issue

from next.js.

Might be related to this issues - #65796

Sandbox - https://codesandbox.io/p/github/navin-moorthy/next.js-layout-shift-from-14.2.2/main?checkout=true&workspaceId=51b6ee02-0790-44a1-b61c-2e7846c65016

The flickering can be seen by doing the below steps, -

1. Go to HomePage /
2. Go to AboutPage /about
3. Hard refresh
4. Go to Homepage '`'
5. To reproduce again, Hard refresh
6. Go to AboutPage /about

Edit(12 Jun 2024): This got fixed in https://github.com/vercel/next.js/releases/tag/v14.2.4

from next.js.

We've landed #67014 on canary as the fix, please test it out 🙏 The repro works well locally for me. I'll close this for now

from next.js.

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

from next.js.