reduxjs / redux

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

Why dehydration resets dispatcher? #51

Closed vslinko closed 9 years ago

vslinko commented 9 years ago
export default async function renderPage(url) {
  const dispatcher = createDispatcher(composeStores(stores))

  const {title} = await dispatcher.perform(runTransition(url))

  // I need to get atom to make some checks
  const {atom} = dispatcher.dehydrate()

  if (!atom.routerStore.componentKey) {
    return {
      status: 404
    }
  }

  // But after that checks I should return atom into dispatcher for render usages
  // I want to remove this step
  dispatcher.hydrate({atom})

  const pageContent = React.renderToString(
    <Provider dispatcher={dispatcher}>
      {() => <Application dispatcher={dispatcher} />}
    </Provider>
  )

  return {
    body: appTemplate(title, pageContent, atom)
  }
}
gaearon commented 9 years ago

Should I add a separate method like dispose? I'm not sure what the convention is regarding (de)hydration and disposal.

acdlite commented 9 years ago

It seems we just need a way to access the atom.

dispose() seems like the better word for a method that clears the internal state. dehydrate() in other Flux libraries more closely means "convert state to a serialized string so I can send it to the client."

vslinko commented 9 years ago

I can't imagine that someone needs to dispose dispatcher. Btw I have some API proposes:

// 1. current
const hydro = dispatcher.dehydrate()
dispatcher.hydrate(hydro)

// 2. extract dispose
const hydro = dispatcher.dehydrate()
dispatcher.dispose()
dispatcher.hydrate(hydro)

// 3. create getAtom method
const atom = dispatcher.getAtom()
dispatcher.hydrate({atom})
acdlite commented 9 years ago

We need to dispose the dispatcher inside componentWillReceiveProps() to support prop changes / hot reloading. Right now we use the methods dehydrate() and hydrate() but I propose that dispose() and initialize() are closer to what those methods actually do.

See https://github.com/gaearon/redux/blob/master/src/components/Provider.js#L26-L30

acdlite commented 9 years ago

:+1: for getAtom() method

I think hydrate() works as a way to replace existing atom, but I don't know if it should also deal with subscriptions...

gaearon commented 9 years ago

Yes, the use case for disposing the dispatcher is hot reloading.

I'm done for today but I'd merge a PR that addresses this. @acdlite Can you come up with an API to satisfy both use cases? I don't mind adding more methods if it makes server rendering more convenient.

acdlite commented 9 years ago

For server rendering:

For hot reloading:

I'll submit a PR when I get home.

gaearon commented 9 years ago

Looks good to me!

Maybe initialize({ atom, subscriptions })? So it matches the shape.

acdlite commented 9 years ago

Good idea!

gaearon commented 9 years ago

I just added getAtom in master (for a different purpose though): ff56611fdb3b599519fd8998dcd8d650d1e0b683

gaearon commented 9 years ago

Closed by #54