Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage with redux-persist #68

Closed
chapati23 opened this issue Aug 4, 2017 · 18 comments
Closed

Usage with redux-persist #68

chapati23 opened this issue Aug 4, 2017 · 18 comments

Comments

@chapati23
Copy link
Contributor

How would I use this with redux-persist?

  // routesMap.js
  …
  WALLET_SELECTOR: {
    path: '/wallet-selector/:currency',
    thunk: (dispatch, getState) => {

       // I don't seem to have the rehydrated state here.
       // Is it somehow possible to 'await' rehydration here?
      const { steps } = getState()

      canContinueIf(steps.currencySelected, dispatch)
    }
  },
  …
@faceyspacey
Copy link
Owner

I don't currently know, but I need to find out. Can you try applying it directly to the demo in isolation and see if there are any problems:

https://github.com/faceyspacey/redux-first-router-demo

Probably the best way to figure out how redux-persist plays with SSR + rehydration.

@ganlub
Copy link

ganlub commented Aug 11, 2017

I implemented it along with RFR and everything works as expected. There will be actions being fired before the re-hydration happens so you will need to rather use this or this.

@faceyspacey it would be nice to have it on the demo!

@chapati23
Copy link
Contributor Author

Here's how I solved it. Maybe it's helpful to somebody. Had to use the cookies adapter for redux-persist so I had the state I'm interested in on the server already. Not sure if it's really the best solution but it works for me :)

// src/routesMap.js
// …
    thunk: (dispatch, getState) => {
      const { steps } = getState() // up to you to handle via standard redux techniques
      canContinueIf(steps.someStepCompleted, dispatch)
    }
// …
const canContinueIf = (stepCompleted, dispatch) => {
  if (!stepCompleted) {
    const action = redirect({ type: 'SOME_DEFAULT_ROUTE' })
    dispatch(action)
  }
}
// server/configureStore.js
export default async (req, res) => {
  // …
  const cookies = req.cookies
  const parsedCookies = {}
  Object.entries(cookies).forEach(([key, value]) => {
    const decodedKey = decodeURIComponent(key)
    const keyWithoutReduxPersistPrefix = decodedKey.replace(/reduxPersist:/, '')
    if (key !== 'reduxPersistIndex') {
      parsedCookies[keyWithoutReduxPersistPrefix] = JSON.parse(value)
    }
  })
  const preLoadedState = { ...parsedCookies } // onBeforeChange will authenticate using this
  // …
}
// src/configureStore.js
export default (history, preLoadedState) => {
  // …
  const store = createStore(rootReducer, preLoadedState, enhancers)

  if (!isServer) {
    persistStore(store, {
      blacklist: ['location', 'page'],
      storage: new CookieStorage()
    })
  }
// …
}
// server/index.js
import cookieParser from 'cookie-parser'
// …
app.use(cookieParser())
// …

@faceyspacey
Copy link
Owner

faceyspacey commented Aug 12, 2017

excellent! ...i think it's likely the only way--how else are we going to guarantee the client state becomes redux state on the server? Headers doesn't work, since direct visits can't contain headers like ajax requests in SPAs. Cookies is the only option.

Very well done. I'll probably even document this approach at some point. If you're up to, feel free to make a pr with a new .md in the docs folder. Feel free to give yourself credit with a link, etc.

@faceyspacey
Copy link
Owner

...perhaps a more formal solution/package would let you blacklist keys so huge amounts of domain state isn't stored in cookies (which would slow down requests). We only want UI state after all.

Or perhaps there is a reason to have domain state--black/whitelist options would let the user determine what should be stored in cookies.

@faceyspacey
Copy link
Owner

@ganlub so the idea is that to buffer and delay actions that enhancers like the one from RFR might trigger, so that when route thunks are called, they have everything they need from redux-persist as well? Very pro!

@chapati23
Copy link
Contributor Author

@faceyspacey created a little doc locally. tried creating a feature branch, commiting with npm run cm and then pushing but don't have permissions. what's the workflow?

@chapati23 chapati23 reopened this Aug 12, 2017
@chapati23
Copy link
Contributor Author

figured it out, here goes: #78

@faceyspacey
Copy link
Owner

excellent. was just writin.

@faceyspacey
Copy link
Owner

im just gonna close this, since we have the PR (doing a little cleanup)

@brownbl1
Copy link

brownbl1 commented Aug 13, 2017

I know you closed this, @faceyspacey, but I wanted to add a few more details that I've been teasing out over the past hour.

First, I liked the idea of using redux-action-buffer as @ganlub suggested so I went down that route. My idea was that I could add the action buffer middleware to queue up and fire all actions (including the initial location action) only after the persist/REHYDRATE action is fired. The catch though is I was trying this with the redux-persist@next package. The problem is that in the v5 rewrite of redux-persist, they also dispatch a persist/PERSIST action before the persist/REHYDRATE. It seems that persist/REHYDRATE isn't fired until persist/PERSIST is fired. See this issue for reference: rt2zz/redux-action-buffer#14

I've created a tentative pull request in redux-action-buffer to effectively whitelist certain actions (namely persist/PERSIST) which fixes this use case of redux-action-buffer.

Using this approach, if we set up our configureStore to look like this:

const middlewares = applyMiddleware(
	createActionBuffer(REHYDRATE, null, [PERSIST]),
	middleware,
	thunk,
	logger
)

I get the desired be behavior and my action sequence looks like:
persist/PERSIST
persist/REHYDRATE
HOME // redux-first-router action
INCIDENTS_FETCHED

Does that make sense and do you see any issues with this? I'm really only messing around with it in the browser (not native or server at this point). It's possible I've missed a bigger problem here but this seems to solve all of my issues with not having access to the persisted store in onBeforeChange or route thunks.

It should probably be stated clearly that I haven't yet had a chance to see how this should work with the universal suite you've been working on. My very limited use case is that I am making an ajax request to a lambda function which returns a jwt that I'd like to store in localStorage. I don't have a server at all at this point, and I'm not trying to store all of my app state in localStorage; just the user token/info.

Also, thanks for the great work on this router!

@faceyspacey
Copy link
Owner

faceyspacey commented Aug 14, 2017

Thanks for taking the time to do this analysis!

...so i havent had time to focus on redux-persist personally unfortunately. So that means I'm lacking in answers.

But I can say this: i dont really like the fact that now u gotta add createActionBuffer. It's just one more thing. It's the reverse steve-jobs "one more thing" where u dont get something u want in that "one more thing." A never ending set of negative "one more things" lol.

So to me that's usually a signal to dig my heals in at some point and take a holistic approach and see if we can do "better."

I still can barely wrap my head around what the buffer thing is doing since i havent tried it--I get it, it's delaying actions. But maybe there's a better way. Why can't we just get a damn callback or await on redux persist somehow. Maybe we can get a PR into the v5 that lets u do this. If this is a genuine problem, they should build in a solution to it, not provide 3rd party hacks via additional buffer middleware.

Feel free to let me know if I'm far off in my limited birds eye perspective.

@brownbl1
Copy link

No problem. Just thought it might be helpful to someone else, but I agree it feels like a hack and should be viewed as a stop-gap solution. And I'd love to have all of this become effortless, as do you!

I don't think you're far off in seeing that this approach isn't ideal, but I'm not really sure how an await/hook into redux-persist would look.

Maybe the persistStore method could return an onRehydrated callback on the returned Persistor type? See: https://github.com/rt2zz/redux-persist/blob/v5/docs/api.md#persiststorestore

Then we could hook into that inside onBeforeChange et. al. Other than that, though, not 100% sure how it would work, and haven't taken the time to dig into the redux-persist code yet.

My PR should be merged shortly though over on rt2zz/redux-action-buffer#15 so that solution should work for now for my and maybe other use cases.

@ganlub
Copy link

ganlub commented Aug 16, 2017

@faceyspacey I ended up going with the initialDispatch: false, I wasn't aware of this property, so I removed the redux-action-buffer because it's not needed at all.

In case it helps somebody, persistStore allows you to add a callback when the re-hydration is complete, so there you can call initialDispatch(). There's as well a gap of a few milliseconds while all of this happens, so I added a spinner so it won't render a blank screen. I have a page reducer (I took this approach from the official demo) and I set the initial state like this:

export default (state = LOADING, action = {}) => {
  return components[action.type] || state
}

const components = {
  ROUTE_HOME,
  ROUTE_LOGIN,
  NOT_FOUND
}

@brownbl1
Copy link

@ganlub Are you using the v5 branch of redux-persist? If so, I couldn't find the callback as I believe it's being removed. A few people have suggested adding it back, which I would be in favor of.

@ganlub
Copy link

ganlub commented Aug 16, 2017

@itslittlejohn Oh I see, no I'm using v4.8.3.
I haven't had the chance of checking v5.

@brownbl1
Copy link

Not to worry though. Seems like the callback behavior is coming back: rt2zz/redux-persist#408

@ghost
Copy link

ghost commented Jan 16, 2018

I tried out the callback that you pass to persistStore and I found that it only fires once.

I need to do something every time REHYDRATE happens (to check if the user is logged out). So the action buffer should work for me.

EDIT: The action buffer actually only fires the callback once as well, when it releases the buffer. So, I ended up just handling REHYDRATED in one of my reducers and then setting a timeout to fire an action to redirect the user to the login page when rehydration logged them out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants