brocoders / redux-async-connect

It allows you to request async data, store them in redux state and connect them to your react component.
645 stars 103 forks source link

Proof of concept of deferred props #24

Open sergesemashko opened 8 years ago

sergesemashko commented 8 years ago

This a proof of concept for having deferred props.

Usage

Pass deferred props as a 2nd parameter to @asyncConnect:

@asyncConnect({...props}, {...deferredProps})

Under the hood

TODO:

@sars, let me know what do you think

sars commented 8 years ago

@sergesemashko Thank you, for this PR, we actually discussed this approach here: https://github.com/erikras/react-redux-universal-hot-example/pull/872#discussion_r50781541 Is it what you are trying to solve?

To be honest, i don't really like this approach - I mean new static method and so on... trying to find more elegant solution....

sergesemashko commented 8 years ago

Thanks for the feedback, @sars. I understand. But I would like to know if there are more specific concerns about static methods, etc. or your vision how it would be better to implement this feature. Let's list points or concerns and work towards them. I would like to help to make deferred props happen in this project. Thanks

krzysztofpniak commented 8 years ago

Hey, any news when async props can be available here? Without them I do not really see benefit of having loading and loaded properties in store.

sars commented 8 years ago

@krzysztofpniak I'm working on v1.0.0 there will be several changes there including deferred props should be ready in 1-2 days

sars commented 8 years ago

@sergesemashko, @krzysztofpniak , take a look please: https://github.com/Rezonans/redux-async-connect/tree/v1

there is new interface for @asyncConnect decorator in this version.

Now you can use it like:

@asyncConnect([
  {key: 'lunch', promise: ({params, helpers}) => helpers.client.get('/lunches/' + params.lunchId), group: 'smth'}
])

and then in your client.js:

const component = (
  <Router render={(props) =>
        <ReduxAsyncConnect {...props} filter={item => item.group === 'smth'} helpers={{client}} />
      } history={browserHistory}>
    {getRoutes(store, client)}
  </Router>
);

and server.js:

loadOnServer({...renderProps, store, helpers: {client}}).then(() => {
  // ...
}

Does it works for you?

sergesemashko commented 8 years ago

hey @sars, sorry for the delay. I assume there should an additional filter for server:

loadOnServer({...renderProps, store, helpers: {client}}).then(() => {
  const appHTML = renderToString(
        <Provider store={store} key="provider">
          <ReduxAsyncConnect filter={item => item.group !== 'deferred'} {...renderProps} />
        </Provider>
      )
}

Right?

From the first look, this way of defining deferred props not too obvious and requires patching of the application with filter. It would make sense to make it more explicitly, I think.

Overall, it should work. Just curious, what are the other purposes of filters aside deferred props?

Thanks

mmahalwy commented 8 years ago

@sars is the purpose of the array suppose to be sequential? Also, is the static method going away?

sars commented 8 years ago

@mmahalwy no, all promises invokes simultaneously like in previous version static method still can be used, but it looks much inconvenient now, it's interface changed as well.