rt2zz / redux-persist

persist and rehydrate a redux store
MIT License
12.94k stars 866 forks source link

Problem with SSR on v5 #457

Open williamoliveira opened 7 years ago

williamoliveira commented 7 years ago

When state comes rehydrated from server it doesn't gets persisted anymore on the front.

I put a console.log inside setItem of the storage I am using (redux-persist-cookie-storage) and I it never gets called

My workaround was to remove _persists before inserting state into HTML on server side.

Is this a bug or is it by design?

rt2zz commented 6 years ago

The thought was that in a node SSR environment you would not persist. Is there a reason you need to or want to persist during SSR?

Two thoughts

  1. If you do skip persistStore during SSR, you also need to not render PersistGate
  2. if you do want to persist during SSR, then yes removing _persist is a workaround.

I am open to making adjustments. At a minimum it sounds like we need a simpler way to make PersistGate a passthrough component during ssr. maybing something like a disabled prop.

williamoliveira commented 6 years ago

Wont skip persistStore disable redux-persist altogether or the hydration part works independently? Cause I need the hydration to happen on the server so I can get the data from cookies

rt2zz commented 6 years ago

ok now I better understand. So there is a problem in the code, in these lines: https://github.com/rt2zz/redux-persist/blob/v5/src/persistReducer.js#L56-L70

we noop if persist state already exists. I think the fix may be as simple as adding one line before line 58:

if (!_persistoid) _persistoid = createPersistoid(config)

Its not clear to me however, in this case where the server has already rehydrated the state, do you want the frontend to rehydrate again? My guess is no in which case that one line above I think fixes the issue.

Let me think on this a bit longer before I make the change to the lib.

williamoliveira commented 6 years ago

I dont need redux-persist to rehydrate again on the client because I am already rehydrating redux directly with initialState, the problem is it doesn't persists changes to state anymore on the client

so the problem is: if _persist is there, further state persisting is disabled

williamoliveira commented 6 years ago

Oh, I see what you mean now, in my case _persistoid is null at https://github.com/rt2zz/redux-persist/blob/v5/src/persistReducer.js#L132, your proposed change seems to be enough

rt2zz commented 6 years ago

just published rc.2 with this: https://github.com/rt2zz/redux-persist/blob/v5/src/persistReducer.js#L57-L58

I think that solves the problem

williamoliveira commented 6 years ago

Thanks!

wootencl commented 6 years ago

As this issue is still open I seem to be having the same problem of setItem not being called and figured I would raise it here. Fairly new to redux so definitely possible I've configured something wrong. I'm also using custom cookie based storage via universal-cookie (instead of redux-persist-cookie-storage as I had already used it elsewhere in my project). Here are the relevant files:


configureStore.js

import { persistStore, persistReducer } from 'redux-persist';
import { createStore, applyMiddleware, compose } from 'redux';
import thunk from 'redux-thunk';
import { CookieStorage } from '../utils';
import rootReducer from '../reducers';
import createHelpers from './createHelpers';
import createLogger from './logger';

export default function configureStore(initialState, helpersConfig, initialCookies = undefined) {
  const helpers = createHelpers(helpersConfig);
  const middleware = [thunk.withExtraArgument(helpers)];
  const persistConfig = {
    key: 'root',
    storage: CookieStorage(initialCookies),
    debug: __DEV__
  };

  const reducer = persistReducer(persistConfig, rootReducer);

  let enhancer;

  if (__DEV__) {
    middleware.push(createLogger());

    // https://github.com/zalmoxisus/redux-devtools-extension#redux-devtools-extension
    let devToolsExtension = f => f;
    if (process.env.BROWSER && window.devToolsExtension) {
      devToolsExtension = window.devToolsExtension();
    }

    enhancer = compose(applyMiddleware(...middleware), devToolsExtension);
  } else {
    enhancer = applyMiddleware(...middleware);
  }
  // See https://github.com/rackt/redux/releases/tag/v3.1.0
  const store = createStore(reducer, initialState, enhancer);
  // eslint-disable-next-line
  const persistor = persistStore(store);

  // Hot reload reducers (requires Webpack or Browserify HMR to be enabled)
  if (__DEV__ && module.hot) {
    module.hot.accept('../reducers', () =>
      // eslint-disable-next-line global-require
      store.replaceReducer(require('../reducers').default),
    );
  }

  return store;
}

CookieStorage.js

import Cookies from 'universal-cookie';
import moment from 'moment';
import Promise from 'bluebird';

export default function (initialCookies = undefined) {
  const cookies = new Cookies(initialCookies);
  return {
    getItem: (key) => (new Promise((resolve, reject) => {
        try {
          const s = cookies.get(key);
          resolve(s);
        } catch (e) {
          reject(e);
        }
      })
    ),
    setItem: (key, data) => (new Promise((resolve, reject) => {
        try {
          // TODO Set `httpOnly:true` + `secure:true`
          cookies.set(key, data, { 
            path: '/',
            expires: moment().add(3, 'd').toDate()
          });
          resolve();
        } catch (e) {
          reject(e);
        }
      })
    ),
    removeItem: (key) => (new Promise((resolve, reject) => {
        try {
          cookies.remove(key);
          resolve();
        } catch (e) {
          reject(e);
        }
      })
    )
  }
}

Store creation in server.js

...
const store = configureStore(initialState, {
      axios: createAxios(config.api.url),
    }, req.headers.cookie);
...
williamoliveira commented 6 years ago

@wootencl did you update redux-persist to 5.0.0-rc.2 ?

wootencl commented 6 years ago

I did. Still no luck sadly 😞

luongvm commented 6 years ago

Is this issue still relevant?

I happened to have SSR rehydrate working, that is, when curl with an appropriate cookie, the response is hydrated (for example, the counter number changed). Using cookie as storage js-cookie on client-side and cookies with NodeCookiesWrapper on server-side.

There's just another issue with this: on client side, the ReactDOMServer.hydrate call will complain about unmatched data because that on the first call, persistStore haven't run yet and not have the same data from cookie.

But when I delay the hydrate call by putting it in the persistStore callback, it then won't attach the event handlers correctly.

I think it's expected because when the page is loaded, the hydrate event wont happen until persistStore finished its run, and of course if you tried to hydrate before you have the same data from cookie, it will be unmatched data.

So as opposed to official ReactDOMServer recommendation, I used renderToStaticMarkup then use render (outside persistStore callback) and it worked well (no complain about unmatched data).

Am I doing it wrong?

rt2zz commented 6 years ago

@joturako I have never done SSR but I have a suspicion that if you are using redux persist cookie storage then you actualy do not want the FE to rehydrate at all. Instead you want initial redux state to be initialized with whatever the backend rehydrated from cookies.

Perhaps this is more of a question for SSR in general with redux: how does the server set initial state for redux on the client? I think if I better understood that then we could make it work with the redux-persist api. E.G. one option is to not have redux-persist rehydrate at all, but still write state updates. That would look roughly like:

import { createPersistoid } from 'redux-persist'

// later with store in scope
let persistoid = createPersistoid(persistConfig)
store.subscribe(() => persistoid.update(store.getState())
luongvm commented 6 years ago

@rt2zz my general flow is: when you request (for example curl) with cookie to the server, it then will parse the cookie -> rehydrate from said cookie -> render from rehydrated state -> client receive the page. Then client-side script run, redux-persist get called and rehydrate from the same cookie because that's the cookie get sent to the server, so the final render is the same.

The problem I described in my previous comment, there's one factor caused that trouble: the cookie set by the server is httpOnly, which mean even after client-side booted & redux-persist kicked in and rehydrated, it does nothing because it can't read the cookie at all.

This is my working bits

import { CookieStorage } from 'redux-persist-cookie-storage'

const orgCS = new CookieStorage(cookies, {
    setCookieOptions: { httpOnly: false }
  })
const config = {
...
  storage: orgCS
... 
}

persistCombineReducers(config, {todoReducers})

For server-side that cookies is this cookieJar

import { NodeCookiesWrapper } from 'redux-persist-cookie-storage'
import Cookies from 'cookies'
 const cookieJar = new NodeCookiesWrapper(
    new Cookies(req, res, {
      // secure: false,
      httpOnly: false
    })
)

For client-side that cookies is this Cookies

import Cookies from 'js-cookie'

Then you call persistStore and wait to hydrate/response after it's finished. It will not complain about un-matched data afterward.

rt2zz commented 6 years ago

ah nice - we should add this to the docs

luongvm commented 6 years ago

Hey there, just want to add some gotcha here: your v5.4.0 work with no problem. but recently I ran into a crash with a newer version. even after await store.flush() =>res.send, the store on the server-side never stop listen to update and thus trigger another cookie write (by setting header on res variable) and make the server crash: 'Can\'t set headers after they're sent'. I guess that's because of this line

+    if (whitelist && whitelist.indexOf(key) === -1 && key !== '_persist') return false
-    if (whitelist && whitelist.indexOf(key) === -1) return false

So if anybody ran into a sudden crash with the same error please check the version. I'm thinking of a fix, probably a flag/function to stop the persist<->update cycle on the server-side, because it will not make sense after we already call res.send.

rt2zz commented 6 years ago

hm, there is no way to "destroy" persistence, but you can pause the persistoid via persistor.pause() I wonder if that might solve your issue?

Still it seems to me that if you are still getting updates after sending the response thats means the store and the persistor are both still in memory on your server which would cause memory to grow forever?

luongvm commented 6 years ago

ah, now that you mention it, this could be a potential memory leak if everything is not destroyed after we handle the request and send the response back. I will investigate next week and see if it grows like you said. In any case I just want to keep the flow work like on the client-side, that is: fetch from storage, populate the store, read from store and render. But on the server-side I guess there's one more step: tearing down everything after we sent a response. Have a nice weekend.