reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.54k stars 15.28k forks source link

Best practice on handling data flow for login / signup pages with redirect #297

Closed emmenko closed 8 years ago

emmenko commented 8 years ago

EDIT: what I'm trying to understand here is how to handle data flow when you have a page with a form. After you submit the form you either redirect to another page or render an error message.

The challenge is where this information should be temporary stored in the data flow. Because after you redirect or render the message, the state (or information you stored) is not relevant anymore and should be reset or cleaned.


In a backend web application you usually have pages like login, signup, reset password, etc.

When the user navigates to the app and is not logged in, he gets redirected to login page. This can be easily done with RR onEnter hook.

Then the user will fill up the form and submit it. At this point usually if the request was successful you want to redirect to application root or to some other "secured" page. If it failed, you want to stay on the page and show an error message.

(This applies not only for login page, but other pages as well as mention before. I will just focus on the login for now)

So far you have this flow (which is common use case):

If we implement this flow with redux, I guess I have following setup:

// actions
function login (form) {
  return dispatch => doLogin(form)
    .then(() => dispatch({ type: 'LOGGED_IN' }))
    .catch(() => dispatch({ type: 'LOGIN_FAILED' }))
}

// reducer
const initialState = {
  // some fields...,
  loggedIn: false,
  shouldRedirect: false,
  errorMessage: null
}
function application (state = initialState, action) {
  switch action.type {
    case 'LOGGED_IN':
      return Object.assign({}, state, { loggedIn: true, shouldRedirect: true })
    case 'LOGIN_FAILED':
      return Object.assign({}, state, { loggedIn: false, shouldRedirect: false, errorMessage: action.error.message })
  }
  return state
}

// component
@connect(state => { application: state.application })
class Login extends React.Component {
  componentWillUpdate () {
    const { router } = this.context
    const { application } = this.props
    if (application.shouldRedirect)
      router.transition(...)  
  }

  onSubmit () {
    const actions = bindActionCreators(applicationActions, dispatch)
    actions.login({...})
  }

  render () {
    const { errorMessage } = this.props
    return (
      <div>
        {errorMessage ? <p>{errorMessage}</p> : null}
        <Form {...props} onSubmit={onSubmit}>
          {...}
        </Form>
      </div>
    )
  }
}

Given this flow is correct - hopefully :) - you can see that I have 2 flags in the application state: shouldRedirect and errorMessage. Those flags are only used when submitting the form.

First question: is that ok to have those flags in the application state?

Another thing to consider is that I have to "reset" those flags when I redirect because if I want to go to another page (e.g. signup, ...) I should have a clean state ({ shouldRedirect: false, errorMessage: null }).

So I might want to dispatch another action like

// actions
function resetSubmitState () {
  return { type: 'RESET_SUBMIT' }
}

// component
componentWillUpdate () {
  const { store, router } = this.context
  const { application } = this.props
  if (application.shouldRedirect) {
    store.dispatch(applicationActions.resetSubmitState())
    router.transition(...)  
  }
}

Have anybody had this problem? Am I missing something or is that the "correct" way to do it?

Are there any other alternatives?

Any feedback is very welcomed! :)

johanneslumpe commented 8 years ago

Instead of having shouldRedirect in there, how about having a session reducer, which just tracks isLoggedIn? That way you can redirect from your login page to any other page by checking isLoggedIn instead of shouldRedirect.

Also, resetting the state could be done by just processing the LOGGED_IN action in your reducer. If that action happens you know you can safely reset the error state to a null value.

Does this make sense? :D

emmenko commented 8 years ago

It makes sense if there is just the login page, but it's a bit more generic approach. For example for a signup page, or password reset page. Even if you are logged in you should still be able to see those pages.

And those flags should be used by all those pages, because they all have the same flow.

Hope it's more clear now :)

johanneslumpe commented 8 years ago

Yeah, from my point of view "shouldRedirect" is just too generic in order to be associated with a certain action. It might open up bugs where you somehow forget to set that flag and then a page redirects to another page to which it actually shouldn't redirect.

But sure, if you want to clean your state, you can just do that with an action. Or you could listen to a history change outside of react and then trigger the state-clearing action. That might be preferable because you don't have to scatter that logic throughout your components but you can keep it in a single place.

But that is only a possible solution for the state clearing. I'm not sure yet how to solve the redirection in a nice way.

emmenko commented 8 years ago

The question is also: does it make sense to have those flags in the state at all? If not, how should this flow be implemented?

emmenko commented 8 years ago

@gaearon if you have some time I'd love to hear your feedback as well. Thanks! :+1:

michalkvasnicak commented 8 years ago

In my application I am redirecting in middleware and I don't use any flags. But I am not using react-router.

// action

/**
 * Creates login action
 *
 * @param {string} email
 * @param {string} password
 *
 * @returns {{types: *[], promise: *}}
 */
export function login(email, password) {
    return {
        types: [LOGIN_REQUEST, LOGIN_SUCCESS, LOGIN_FAILURE],
        promise: post('/login').send({ email: email, password: password }).promise()
    };
}

// auth middleware 
/**
 * Intercepts LOGIN action and redirects to login screen
 * Otherwise just sends action to next middleware
 *
 * @returns {Function}
 */
function authMiddleware({getState, dispatch}) {
    return (next) => (action) => {
        if (typeof action === 'object' && action.hasOwnProperty('type')) {
            if (action.type === LOGIN_SUCCESS) {
                next(action); // send it to next so identity will be set

                // get current route
                const state = getState();
                let path = '/dashboard';

                if (typeof state['router'] === 'object' && typeof state['router']['route'] === 'object' && null !== state['router']['route']) {
                    if (state.router.route.name === 'login' && typeof state.router.route.query['to'] === 'string') {
                        path = state.router.route.query.to;
                    }
                }

                return next(actions.transitionTo(path));
            }
        }

        return next(action);
    };
}

It is not refactored, it is inteded only for test purposes of my router solution :).

frederickfogerty commented 8 years ago

This is a copy and paste from my application and how I handle loading user data and logging in. I'll try to comment the code as best as possible, and I'll be on reactiflux for questions.

const AppConnector = createConnector((props$, state$, dispatch$, context$) => {
  // true when the user is logged in - drawn from Store state
  const loggedIn$ = state$
    .map(s => s.apiKey !== null)
    .distinctUntilChanged()

  // this returns an observable of a path to redirect to
  // in my app, I have a url like `/login?nextPath=dashboard`, but if that
  // isn't set, just go home
  const redirectTo$ = routeState$
    .map(s => s.router.query.nextPath || '/home')

  const redirect$ = loggedIn$
    .withLatestFrom(
      context$,
      redirectTo$,
      (loggedIn, context, path) => () => context.router.transitionTo(loggedIn ? path : '/login') // when the loggedIn state changes, i.e. user has logged in or out, respond to that change
    )
    .do(go => go())

  // It's awesome to have this here, because it means that no matter 
  // where the user loads in to the app (all child views of this view), 
  // the app will realise it needs to load data, and loads the according data 
  const userData$ = state$
    .map(getUser)

  const userDataIsEmpty$ = userData$
    .map(user => user.id === null || typeof user.id === 'undefined')
    .filter(s => s === true)

  const loadUserData$ = Rx.Observable.combineLatest(loggedIn$, userDataIsEmpty$, (logged, userDataIsEmpty) => loggedIn && userDataIsEmpty)
    // only when user is logged in but has no user data
    .filter(s => s === true)
    // fetch data with an action creator
    .withLatestFrom(dispatch$, (userData, dispatch) => () => dispatch(UserActions.loadUserData()))
    .forEach(go => go())

  return Rx.Observable.combineLatest(props$, state$, context$, redirect$, (props, state, context) => ({...props, ...state, ...context}))
}, render)
frederickfogerty commented 8 years ago

In terms of answering your questions @emmenko, I think having shouldRedirect in your state is not acceptable, and should be done in a more functional approach. Having errorMessage in application state is fine imo, just make sure it's under a login state, such as state.auth.errorMessage, rather than a top-level errorMessage

gaearon commented 8 years ago

I think having shouldRedirect in your state is not acceptable, and should be done in a more functional approach

I agree. Things that are only useful once shouldn't be in the state IMO.

frederickfogerty commented 8 years ago

Also, as per slack, I don’t think you should be storing isLoggedIn in application state, as it is more really a computed property, which can be calculated with a simple selector like const isUserLoggedIn = state => state.user.id !== null

See https://github.com/faassen/reselect for more selector usage

emmenko commented 8 years ago

@frederickfogerty thanks for the example.

Question though: this works fine for login because you usually have a loggedIn flag in the state. But what about other pages like signup, password-reset, etc? Those pages behave the same as login: redirect if ok, otherwise show error message. So how do you handle those cases? I'm struggling to find the correct flow for those cases, and I'm not sure where the state should live.

Things that are only useful once shouldn't be in the state IMO.

@gaearon I agree, but I'm still not sure what's the correct approach for that. You still need to have a state somewhere...

This is how I'm doing it atm without storing anything in the state.

// actions
function login () {
  return { type: 'LOGGED_IN' }  
}

function submitLogin (form, dispatch) {
  return api.doLogin(form)
  .then(() => {
    dispatch(login()) // can dispatch something at this point
    return Promise.resolve()
  })
}

// in the component (login, signup, ...)
onSubmit () {
  actions.submitLogin(form, dispatch) // this returns a promise
  .then(() => this.setState({ shouldRedirect: true }))
  .catch(error => this.setState({ shouldRedirect: false, errorMessage: error }))
}

I don’t think you should be storing isLoggedIn in application state, as it is more really a computed property

Yep, makes sense.

emmenko commented 8 years ago

@gaearon I guess the question for this general workflow (redirect or show error) is if it should be done with "redux" or in a different way. And what would be this alternative?

therealmarv commented 8 years ago

related and I will pin it here... actually I was also dealing with this problem and currently trying to reimplement following example into Redux. Nothing showable yet because I'm doing this in my free time: https://auth0.com/blog/2015/04/09/adding-authentication-to-your-react-flux-app/

It is pure Flux: https://github.com/auth0/react-flux-jwt-authentication-sample

Maybe it is useful for best practices?!

frederickfogerty commented 8 years ago

@emmenko

So how do you handle those cases?

As long as your using is logging in, then isLoggedIn (which isn't in app state, but minor) is still changed, and this will redirect them. It doesn't care where that state change came from :)

The best approach I've taken is to add this logic to a connector where the functionality is needed for its children e.g. for authorisation, say you have a AppProtected component, where everything that is a child of that component needs authorisation, then you put this logic in the connector of the AppProtected component.

This is how I'm doing it atm without storing anything in the state.

I definitely don't like that approach, that has two way data flow between the UI and the ACs, which goes against flux. If you're tied to doing it imperatively, you could do something like

componentWillReceiveProps(nextProps) {
  if (['/login', '/sign-up'].indexOf(this.props.router.path) !== -1 && this.props.isLoggedIn) {
    this.context.router.transitionTo(this.props.router.query.nextPath || '/home')
  }
}

This works for all the pages you mentioned

emmenko commented 8 years ago

Sorry but I still don't see how I handle the redirect / error rendering.

Let's take a step back and say I have the reset-password page. We also assume that we should not touch any state by storing some kind of one-shot flags (as mentioned before). When I click to submit the form, what should happen?

This is what I would like to know, how the flow should be and where should it live.

PS: thanks for your feedback so far!

frederickfogerty commented 8 years ago
[navigates to reset-password page] -> 
[types in email and hits submit] -> 
[onSubmit fires bound AC] ->
on successful reset this continues with:
[RESET_PASSWORD_SUCCESS] -> 
[resetPassword reducer returns state {passwordReset: true}] ->
[resetPasswordComponent.componentWillReceiveProps will check if state.passwordReset is true, and then will transition to its route]
on a failed reset this continues with:
[RESET_PASSWORD_FAILURE] -> 
[resetPassword reducer returns state {passwordResetError}] -> 
[resetPasswordComponent.componentWillReceiveProps() will check if state.passwordReset is true, and since is it not, nothing will happen - in render(), the error will be displayed]

This is nowhere near as elegant as for the login and sign up pages, as it is very coupled

frederickfogerty commented 8 years ago

I have to sleep, I'll answer any more questions tomorrow

emmenko commented 8 years ago

So we are storing the flags in the state again ;)

PS: good night!

stremlenye commented 8 years ago

I think the question is a little bit deeper then just generalising 'submit forms' – it is more about what actually the state is and which data should appear in store and which not.

I like to think about View part of flux data flow as pure function render(state): DOM but it's not totally true. Lets take as an example simple form component which renders two input fields:

class Form extends React.Component {

  onFieldChanged (event) {
    this.setState({[event.target.name]: event.target.value})
  }

  render () {
    return (
      <form onChange={::this.onFieldChanged}>
        <input type="text" name="name" />
        <input type="text" name="surname" />
        <input type="email" name="email" />
      </form>
    )
  }
}

During typing into this fields some internal state modifications performed so we will have something like this at the end:

{
  name: 'John'
  surname: 'Snow'
}

So we got some state which doesn't linked to the application state and nobody cares about it. When we will revisit this form next time it will be shown erased and the 'John Snow' will be gone forever. Looks like we modified DOM without touching flux data flow.

Lets assume that this is subscription form for some notification letter. Now we should interact with outer world and we'll do it using our special action.

import { subscribe } from 'actions'

class Form extends React.Component {

  onFieldChanged (event) {
    this.setState({[event.target.name]: event.target.value})
  }

  onSubmit (event) {
    event.preventDefault()
    const { name, surname, email } = this.state
    subscribe(name, surname, email)
  }

  render () {
    return (
      <form onChange={::this.onFieldChanged} onSubmit={::this.onSubmit}>
        <input type="text" name="name" />
        <input type="text" name="surname" />
        <input type="email" name="email" />
        <button type="submit">Subscribe</button>
      </form>
    )
  }
}

When user pushes submit button he expect any response from the UI was it succeed of not and what to do next. So we want to keep track the request state to render loading screen, redirect or show error message. The first idea is to dispatch some internal actions like loading, submitted, failed to make application state responsible for that. Because we want to render the state and don't think about details.

  render () {
    //provided by connector of whatever
    const { props: { isLoading, error } } = this
    return (
      <form onChange={::this.onFieldChanged} onSubmit={::this.onSubmit}
        disabled={isLoading}>
        <input type="text" name="name" />
        <input type="text" name="surname" />
        <input type="email" name="email" />
        <button type="submit">Subscribe</button>
        {error ? <p>{ error }</p> : null}
      </form>
    )
  }

But here the devil appears: now we have this page state persisted and next render(state) call will display form in a dirty state, so we need create cleanup action to call it each time we mount form. And for each form we have we will add same boilerplate of loading, submitted, failed and cleanup actions with stores/reducers… until we stop and think about the state we actually want render.

Does submitted belongs to application state or it is just component state, like name field value. Does the user really care about this state to be persisted in a cache or he want to see the clean form when he revisit it? The answer: it depends.

But in case of pure submit forms we could assume that the form state has nothing to do with application state: we want to know the fact is user logged in or not in case of login page, or we don't want to know anything about has he changed password or failed: if request succeed just redirect him somewhere else.

So how about proposition we don't need this state somewhere else except Component? Lets give it a try. To provide more fancy implementation we could assume the in 2015 we live in the asynchronous world and we have Promises everywhere, so let our action return a Promise:

function subscribe (name, surname, email) {
  return api.request('/subscribtions', 'create', { name, surname, email })
}

At next step we could start keeping track of the action in our Component:

onSubmit (event) {
  event.preventDefault()
  const { name, surname, email } = this.state
  subscribe(name, surname, email)
    .then(() => { this.setState({ submitted: true }) })
    .catch(error => { this.setState({ error }) })
}

componentWillUpdate (object nextProps, object nextState) {
  if(nextState.submitted)
    redirect('somewhere')
}

Looks like we've got everything we need to render component without polluting application state and got rid of need to clean it. So we are almost there. Now we could start generalising Component flow. First of all lets split to concerns: keeping track of action and state update reactions: (Lets do it in terms of redux to stay in repo bounds)

import React, { PropTypes } from 'react'
import { bindActionCreators } from 'redux'

// Keeps track of action
export default function connectSubmitForm (Form, submitAction) {
  return React.createClass({
    contextTypes: {
      //redux Store
      store: PropTypes.object.isRequired
    },

    getInitialState () {
      return {}
    },

    onSubmit (...args) {
      const { context: { store: { dispatch } } } = this
      const { submitAction: submit }
        = bindActionCreators({ submitAction }, dispatch)
      submit(...args)
        .then(() => this.setState({ submitted: true }))
        .catch(error => this.setState({ error }))
    },

    render () {
      const {
        onSubmit,
        props,
        state: { submitted, error }
      } = this
      return (<Form {...props} onSubmit={onSubmit} submitted={submitted}
        error={error} />)
    }
  })
}

and

// redirect to path if predicate returns true
export default function redirect (path, predicate) {
  return Component =>
    class Composed extends React.Component {

      componentWillMount () {
        if (predicate(props))
          redirectTo(path)
      }

      componentWillReceiveProps (nextProps) {
        if (predicate(nextProps))
          redirectTo(path)
      }

      render () {
        return <Component {...this.props} />
      }
    }
}

//redirect to path if submitted
export default function redirectSubmitted (path) {
  return redirect(path, ({ submitted }) => submitted)
}

Now on one hand we have decorator which provide submitted and error props with onSubmit callback and on other hand decorator which redirects somewhere if gets the property submitted === true. Lets attach them to our form and see want we get.

@redirectSubmitted('/')
class Form extends React.Component {

  onFieldChanged (event) {
    this.setState({[event.target.name]: event.target.value})
  }

  onSubmit (event) {
    event.preventDefault()
    const { name, surname, email } = this.state
    this.props.onSubmit(name, surname, email)
  }

  render () {
    return (
      <form onChange={::this.onFieldChanged} onSubmit={::this.onSubmit}>
        <input type="text" name="name" />
        <input type="text" name="surname" />
        <input type="email" name="email" />
        <button type="submit">Subscribe</button>
        {this.props.error ? <p>{ this.props.error }</p>: null}
      </form>
    )
  }
}

export default submitForm(Form, subscribe)

And here it is: pure render form with no dependencies of routing library, flux implementation, application state, doing what it was created for: submit and redirect.

Thanks for reading and sorry for taking your time.

gaearon commented 8 years ago

Fair enough! To be honest I'm still interested in looking at storing local component state in Redux: #159.

iclanzan commented 8 years ago

@stremlenye That is pretty slick. Thank you for sharing!

stremlenye commented 8 years ago

@iclanzan thx for reading :) Actually now I am thinking about @gaearon proposal he mentioned in previous comment. Take a look at #159 discussion to get some significant ideas.

gaearon commented 8 years ago

Closing—when RR 1.0 comes out, we'll cover this in “Usage with React Router” recipe. This can be tracked in https://github.com/rackt/redux/issues/637.

tomazzaman commented 8 years ago

@stremlenye your approach is pretty awesome! Would it make sense to - instead of using a decorator - use a react-router higher order component in which all the unathorized ones would be nested and it would be responsible for redirecting once the auth store would have a user in?

<ReduxRouter>
    <Route component={ Auth }>
      <Route path="/" component={ Login } />
      <Route path="/reset-password" component={ ResetPassword } />
    </Route>
  </ReduxRouter>

Something like this, where the Auth only takes care of detecting if the user is logged in. This way, I still get to manipulate my store by dispatching actions from the Login component. Not so pure, but less code and easier to get my head around :)

stremlenye commented 8 years ago

@tomazzaman Thanks :) Most of the React applications I've wrote included something like ApplicationContainer which could be used for the same purpose without new Route implementation. If you are using RR-v1.0.0-rc1 you could reach same goals with onEnter hook: just check if current state matches to authenticated or not.

ir-fuel commented 8 years ago

I've been wrapping my head around this too, and it seems @stremlenye did a great job showing how it can be done. The problem is that one tends to get stuck in the "everything needs to go through the action - reducer - update-component-with-store-state loop" (me too) and that we then tend to pollute the store with all kinds of state that's only used in very specific scenarios in the app (on 1 screen, stuff such as "did I succesfully execute the reset password"), whilst this can be easily handled by just doing the async calls inside the component and updating the component itself using this.setState instead of polluting the global state + having to reset all those state vars in case we were to open that component a second time later on. Not even mentioning polluting the state reducers with all the begin/success/error handling code.

Will try adapting my code here too to see if it cleans things up.

stremlenye commented 8 years ago

@ir-fuel if will have time could you push a small example of what you will get to the github?

ir-fuel commented 8 years ago

Well it will be hard to do that, but I can give a small example of what I did. I have a component where a user can ask to reset his password. The actual reset operation happens like this now:

  submitForgotPassword = (username) => {
    this.setState({isLoading:true})
    doForgotPassword(username).then((result) => {
      this.setState({passwordIsReset:true,isLoading:false})
    }).catch((result) => {
      this.setState({passwordIsReset:false,isLoading:false,errorMessage:result.error})
    })
  }

doForgotPassword is a function that does an ajax call to my server.

So no roundtrips to the store/reducers, no polluting the global state with stuff you only care about inside of one component and is purely used to temporarily show state in that component, and no need to reset a bunch of state vars inside your reducers.

and render looks something like this

  render() {

    let divClassName = ''

    if(this.state.isLoading) {
      divClassName = 'disable-controls'
    }

    let child = null

    if(this.state.passwordIsReset)
      child = (<ForgotPasswordSuccess/>)
    else
      child = (<ForgotPasswordForm submit={this.submitForgotPassword} errorMessage={this.state.errorMessage}/>)

    return (
      <div className={divClassName}>
        {child}
      </div>
    )
  }

I hope this is clear.

stremlenye commented 8 years ago

Looks good :+1: The only trick that should be covered (from my perspective) is adjusting middleware to return promise out of the lifted action (just proxy the return value of the action probably). Doing this we will allow action to modify state in a normal flow if is needed keeping the opportunity to hook up onto the Promise stuff in component scope.

ghost commented 8 years ago

i think it's ok to think about your app state as a already-logged-in-state. if no user logged, why do u need an app state? sure, you could have one inside the Login component.

app logic at Login page shouldn't be that complex to own an app top state, right?

sompylasar commented 8 years ago

@SkateFreak Login flow is a part of an app. Didn't you think of a login flow state (progress, error), signup flow state etc. for which you'd better use React, too (likely with Redux, if you're already using it for other app components).

ghost commented 8 years ago

@sompylasar i am in favor of an app state, i just think that as long as the user is OUTSIDE of my app, i can serve him a simpler app without a redux store. my redux store will kick in as soon as you ENTER my app.

this approach worked for me until this very moment, but maybe i will understand the need for a login-phase app state in my future projects

ir-fuel commented 8 years ago

You can at the root level of your store split your store in the 'app' part and the 'user/login' part, each with its own reducer. So in that case you will never pollute your app reducers with login reducer code and vice versa.

From: SkateFreak notifications@github.com<mailto:notifications@github.com> Reply-To: rackt/redux reply@reply.github.com<mailto:reply@reply.github.com> Date: Sunday 13 December 2015 13:48 To: rackt/redux redux@noreply.github.com<mailto:redux@noreply.github.com> Cc: Joris Mans joris@astus.be<mailto:joris@astus.be> Subject: Re: [redux] Best practice on handling data flow for login / signup pages with redirect (#297)

@sompylasarhttps://github.com/sompylasar i am in favor of an app state, i just think that as long as the use is OUTSIDE of my app, i can serve him a simpler app without a redux store. my redux store will kick in as soon as you ENTER my app.

this approach worked for me until this very moment, but maybe i will understand the need for a login-phase app state in my future projects

Reply to this email directly or view it on GitHubhttps://github.com/rackt/redux/issues/297#issuecomment-164287194.

ghost commented 8 years ago

correct me if im wrong but isn't the purpose of a flux store to share data and app-state between routes and components? why do i need to "reduce" my state and use my "actions" to maintain a top-state if all of it won't matter when the user sign in?

i don't see the advantages, although it is a very organised and clean way to do things. i manage my pre-login state at my <Sign /> component, which contains SignIn, SignUp and ForgotPassword.

ir-fuel commented 8 years ago

Because a signup might be a more complicated process than just asking for a username and a password. It all depends on the complexity of your flow.

From: SkateFreak notifications@github.com<mailto:notifications@github.com> Reply-To: rackt/redux reply@reply.github.com<mailto:reply@reply.github.com> Date: Sunday 13 December 2015 14:00 To: rackt/redux redux@noreply.github.com<mailto:redux@noreply.github.com> Cc: Joris Mans joris@astus.be<mailto:joris@astus.be> Subject: Re: [redux] Best practice on handling data flow for login / signup pages with redirect (#297)

correct me if im wrong but isn't the purpose of a flux store to share data and app-state between routes and components? why do i need to "reduce" my state and use my "actions" to maintain a top-state if all of it won't matter when the user sign in?

i don't see the advantages, although it is a very organized and clean way to do things. i manage my pre-login state at my component, which contains SignIn, SignUp and ForgotPassword.

Reply to this email directly or view it on GitHubhttps://github.com/rackt/redux/issues/297#issuecomment-164287893.

ghost commented 8 years ago

i agree. but i see SignUp as a form, with a state - which should be sufficient no matter how complex imo...

on the other hand, what do i know about sign ups... people are crazy

kpaxqin commented 8 years ago

Currently I've meet some similar but different problem.

I'm working on a flow like a super complex sign up, and we have some continues page to let user input required info, each page is a form. The flow looks like:

FormPage_A -> FormPage_B -> FormPage_C -> SuccessPage

The problem is that we don't make any API request before FormPage_C. We just save data in browser on FormPage_A/B and send all of them to API in FormPage_C.

So, the question is should we put the form data of FormPage_A and FormPage_B into application state?

If yes, it seems we're saving some temporary data that has no effect on UI into application state. And since the application state of redux lives in memory, what will happen when user refresh page in FormPage_B?

If no, were is the right place to put it ? Should we build another layer like fake api out of redux flow to save it ?

@frederickfogerty @gaearon @ir-fuel @stremlenye

ir-fuel commented 8 years ago

I would just pass the properties in this.state that are relevant to your submit as props to the next step. so FormPage_A.state will be passed in FormPage_B.props etc. and do the submit in FormPage_C

or if you have a parent component that manages this, store the state in that parent control's state property and after the last step submit all

Just make sure that if the user decides to reload the page that you start from the beginning again.

stremlenye commented 8 years ago

@kpaxqin I think in this case you could declare a managing component like Flow which will handle the subset of steps with their states and generic back-forward logic for each. So you could make your steps as stateless as possible and keep track of the state of whole process in enclosing component. Something like:

<Flow onForward={…} onBackward={}>
  <Step1 />
  <Step2 />
  <Step3 />
  …
  <Finish onSubmit={this.props.submit(this.state.model) />
</Flow>
kpaxqin commented 8 years ago

@ir-fuel @stremlenye

Thanks for your suggestions. It seems that we all don't want to put those temporary things into application state.

But when user refresh page in half of the flow

Just make sure that if the user decides to reload the page that you start from the beginning again.

It seems not a good user experience, we're a mobile site (that's one of the reason why we split huge form into small pages), it's painful to input things on mobile, and I believe user will go mad if they lose all the things after hit the refresh button, maybe they'll just give it up.

I think maybe build the fake api out of redux flow is an acceptable choice, submit things to the fake api on each step and retrieve them when needed just like we have api for it, and since we use it like api, the only thing connect to it is action creators, pages and components will be clean and it's also easy to handle error (use the common code for handle api error).

It make sense for me because once we want to persistent those data, it shouldn't in component state or application state, the side effect should be handled in action creator

//fake api
const fakeApi = {
  getStepA: ()=>{
    /* get things from LS */
    return Promise
  },
  setStepA: ()=>{
    /* put things into LS */
    return Promise
  }
}

// action creator
submitA(dispatch, getState) { // thunk function
  fakeApi
    .setStepA()
    .then(dispatchSuccess)
    .then(transitionToStepB)
    .catch(dispatchError);
}

It make things clear for me but it looks weired to add something our of redux flow into our app.

ir-fuel commented 8 years ago

I really wouldn't care about the user reloading your page. On the other side, you tell us that you have a complicated sign up process with several forms, and then you say it's a mobile site. Don't you think it will also drive users mad and have them give up if they have to fill out too many things on a mobile device? I think it's even worse than not supporting reload. I don't think many sites support page reloading in the middle of a signup process anyway.

I think it's just making things a lot more complicated for something that really doesn't happen a lot. I would focus more on trying to simplify the sign up process, or split it in multiple steps over your website so the user does not have the impression he is filling out X pages of forms before finally getting his account active.

And if you really want to support mid-signup reload, just have the parent component that controls all these steps store its state in localStorage.

sompylasar commented 8 years ago

@kpaxqin I would go with the "fake API" approach. Why fake BTW, it's just intermediate storage API that you need for your multi-step process. Not every API should necessarily be on a server and accessed via HTTP or other networking protocol.

What I agree with @ir-fuel upon is that all the multi-step process state should be stored, not just the current step.

@ir-fuel Reload could happen without explicit user intent, for example, a user can switch to another mobile app, like a notebook, or a phone call, and the browser could lose its state because of insufficient memory on the device, so it will reload when the user comes back.

ir-fuel commented 8 years ago

The problem again is that you are polluting global application state with something that you only need in a few screens in your webapp.

stremlenye commented 8 years ago

@sompylasar @kpaxqin I think @ir-fuel suggested the right approach – if you need something intermediate to be eventually persisted, persist it on the client side: much easier to implement, doesn't really involve server into persisting inconsistent data, doesn't perform any http requests (what is also a major problem on mobile devices) and extremely easy to change the approach in future.

And about persisting any data somewhere – think how you will clean it after process failed or done already – that was the initial theme of a topic. Another option which you could use – persist user data in the url. If your forms consist of couple text fields that could be the good tradeoff solution.

ir-fuel commented 8 years ago

It's not so much "store it client or server side", it's more a matter of "do I handle this 'ad hoc' in my parent component class" vs "do I do the whole action/reducer/state dance". I don't like going through the action/reducers if it is for temporary state. I only do this for real application state.

stremlenye commented 8 years ago

@ir-fuel :+1:

sompylasar commented 8 years ago

@stremlenye Please read my answer again. I say explicitly that you don't have to make a server-side API to make a persistence API layer.

@ir-fuel Not going through actions/reducer cancels predictability and replayability of the state transitions.

sompylasar commented 8 years ago

@ir-fuel You can easily clear your app state after the form is submitted, so it does not get "pollutted".

sompylasar commented 8 years ago

RE: clearing. Make an action CLEAR and have a button in your UI to reset the sign-up flow. You can make it via router, too (dispatch that action when visiting the initial signup URL, and if the signup has started, change the URL to something else).

stremlenye commented 8 years ago

@sompylasar there is no problem to reset your state with an explicit button – problems occur after unhandled failures: it is hard to predict the ways state could be corrupted and define a restore strategy for each particular case.