kirill-konshin / next-redux-wrapper

Redux wrapper for Next.js
MIT License
2.67k stars 265 forks source link

Hydration happens after first render when navigating from a different page #280

Closed moneszarrugh closed 3 years ago

moneszarrugh commented 4 years ago

When I navigate to a page which I added the getStaticProps wrapper to, the hydration happens after an innitial render which results in my component getting rendered breifly without the new state changes made from the dispatch done in getStaticProps, then the hydration happens and the component rerenders with the new state.

Note that this issue only happens when I navigate from a different page. When I refresh from the page directly, the hydartion happens first, as it should, and then the component is rendered.

Sorry for lack of code snippets, will add if needed.

Edit: Code was added below.

I have create a simple counter app. The count starts at 0 but it is reset to 10 inside getStaticProps.

I have added console logs inside the HYDRATE function and the counter component

The console logs are used to indication when the hydration happens and when the counter app renders

The result of refreshing the http://localhost:3000/counter page is:

HYDRATE...
render...

However, the result of loading http://localhost:3000 and then navigating to http://localhost:3000/counter by clicking a Link is:

render...
HYDRATE...
render...

The problem is that there are 2 renders. One before hydration and another one after. The expected behaviour is one render AFTER hydration, same as above.

// store/counter.js

import { createSlice } from '@reduxjs/toolkit';
import { HYDRATE } from 'next-redux-wrapper';

const slice = createSlice({
  name: 'counter',
  initialState: {
    count: 0,
  },
  reducers: {
    increment: (counter, action) => {
      counter.count += action.payload;
    },
    decrement: (counter, action) => {
      counter.count -= action.payload;
    },
    setCount: (counter, action) => {
      counter.count = action.payload;
    },
  },
  extraReducers: {
    [HYDRATE]: (counter, action) => {
      console.log('HYDRATE...');
      counter.count = action.payload.counter.count;
    },
  },
});

export default slice.reducer;

export const { increment, decrement, setCount } = slice.actions;

export const selectCount = state => state.counter.count;
// store/configureStore.js

import { configureStore } from '@reduxjs/toolkit';
import { createWrapper } from 'next-redux-wrapper';
import counter from './counter';

export const wrapper = createWrapper(() =>
  configureStore({
    reducer: {
      counter,
    },
    devTools: true,
  })
);
// pages/_app.js

import '../styles/globals.css';

import { wrapper } from '../store/ configureStore';

function MyApp({ Component, pageProps }) {
  return <Component {...pageProps} />;
}

export default wrapper.withRedux(MyApp);
// pages/index.js

import Head from 'next/head';
import Link from 'next/link';

import styles from '../styles/Home.module.css';

export default function Home() {
  return (
    <div className={styles.container}>
      <Head>
        <title>Counter App</title>
        <link rel="icon" href="/favicon.ico" />
      </Head>

      <main className={styles.main}>
        <div className={styles.title}>
          <Link href="/counter">Go to counter</Link>
        </div>
      </main>
    </div>
  );
}
// pages/counter.js

import Head from 'next/head';
import styles from '../styles/Home.module.css';

import { wrapper } from '../store/ configureStore';
import { useDispatch, useSelector } from 'react-redux';

import { selectCount, increment, decrement, setCount } from '../store/counter';

export default function Counter() {
  const dispatch = useDispatch();
  const count = useSelector(selectCount);
  console.log('render...');

  return (
    <div className={styles.container}>
      <Head>
        <title>Counter App</title>
        <link rel="icon" href="/favicon.ico" />
      </Head>

      <main className={styles.main}>
        <h1>Counter App</h1>
        <h2>Count: {count}</h2>
        <button onClick={() => dispatch(increment(1))}>Increment</button>
        <button onClick={() => dispatch(decrement(1))}>Decrement</button>
      </main>
    </div>
  );
}

export const getStaticProps = wrapper.getStaticProps(async ({ store }) => {
  store.dispatch(setCount(10));
});
kirill-konshin commented 4 years ago

I need to see the code.

moneszarrugh commented 4 years ago

I need to see the code.

Code was added

kirill-konshin commented 4 years ago

Can you please create a code sandbox?

moneszarrugh commented 4 years ago

Can you please create a code sandbox?

https://codesandbox.io/s/next-redux-wrapper-bug-8q0fc

https://github.com/moneszarrugh/next-redux-wrapper-bug

kirill-konshin commented 4 years ago

I cannot reproduce... I tried both ways you described β€” direct link and via navigation. Both yielded same outcome: HYDRATE -> render.

https://8q0fc.sse.codesandbox.io/counter this is your sandbox, if I open it directly, I see correct render all the times.

moneszarrugh commented 4 years ago

I cannot reproduce... I tried both ways you described β€” direct link and via navigation. Both yielded same outcome: HYDRATE -> render.

https://8q0fc.sse.codesandbox.io/counter this is your sandbox, if I open it directly, I see correct render all the times.

Yes, I can see that too. But that is the log from node terminal, not from the browser. Please open up the browser debug console with F12 and look again.

Jekins commented 4 years ago

I have the same problem with getServerSideProps and async redux-thunk. I don't understand how to fix this. Link for reproduce: https://codesandbox.io/s/flamboyant-meadow-v1wd1

kirill-konshin commented 4 years ago

@Jekins looks like when you switch pages, your state still has old entities, thus selector cannot find a proper page by new ID.

image

Also looks like the HYDRATE action is dispatched "late", e.g. your page has already rendered, and here comes HYDRATE:

image

Technically, this is legit, because of how library works. Your page may receive new props earlier than the HYDRATE dispatch because the code that does it is in effect: https://github.com/kirill-konshin/next-redux-wrapper/blob/master/packages/wrapper/src/index.tsx#L189

Potentially, using useLayoutEffect in an isomorphic way may fix the issue:

import { useLayoutEffect, useEffect } from 'react';
const useIsomorphicLayoutEffect = typeof window !== 'undefined' ? useLayoutEffect : useEffect;

But I don't have time to try it. If you can help me with this I'd appreciate.

Jekins commented 4 years ago

@kirill-konshin I did what you said, but it didn't work. This code is not executed, console.log('TEST') is not output. With only react_1.useEffect also console.log is not output:

var useIsomorphicLayoutEffect = getIsServer ? react_1.useEffect : react_1.useLayoutEffect;

useIsomorphicLayoutEffect(function () {
  console.log('TEST')

  if (isFirstRender.current) {
    isFirstRender.current = false;
    return;
  }
  hydrate();
}, [hydrate]);
Jekins commented 4 years ago

I noticed that after routing, the state is reset and then only adds new dispatches for the new page. State not persisted from page to page. This is a big problem for me and do not understand how fix it.

Jekins commented 4 years ago

If it helps someone, then instead of export const getServerSideProps = wrapper.getServerSideProps( async ({ store }) => { I redid it to Page.getInitialProps = async ({ store }) => and this solves the problem.

Don't forget wrapper.withRedux (...)

kirill-konshin commented 4 years ago

Unfortunately, right now I'm still out of time to work on this issue. It looks like library works as intended, even though it may be counter-intuitive at first. But there could be a lag between when your new prop from path appears at page level and when HYDRATE action is dispatched. Ideally, there should be a way to synchronously dispatch. But as @Jekins pointed out, it did not work. I'm not sure why:

Updates scheduled inside useLayoutEffect will be flushed synchronously, before the browser has a chance to paint.

So dispatch was supposed to be performed right away. Also keep in mind, that this hook only executes on client, not on server.

So far my only recommendation would be to check if hydration has happened and show a nice spinner while data is fetching. You should probably do it anyway, since there will always be a lag while you fetch things from server.

khaibula commented 4 years ago

@kirill-konshin Hi! I managed to do something similar.

I created a separate reducer for this case

import { createSlice, PayloadAction } from '@reduxjs/toolkit';

const hydrateSlice = createSlice({
  name: 'isHydrate',
  initialState: false,
  reducers: {
    setHydrate: (state, action: PayloadAction<boolean>) => {
      return action.payload;
    },
  },
});

export const { reducer: hydrateReducer, actions: hydrateActions } = hydrateSlice;

After I connected it to the reducer

export const combinedReducers = combineReducers({
  ...othenReducers,
  isHydrate: hydrateReducer,
});

in main reducer i write

export const reducer = (state: ReturnType<typeof combinedReducers>, action) => {
  if (action.type === HYDRATE) {
    return {
      ...othenRestOperation,
      isHydrate: true,
    } as ReturnType<typeof combinedReducers>;
  }
  return combinedReducers(state, action);
};

I hang up the handler for changing pages (router)

export const ChangeHydrateHandler = () => {
  const router = useRouter();
  const dispatch = useAppDispatch();

  useEffect(() => {
    router.events.on('routeChangeStart', () => {
      dispatch(hydrateActions.setHydrate(false));
    });
  }, []);

  return <></>;
};

I render this handler in _app.tsx

And additionally I created hydrate tracking effect

type UseIsHydrateEffectType = (
  func: () => any,
  dependencies?: any[]
) => any;

export const useIsHydrateEffect: UseIsHydrateEffectType = (func, dependencies = []) => {
  const isHydrate = useAppSelector((state) => state.isHydrate);

  useEffect(() => {
    if (isHydrate) {
      func();
    }
  }, [isHydrate, ...dependencies]);
};

That's all! Don't find fault with the code, this is my first experience with next.js. (And, in general, the first experience of publishing code in Issue) If you have any recommendations on how to improve the code, write.

kirill-konshin commented 3 years ago

@Jekins

var useIsomorphicLayoutEffect = getIsServer ? react_1.useEffect : react_1.useLayoutEffect;

getIsServer is a function, so if used like so it will always return useEffect. But despite this React keeps calling effect after the render, which is legit. The only thing that useLayoutEffect does is it eliminates the flash of missing content.

I honestly don't see a way to fully synchronously dispatch a hydrate event during rendering phase. If I do that the warning will be thrown:

Warning: Cannot update a component (Page) while rendering a different component (withRedux(App)).

I will push the fix to the next release, but even with fix you as App developer still have to be responsible for proper handling of all events in the App. E.g. you should show appropriate content (loading screen or anything else), even if users won't see it, just in case, you know.

Or you can handle it the way @khaibula has proposed.

I see that in Next.js example https://github.com/vercel/next.js/blob/canary/examples/with-redux/store.js#L55 they replace the store. I am not sure it's good solution., because Redux will re-render components even with memoized selectors (createSelector from recompose) if store is replaced: https://codesandbox.io/s/redux-store-change-kzs8q, which may affect performance of the app.

kirill-konshin commented 3 years ago

Use https://github.com/kirill-konshin/next-redux-wrapper/pull/295 to track progress.

kirill-konshin commented 3 years ago

https://github.com/kirill-konshin/next-redux-wrapper/releases/tag/7.0.0-rc.1

stegano commented 3 years ago

Is there a reason to execute hydrate action every time you move a page? I know that the purpose of hydrate is to inject the state of the page rendered on the server side. please let me know if I'm wrong.

kirill-konshin commented 3 years ago

@stegano it is mandatory to keep state consistent on first launch and during navigation.

stegano commented 3 years ago

Hi @kirill-konshin Thanks for your answer πŸ˜€ Sorry, but I haven't understood it yet. If only getInitialProps is used, is there any problem even if the Hydrate action does not occur after the initial SSR?

kirill-konshin commented 3 years ago

Hi @kirill-konshin Thanks for your answer πŸ˜€ Sorry, but I haven't understood it yet. If only getInitialProps is used, is there any problem even if the Hydrate action does not occur after the initial SSR?

If you're dispatching actions at the server (in getInitialProps) you need to deliver this data to client on EVERY page load, when first opened and on subsequent navigation too.

stegano commented 3 years ago

Hi @kirill-konshin Thanks for your answer πŸ˜€ Sorry, but I haven't understood it yet. If only getInitialProps is used, is there any problem even if the Hydrate action does not occur after the initial SSR?

If you're dispatching actions at the server (in getInitialProps) you need to deliver this data to client on EVERY page load, when first opened and on subsequent navigation too.

Thank you for your answer and sorry to keep asking questions

In the case of using geInitialProps, the first server receives data and executes a hydrate action to synchronize the state delivered from the server and the client side state, When the page is moved, getInitalProps is executed on the client side, so it is immediately dispatched to the redux-store

So I think that there is no need to execute the hydrate action any more. please let me know if i'm wrong..

kirill-konshin commented 3 years ago

@stegano you are talking about a deprecated function. You can ignore the hydration action when it’s dispatched from getinitialprops in client. The wrapper works like this for sake of consistency with other data functions like getStaticProps and getServerSideProps.

stegano commented 3 years ago

@stegano you are talking about a deprecated function. You can ignore the hydration action when it’s dispatched from getinitialprops in client. The wrapper works like this for sake of consistency with other data functions like getStaticProps and getServerSideProps.

Thank you for your kind response. I think it makes sense a little now.

Does the "deprecated function" you say mean getInitialProps? as far as I know getInitialProps is not deprecated.

Thanks for making a good library. πŸ˜€

fostyfost commented 3 years ago

When was getInitialProps deprecated? Don't mislead people. On the client, one HYDRATE call is sufficient in the case of getInitialProps.

kirill-konshin commented 3 years ago

@fostyfost

Pardon. Not recommended.

image

Jashnm commented 3 years ago

I noticed that after routing, the state is reset and then only adds new dispatches for the new page. State not persisted from page to page. This is a big problem for me and do not understand how fix it.

Hey! Did you find the solution with getServerSideProps? I'm using version 7.0, followed the redux-toolkit demo, but state is not persisting while navigating from page to page.

kirill-konshin commented 3 years ago

Hey! Did you find the solution with getServerSideProps? I'm using version 7.0, followed the redux-toolkit demo, but state is not persisting while navigating from page to page.

It means that your hydration reducer adds too much data to state, thus overwriting something that client has changed. Read more about state reconciliation here: https://github.com/kirill-konshin/next-redux-wrapper#state-reconciliation-during-hydration. Please use Redux Dev Tools to check what is being dispatched and how it changes the state, and you will see.

Jashnm commented 3 years ago

@kirill-konshin __NEXT_REDUX_WRAPPER_HYDRATE__ is getting dispatched every time, rewriting all the state.

Got the issue, but I'm getting confused on how to do reconciliation in toolkit.

kirill-konshin commented 3 years ago

Please read the documentation. Redux toolkit works exactly the same as bare redux. Whatever is returned from reducer becomes new state. You have to return properly merged state from it.

Jashnm commented 3 years ago

@kirill-konshin Sorry and thanks, I'm bit new to redux side, I tried jsondiffpatch method. Global state is persisting but not in a suitable manner, either it gets nested in some other state or vanishes completely.

const wasBumpedOnClient = stateDiff?.page?.[0]?.endsWith("x");

When I target page here, I get undefined. If you can help me understand this line, what criteria it is fulfilling and how, it'll be really helpful.

kirill-konshin commented 3 years ago

@Jashnm I have no idea how your app is architected, thus I cannot make assumptions how it should work.

https://github.com/kirill-konshin/next-redux-wrapper/blob/types-enchancements/packages/demo-redux-toolkit/store.tsx#L23

Here you can see that hydration reducer will only take certain slice from action, the rest of the state will not be modified.

Consider making a codesandbox to demo what you want to achieve, so that I'll be able to reproduce the issue.

Jashnm commented 3 years ago

@kirill-konshin I read docs thoroughly together with examples from Nextjs repo, and got it working. So no issues now.

shadjiu commented 3 years ago

I have same problem. Fixed with a custom hook.

import { useRef, useEffect } from 'react'

export default function useIsomorficEffect () {
  const isFirstRun = useRef(true)

  useEffect(() => {
    if (isFirstRun.current) {
      isFirstRun.current = false
      return
    }
    return () => {
      isFirstRun.current = true
    }
  }, [])

  return isFirstRun.current
}



And use it inside page or layout:

import useIsomorficEffect from 'hooks/useIsomorficEffect'

const isFirstRender = useIsomorficEffect()

 if (isFirstRender) {
    return null // or loader
  }
schmafu commented 3 years ago

it would really helpful if we can detect the first hydration action. @kirill-konshin you already check for "isFirstRender", if you could add an optional isInitialHydrate, something like:

store.current.dispatch({
    type: HYDRATE,
    payload: getDeserializedState(initialState, config),
    isInitialHydrate: isFirstRender,
} as any)

so i can check in my reducer for the initial hydrate and ignore the other hydrate actions:

const reducerWithHydrate = (state: RootState, action: AnyAction): RootState => {
    switch (action.type) {
        case HYDRATE: {
            if (!action.isInitialHydrate) {
                return state
            }
            const nextState: RootState = { ...state, ...action.payload }           
            return nextState
        }
        default:
            return rootReducer(state, action)
    }
}

in my opinion getInitialProps is still the best solution for a SPA with SSR. i don't want my nodejs server to fetch data for every client for every page request. the client is allowed to talk with the backend, the round trip over the nextjs node server is not necessary.

i guess nextjs is recommending getSSP/SP because a lot of people do not need a SPA.

kirill-konshin commented 3 years ago

@schmafu just put some flag in the store during first hydration and check it during following hydrations.

schmafu commented 3 years ago

@schmafu just put some flag in the store during first hydration and check it during following hydrations.

i am using redux-saga, so one possible solution would be to listen in a saga for the first HYDRATE action and then i need to dispatch a e.g. SET_HYDRATE action with the payload from HYDRATE and the reducerWithHydrate handles that one as well and sets the new state.

i guess that could work but it would be really nice if next-redux-wrapper could somehow distinct between the INITAL_HYDRATE and HYDRATE :) i think there is a valid use case for that.

juunzzi commented 3 years ago

image why this hydrate action two time in One Page Render time ?

kirill-konshin commented 3 years ago

why this hydrate action two time in One Page Render time ? This is by design due to nuances how Next.js works. It's mentioned in readme.

stegano commented 3 years ago

Hi kirill-konshin Thanks for your answer πŸ˜€ Sorry, but I haven't understood it yet. If only getInitialProps is used, is there any problem even if the Hydrate action does not occur after the initial SSR?

If you're dispatching actions at the server (in getInitialProps) you need to deliver this data to client on EVERY page load, when first opened and on subsequent navigation too.

Thank you for your answer and sorry to keep asking questions

In the case of using geInitialProps, the first server receives data and executes a hydrate action to synchronize the state delivered from the server and the client side state, When the page is moved, getInitalProps is executed on the client side, so it is immediately dispatched to the redux-store

So I think that there is no need to execute the hydrate action any more. please let me know if i'm wrong..

I made a utility that only synchronizes the server state when the first hydrate action occurs after the page is loaded. Hope this helps people who have similar problems.

Liamxiexuc commented 3 years ago

If it helps someone, then instead of export const getServerSideProps = wrapper.getServerSideProps( async ({ store }) => { I redid it to Page.getInitialProps = async ({ store }) => and this solves the problem.

Don't forget wrapper.withRedux (...)

Same issue, do u have any code example for this? And It seems in this way it would not compatible with Next.js 9's Auto Partial Static Export

Shaker-Hamdi commented 2 years ago

I have the exact same issue. When I add "getServerSideProps" to a page and navigate to it. the __NEXT__ REDUX WRAPPER HYDRATE action gets called and clears the state. If I remove "getServerSideProps" the state stays when navigating between pages. How to solve this?

This is how I'm setting up my store ...

import { HYDRATE } from "next-redux-wrapper";

import { AnyAction, combineReducers } from "@reduxjs/toolkit";

import { counterReducer } from "../features/counter";
import { globalReducer } from "../features/global/reducer";
import { userReducer } from "../features/user";

const combinedReducer: any = combineReducers({
  global: globalReducer,
  counter: counterReducer,
  user: userReducer,
});

export const rootReducer = (
  state: ReturnType<typeof combinedReducer>,
  action: AnyAction
) => {
  if (action.type === HYDRATE) {
    const nextState = {
      ...state, // use previous state
      ...action.payload, // apply delta from hydration
    };
    return nextState;
  } else {
    return combinedReducer(state, action);
  }
};
Shaker-Hamdi commented 2 years ago

I fixed my issue by implementing this line of documentation ...

if (state.count) nextState.count = state.count; // preserve count value on client side navigation,

So, here's my code ...

import { HYDRATE } from "next-redux-wrapper";

import { AnyAction, combineReducers } from "@reduxjs/toolkit";

import { counterReducer } from "../features/counter";
import { globalReducer } from "../features/global/reducer";
import { userReducer } from "../features/user";

const combinedReducer: any = combineReducers({
  global: globalReducer,
  counter: counterReducer,
  user: userReducer,
});

export const rootReducer = (
  state: ReturnType<typeof combinedReducer>,
  action: AnyAction
) => {
  if (action.type === HYDRATE) {
    const nextState = {
      ...state, // use previous state
      ...action.payload, // apply delta from hydration
    };
    if (state.user) nextState.user = state.user; //preserve user value on client side navigation
    return nextState;
  } else {
    return combinedReducer(state, action);
  }
};

The only question now is, should I do this to every piece of state that I have in order to preserve the value on client-side navigation?

If I have a state.product, I have to duplicate that line to be ... if (state.product) nextState.product= state.product;

That sounds like there should be a better way to handle this.

wallynm commented 11 months ago

Just to reinforce, this sugestion solved my issue aswell, I'm my project were using getServerSideProps, with each new render, the old props gets cleaned in a first setup of the Redux tree. Implementing this hydratation check at frontend level solved my issue aswell! Thank you by sharing it @Shaker-Hamdi

The server dispatches a action to clean the reducer, but with this check the frontend now decides to not clean the data! Might had missed this piece of info in the docs.

I fixed my issue by implementing this line of documentation ...

if (state.count) nextState.count = state.count; // preserve count value on client side navigation,

So, here's my code ...

import { HYDRATE } from "next-redux-wrapper";

import { AnyAction, combineReducers } from "@reduxjs/toolkit";

import { counterReducer } from "../features/counter";
import { globalReducer } from "../features/global/reducer";
import { userReducer } from "../features/user";

const combinedReducer: any = combineReducers({
  global: globalReducer,
  counter: counterReducer,
  user: userReducer,
});

export const rootReducer = (
  state: ReturnType<typeof combinedReducer>,
  action: AnyAction
) => {
  if (action.type === HYDRATE) {
    const nextState = {
      ...state, // use previous state
      ...action.payload, // apply delta from hydration
    };
    if (state.user) nextState.user = state.user; //preserve user value on client side navigation
    return nextState;
  } else {
    return combinedReducer(state, action);
  }
};

The only question now is, should I do this to every piece of state that I have in order to preserve the value on client-side navigation?

If I have a state.product, I have to duplicate that line to be ... if (state.product) nextState.product= state.product;

That sounds like there should be a better way to handle this.

haZya commented 7 months ago

If you don't want to manually check for the existing state for each reducer, it is possible to use something like a deepMerge to selectively merge the two states. There probably won't be a silver bullet solution but this should work okay for handling typical hydration during page navigation.

const isEmptyValue = (value: any) => {
  return (
    value === null ||
    value === undefined ||
    (typeof value === "number" && isNaN(value)) ||
    (Array.isArray(value) && value.length === 0) ||
    value === ""
  );
};

const deepMerge = (state: any, payload: any) => {
  const nextState = { ...state };

  for (let key in payload) {
    if (Object.prototype.hasOwnProperty.call(payload, key)) {
      const payloadValue = payload[key];
      const stateValue = state?.[key];
      if (payloadValue !== undefined && !isEmptyValue(payloadValue)) {
        if (Array.isArray(payloadValue) && !payloadValue.length) {
          nextState[key] = stateValue;
        } else if (isPlainObject(payloadValue)) {
          nextState[key] = deepMerge(stateValue, payloadValue); // Recursive call for nested objects
        } else {
          nextState[key] = payloadValue; // Replace with new value if it's not empty
        }
      }
      if (stateValue === undefined) {
        nextState[key] = payloadValue; // Additional fields from the payload
      }
    }
  }
  return nextState;
};

const reducer: typeof combinedReducer = (state, action: UnknownAction) => {
  if (action.type === HYDRATE) {
    return deepMerge(state, action.payload);
  } else {
    return combinedReducer(state, action);
  }
};