vercel / next.js

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

Page state is not reset for navigating between dynamic routes #9992

Closed ZigZagT closed 3 years ago

ZigZagT commented 4 years ago

Bug report

Describe the bug

The page state is not reset for navigation between dynamic routes that served by the same source component.

for example, give page source /a/[param]/index.js, when navigating from /a/1 to /a/b, states on the page won't' be reset.

The causing is that for this kind of navigation, what actually happened is the same React Component been rendered with different props. Thus react takes it as a component is rerendering itself, and causing the new navigated page receive stale states.

To fix it, just add {key: <composed base on URL routing params> } to page initial props.

To Reproduce

I've created a live example:

Edit next-bug-dynamic-route-not-rerender

Expected behavior

Page should be fully reloaded and states should be reset.

Screenshots

N/A

System information

Additional context

Janpot commented 4 years ago

Yes! I've seen similar confusing behavior when using useEffect.

  React.useEffect(() => {
    console.log("page initial render");
  }, []);

It only fires once, even when you change the page, but stay on the same route. I'm not sure if it's wrong, but it's definitely not intuitive in my opinion. It's non-intuitive because it works differently when changing the page, but coming from a different route.

Shaker-Hamdi commented 4 years ago

@BananaWanted I just got this exact issue and thanks to @Janpot he pointed me to this post and tried your solution and it worked for me.

I'm not sure if this is a "next router" issue or a react issue, but even with your workaround/solution sometimes the browser back button doesn't work as expected, so I think this should be fixed.

samuelgoldenbaum commented 4 years ago

Great find, suggest docs are updated. For future devs, same should be applied in getStaticProps()

export async function getStaticProps({params}) {
    const props = await getData(params);

    // key is needed here
    props.key = data.id; 

    return {
        props: props
    }
}
pom421 commented 4 years ago

Thanks for the example with the key props. Makes me think of a resource I read recently from Kent C Dodds on the topic : https://kentcdodds.com/blog/understanding-reacts-key-prop

For my need, I have made a bespoke key based on the current timestamp, to ensure to always have a new value :

MyPage.getInitialProps = (ctx) => {
    const { id } = ctx.query
    if (!id || isNaN(id)) return { initialUser: {}, key: Number(new Date()) }
}
tmikeschu commented 4 years ago

Confirming that

Page.getInitialProps = (c) => {
  return {
    id: String(c.query.id),
    key: String(c.query.id),
  };
};

Produced the expected mount/unmount effects that @Janpot mentioned.

ishan123456789 commented 4 years ago

Any solution?

tomgreeEn commented 4 years ago

I have this issue also. The suggested solution (adding a key) works for me in development builds but not in full builds. Any idea why that could be ?

JWeis commented 4 years ago

I also ran into this problem. Adding key to getStaticProps did the trick. I was able to get this solution to build for production with no issues.

danilockthar commented 4 years ago

Great find, suggest docs are updated. For future devs, same should be applied in getStaticProps()

export async function getStaticProps({params}) {
    const props = await getData(params);

    // key is needed here
    props.key = data.id; 

    return {
        props: props
    }
}

where is data come from ? i dont get it

tomgreeEn commented 4 years ago

It's wherever you get data from in your implementation. As I understand any unique string could be used in lieu of data.id (though it's still not actually working for me in production, I need to revisit).

danilockthar commented 4 years ago

It's wherever you get data from in your implementation. As I understand any unique string could be used in lieu of data.id (though it's still not actually working for me in production, I need to revisit).

i got something like this, can i put key with data instead of 'prop' example of samuel ?

export async function getStaticProps({ params }) { const {data} = await comercioBySlug(params.slug) return{ props: {data}, unstable_revalidate: 10 } }

zyzski commented 4 years ago

seems like this may be happening in my app even when the dynamic part is in the middle of a route users/[id]/analytics -> users/[id]/settings

thanks for posting a workaround

modulizer-lee commented 4 years ago
export async function getStaticProps({params}) {
    const props = await getData(params);

    // key is needed here
    props.key = data.id; 

    return {
        props: props
    }
}

I'm using Next version 9.4.4 and this solution isn't working for me

tmikeschu commented 4 years ago

@modulizer-lee key needs to be at the root of the object. You may mean to return props.

modulizer-lee commented 4 years ago

@tmikeschu sorry, I'm a little confused about what you mean. What I currently have is this:

export default function ProductPage(props) {

  const [something, setSomething] = useState(1)

  return(
    ...
  )
}

export async function getStaticProps({ params }) {
  const slug = params.product
  const props = await client.query({
    query: singleProductQuery,
    variables: { id: slug }
  })

  props.key = props.data.product.slug

  return {
    props: props
  }
}

Does this look wrong?

I figured another solution to this problem would be to simply enclose the contents of the page within a simple layout component and add a key to that, but it only fixes states within contained components and not the state defined outside, like in my example above.

tmikeschu commented 4 years ago

@modulizer-lee pardon me. I hadn't seen the version change and the preference for getStaticProps. I had getInitialProps in my head, in which the return value itself is the component props, without the props key on the returning object.

valse commented 4 years ago

OMG this keyed solution is magic! But shouldn't this be the default behavior for dynamic routes? In my blog post page I had some "refresh" issues navigation client side and I didn't understood why... until now :P

vihapr commented 3 years ago

Hello, any workaround for this issue with getStaticProps ?

vladfulgeanu commented 3 years ago

The problem with using the key prop in getStaticProps is that, while going from one page with dynamic props to another one with different props will work, going back (using window.history.back()) will only show the last page with dynamic props that was reached.

/a/1 ---> /a/2 ---> /a/3 ---> /a/4 ---> /a/5 This will work ok with key in props. /a/5 <--- /a/5 <--- /a/5 <--- /a/5 <--- /a/5 This is how the pages will render when using history.back() while having key in props.

sirdeggen commented 3 years ago

I have just come across a solution in my case.

My eg. is involving a link from one page/[id1]/something.js to some other page/[id2]/something.js

The solution for me was:


export default function SomePage() {
    const [something, setSomething] = useState(undefined)
    const router = useRouter()
    const { id } = router.query
    useEffect(() => {
        if (id) setSomething(undefined) // remove the old page data every time the id chages
    }, [id])

   const { data, error } = useSWR( id ? '/api/get/' + id : null, fetch)

   useEffect(() => {
      if(data) setSomething(data) // the new Page data
    }, [data])

    return (<div>{something?.whatever}</div>

}
MaciejWiatr commented 3 years ago

Im still having This issue when using getServerSideProps, any updates?

kelvinlouis commented 3 years ago

For me the fix with providing a key prop only works if the user navigated to the page using the Link component. If the user navigates back by using the browser history getServerSideProps does not get triggered.

samuelgoddard commented 3 years ago

Experiencing this issue on 10.0.1 - the key: fix doesn't work for me either :( does anyone have any suggestions?

samuelgoddard commented 3 years ago

To elaborate further - example in a repo here - https://github.com/samuelgoddard/fat-free-media/blob/main/pages/services/%5Bslug%5D.js (line 244)

Example of the front-end effect here: https://fat-free.vercel.app/

In the footer, there are 3 links Video identity / Content & Creative / Strategy & Consultancy - these pages are generated dynamically by the example code, if you nav between them, as in go from video identity to content & creative or vica-versa you'll see the behaviour of the page not updating as expected.

This is literally a bug that will stop me from using Next in production so any help is super appreciated. Thanks!

samuelgoddard commented 3 years ago

Fixed with some help from the Discord community - the issue for me was the key sent to my <Component> in _app.jswas wrong, i was previously using:

<Component {...pageProps} key={router.route} />

updating to:

<Component {...pageProps} key={router.asPath} />

fixed it for me

valse commented 3 years ago

Fixed with some help from the Discord community - the issue for me was the key sent to my <Component> in _app.jswas wrong, i was previously using:

<Component {...pageProps} key={router.route} />

updating to:

<Component {...pageProps} key={router.asPath} />

fixed it for me

Great... and now the question is... why this isn't a default setting?

PS: @samuelgoddard great site design ;)

lpke commented 3 years ago

To fix it, just add {key: <composed base on URL routing params> } to page initial props.

You literally saved me hours of pain. I was trying to use effect cleanups to get around my issue but this is all I needed. Thank you.

MANTENN commented 3 years ago

I have the same issue except its with SWR not updating data.

Here's what my SWR looks like:

  if (!initialData || !slug) return null;
  const router = useRouter();
  const { query } = router;
  const { data, error, isValidating, mutate } = useSWR(
    PAGE_QUERY,
    (url) => fetcher(url, { slug: query.slug }),
    { initialData }
  );

getStaticProps

export async function getStaticProps({ params }) {
  const res = await fetcher(PAGE_QUERY, { slug: params.slug });
  const { page, servicePage, serviceAreasPage } = res;
  // when no typee of page is not found return 404 page
  const notFound =
    page.items.length == 0 &&
    servicePage.items.length == 0 &&
    serviceAreasPage.items.length == 0;
  if (notFound) {
    return {
      notFound,
    };
  }
  return {
    props: {
      slug: params.slug,
      initialData: res,
      notFound,
    },
    revalidate: 60,
  };
}
MANTENN commented 3 years ago

https://github.com/vercel/swr/issues/284

MANTENN commented 3 years ago

Changed my getStaticProps to, as others mentioned--worked, to:

export async function getStaticProps({ params }) {
  const res = await fetcher(PAGE_QUERY, { slug: params.slug });
  const { page, servicePage, serviceAreasPage } = res;
  // when no typee of page is not found return 404 page
  const notFound =
    page.items.length == 0 &&
    servicePage.items.length == 0 &&
    serviceAreasPage.items.length == 0;
  if (notFound) {
    return {
      key: params.slug,
      notFound,
    };
  }
  return {
    props: {
      key: params.slug,
      slug: params.slug,
      initialData: res,
      notFound,
    },
    revalidate: 60,
  };
}
jnahumphreys commented 3 years ago

Thanks for raising this issue, I've subscribed as I'd be interested to know if where the onus for adding a key sits (i.e library level or user level). Either way, adding the key resolves the "bug" for me 👍

mattrosno commented 3 years ago

Also experiencing issues with getStaticProps and browser back, even after adding a unique key to the props return.

This feels so wrong but it's working and I can't find a better solution.

Using next version 10.0.4.

export default function Page({
  data,
}) {
  const router = useRouter();
  const uid = useRef(router.query.uid);

  /**
   * If for some reason this component has stale props from navigating back by
   * browser history, reload the entire page.
   *
   * @see {@link https://github.com/vercel/next.js/issues/9992}
   */
  useEffect(() => {
    if (router.query.uid != uid.current) {
      router.reload();
    }
  }, [router.query.uid]);

  return <div>...</div>;
}

export const getStaticProps = async ({ params, previewData = {} }) => {
  const data = await queryByUid('product', params.uid, {
    ...previewData,
  });

  return {
    props: {
      data,
      key: params.uid,
    },
    revalidate: 60,
  };
};

export const getStaticPaths = async () => {
  const { results } = await queryByType('product');

  const paths = results.map((result) => ({
    params: {
      uid: result.uid,
    },
  }));

  return {
    paths,
    fallback: false,
  };
};
valse commented 3 years ago

The working solution is that from @samuelgoddard adding the router.asPath key to the _app page Component: <Component {...pageProps} key={router.asPath} />

MANTENN commented 3 years ago

@mattrosno Sounds like you have a custom _app. Returning props would work without a custom app so if you are using a custom _app then make sure the Component is receiving the props passed through from the custom _app.

fcFn commented 3 years ago

Is this described somewhere in the docs? I really think it should be in the Data Fetching section. Perhaps as another note to each of the data fetching methods (seeing that some of them are already duplicated, e.g. the note about fetch()).

justincy commented 3 years ago

I had the same issue but I only need two stateful components to fully reset so I just added key={router.asPath} to those two components instead of the whole page.

valclark1 commented 3 years ago

This is still happening on 11.0.2-canary.3

When I navigate to a detail page with accepts [id].ts and navigate back to the top level page using breadcrumbs(link or router) a request is made to the previous route with stale props.

I'm using getServerSideProps rendering. This method <Component {...pageProps} key={router.asPath} /> does not work.

Nel-sokchhunly commented 3 years ago

For my issue, I want the useEffect() to trigger when the params of the dynamic route is change. The solution to this is setting the params as the key to useEffect() hook. useEffect(() => { // execute code }, [params.id]); In my case, I use my params id

devuxer commented 3 years ago

Custom _app.tsx with <Component {...pageProps} key={router.asPath} /> definitely solved my issue. Wish I had landed on this issue before killing so many hours on this.

As @valse point out, why isn't this the default? @Timer, could this be considered?

valclark1 commented 3 years ago

From the looks of it when using getServerSideProps key is removed from the props. Still can't find a solution for this. May have to reconsider using Next.js because of this issue.

aldabil21 commented 3 years ago

From the looks of it when using getServerSideProps key is removed from the props. Still can't find a solution for this. May have to reconsider using Next.js because of this issue.

In my case the key in getServerSideProps works.

brunobraga95 commented 3 years ago

In my case I am using link to go from page / to /some_id. When trying to go back to / using the browser arrow the page will not reload again, simply nothing will change.

I added console.logs to the components that should be rendered (NewsCard) and all the logs can be found, which is ever weirder. If they were not even having their render() method called it would make a bit more sense.

I tried the key param, using useEffect with [route.query.id], and passing a key to the at app.js but nothing worked. Any tips?

import Landing from "@containers/landing/Landing";
import { readTranscripts } from "@firebaseDb/index";

function LandingPage(props) {
  console.log(props);
  return <Landing {...props} />;
}

export async function getStaticProps(context) {
  const newsTranscripts = await readTranscripts();
  const transcriptsWithoutFirebaseTimestamp = newsTranscripts.map(
    transcript => ({ ...transcript, createdAt: transcript.createdAt.toDate().toJSON() })
  );
  return {
    props: {
      news: transcriptsWithoutFirebaseTimestamp,
      key: Number(new Date())
    } // will be passed to the page component as props
  };
}

export default LandingPage;

const Landing = ({ news }) => {
  return (
    <Wrapper>
      {news.map(newItem => (
        <React.Fragment>
          <Link href={"/news/" + newItem.title}>
            <a>
              <NewsCard new={newItem} />
            </a>
          </Link>

          <StyledDivider light />
        </React.Fragment>
      ))}
    </Wrapper>
  );
};
aldabil21 commented 3 years ago

In my case I am using link to go from page / to /some_id. When trying to go back to / using the browser arrow the page will not reload again, simply nothing will change.

I added console.logs to the components that should be rendered (NewsCard) and all the logs can be found, which is ever weirder. If they were not even having their render() method called it would make a bit more sense.

I tried the key param, using useEffect with [route.query.id], and passing a key to the at app.js but nothing worked. Any tips?

import Landing from "@containers/landing/Landing";
import { readTranscripts } from "@firebaseDb/index";

function LandingPage(props) {
  console.log(props);
  return <Landing {...props} />;
}

export async function getStaticProps(context) {
  const newsTranscripts = await readTranscripts();
  const transcriptsWithoutFirebaseTimestamp = newsTranscripts.map(
    transcript => ({ ...transcript, createdAt: transcript.createdAt.toDate().toJSON() })
  );
  return {
    props: {
      news: transcriptsWithoutFirebaseTimestamp,
      key: Number(new Date())
    } // will be passed to the page component as props
  };
}

export default LandingPage;

const Landing = ({ news }) => {
  return (
    <Wrapper>
      {news.map(newItem => (
        <React.Fragment>
          <Link href={"/news/" + newItem.title}>
            <a>
              <NewsCard new={newItem} />
            </a>
          </Link>

          <StyledDivider light />
        </React.Fragment>
      ))}
    </Wrapper>
  );
};

I think this is different issue than the OP. Cuz / and /[some_id] are supposed to be 2 different pages. The OP here is talking about the case which when navigating between /[some_id] and /[some_other_id] page would not reset the state, which actually has nothing to do with Next it's just how React works and re-render elements.

However, in the code you've posted, this supposed to be the / page and not the /[some_id] page, cuz I don't see any use of the some_id param nor having a getStaticPaths method which is required when using a dynamic route with getStaticProps. So its a statically generated page without a state, there is nothing to be resetted here.

I tried the key param, using useEffect with [route.query.id], and passing a key to the at app.js but nothing worked.

Did you mean _app.js? As far as I know this is not how you get access to the dynamic route params, you access it using the context provided by one of Nextjs data fetching methods.

Can you show you pages folder structure and your /[some_id] page content. Did you fully read and took a good look at Data fetching and Dynamic Routes sections?

Brenndoerfer commented 3 years ago

In Next 11 it's still not properly reloading/resetting components state when navigating from one dynamic page e.g. /[slug] (/a -> /b) to another.

Is there any known workaround for this?

rverton commented 3 years ago

In Next 11 it's still not properly reloading/resetting components state when navigating from one dynamic page e.g. /[slug] (/a -> /b) to another.

Is there any known workaround for this?

As a workaround, the one referenced by @devuxer here worked for me.

Deep-Codes commented 3 years ago

Fixed with some help from the Discord community - the issue for me was the key sent to my <Component> in _app.jswas wrong, i was previously using:

<Component {...pageProps} key={router.route} />

updating to:

<Component {...pageProps} key={router.asPath} />

fixed it for me

This works thanks a ton ❤️

bhatvikrant commented 3 years ago

Fixed with some help from the Discord community - the issue for me was the key sent to my <Component> in _app.jswas wrong, i was previously using:

<Component {...pageProps} key={router.route} />

updating to:

<Component {...pageProps} key={router.asPath} />

fixed it for me

Perfect! this works!

Pranav2612000 commented 3 years ago

Fixed with some help from the Discord community - the issue for me was the key sent to my <Component> in _app.jswas wrong, i was previously using:

<Component {...pageProps} key={router.route} />

updating to:

<Component {...pageProps} key={router.asPath} />

fixed it for me

This worked for me as well. Thank you. To add, just for completeness, the router is the value defined using useRouter

import { useRouter } from "next/router";
const router = useRouter();    /* Inside the component */
ijjk commented 3 years ago

Hi, I'm gonna close this as a duplicate of https://github.com/vercel/next.js/issues/26270. We have also added a note for this behavior in the documentation here https://nextjs.org/docs/api-reference/next/router#usage

NikitaMazur commented 3 years ago

I tried all the solutions from this topic and not one of them helped me solve the problem with the getStaticProps + browser back button. Only these lines helped to solve the problem. I know it doesn't look very good, but it saved my working day :) useEffect(() => { window.addEventListener('popstate', function() { location.reload() }, false) }, [])