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

Need something more robust to handle React Router params #5

Open
benlesh opened this issue May 19, 2016 · 5 comments
Open

Need something more robust to handle React Router params #5

benlesh opened this issue May 19, 2016 · 5 comments

Comments

@benlesh
Copy link
Contributor

benlesh commented May 19, 2016

According to the documentation on component lifecycle with react-router, users should be making ajax requests and using route params both at componentDidMount and componentDidUpdate. It seems like we might need a decorator (or the decorator) to handle more lifecycle hooks than just dispatchOnMount.

We can either do several decorators for each lifecycle hook we care about, which would incur the overhead of more higher-order components, or, we can have a single HoC to rule them all.

@connect(stuff, stuff)
@dispatchOnMount(stuff)
@dispatchOnUpdate(stuff)
class MyComponent extends React.Component {
  // stuff
}

// or something like 
const MyComponent = compose(
  connect(stuff, stuff),
  dispatchOnMount(stuff),
  dispatchOnUpdate(stuff)
)((props) => (<div>weee</div>));

Or the one lifecycle decorator idea:

@connect(stuff, stuff)
@dispatch({
  onMount: [stuff, stuff],
  onUpdate: [stuff]
})
class MyComponent extends React.Component {
  // stuff
}

// or something like 
const MyComponent = compose(
  connect(stuff, stuff),
  dispatch({
    onMount: [stuff, stuff],
    onUpdate: [stuff]
  })
)((props) => (<div>weee</div>));
@benlesh
Copy link
Contributor Author

benlesh commented May 19, 2016

Worth considering: Should the the list of dispatchable things be provided by a factory? Like dispatchOnMount(() => [(props) => getThunkservable(props), (props) => getThunkservable2(props)]) or perhaps dispatchOnMount((props) => [getThunkservable(props), getThunkservable2(props)])?

@benlesh
Copy link
Contributor Author

benlesh commented May 19, 2016

Also, dispatch probably isn't a good name for the decorator, because of collisions with other variables names... perhaps dispatchOn

benlesh added a commit that referenced this issue May 19, 2016
dispatchOn is a decorator that allows the user to define many lifecycle hooks that dispatch thunkservables as part of a single higher-order component. Allowed hooks are `willMount` (componentWillMount), `mount` (componentDidMount), `update` (componentDidUpdate), and `willReceiveProps` (componentWillReceiveProps)

closes #5

BREAKING CHANGE: `dispatchOnMount` is removed in favor of `dispatchOn`
@BerkeleyTrue
Copy link

Have y'all taken a look at redux-epic containers https://github.com/BerkeleyTrue/redux-epic/tree/master/docs/api#contain

The idea behind: The presentational component requires a certain piece or pieces of data to be present, this can be taken care of by one action. The actually lifecycle hooks aren't important to the user, they just care that the data is there. We provide helper methods for that container that can determine when not to call the action, for instance if the data is already primed.

It becomes the fetch container, redux's connect provides the data container, and the user provides the presentational component.

A good question: What is the main goal of this HoC?

@BerkeleyTrue
Copy link

ping @jayphelps

@jayphelps
Copy link
Member

@BerkeleyTrue we're not thrilled on the existing code in here and I personally use some other HoCs I've created that aren't here yet. We're just experimenting at this point. I'll look at redux-epic's when I get a chance. 👍

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

Successfully merging a pull request may close this issue.

3 participants