react-router-component icon indicating copy to clipboard operation
react-router-component copied to clipboard

getInitialStateAsync fires any time component receives new props

Open gaearon opened this issue 11 years ago • 11 comments

I'm not sure if this is by design, but I see getInitialStateAsync firing whenever component receives new props.

I'm not sure if this is the right/desired behavior. In my getInitialStateAsync I fetch remote data and update my root model (passed via props). This triggers render() on the root component. And so getInitialStateAsync fires again, ad infinitum.

Perhaps this is a bad/unsupported use case for getInitialStateAsync because the component doesn't own this state anyway and I pass {} to callback. But the idea of preparing a component is useful even if async “state” does not go to this.state but means preparing some model in props.

Maybe I should write my own mixin to do this. Still, is it a bug or not? Should receiving new props always trigger getInitialState?

Call stack:

screen shot 2014-03-30 at 4 48 15

gaearon avatar Mar 30 '14 00:03 gaearon

A bug.

After a fix if your async state depends on props (you construct URL from props or something like that) you would need to trigger async state update manually (componentDidMount or componentWillReceiveNewProps).

andreypopp avatar Mar 30 '14 09:03 andreypopp

Fixed in 0.18.0. See the example of updating async state in case of dependence of it on this.props — https://github.com/andreypopp/react-quickstart/blob/master/client.js#L33-L54

andreypopp avatar Mar 30 '14 09:03 andreypopp

Sorry, it still goes into an infinite async loop for me.

Condition currentHandler.type === nextHandler.type is false because currentHandler is previous active tab, and nextHandler is the tab I'm trying to prefetch. So this again triggers setState and prefetchAsyncState and next tab is never loaded.

gaearon avatar Mar 30 '14 10:03 gaearon

Condition currentHandler.type === nextHandler.type is false because currentHandler is previous active tab, and nextHandler is the tab I'm trying to prefetch.

If previous active tab and the next tab are of the same component type then their .type will be identical. So the question is are your active tab and the next tab components of the same type? If not then getInitialStateAsync should be triggered (this matches getInitialState semantics).

The infinite loop maybe caused by the fact that your state doesn't converge. For example when you have state A you trigger update to state B and vice-versa. It's hard to say without looking at the actual code.

andreypopp avatar Mar 30 '14 10:03 andreypopp

The infinite loop maybe cause by the fact that your state doesn't converge. For example when you have state A you trigger update to state B and vice-versa. It's hard to say without looking at the actual code.

I mean you probably have to do somewhere:

...
getInitialStateAsync: function(cb) {
  if (!this.props.model.isPrepared) {
    this.props.model.prepare(cb);
  }
},
...

And of course model's internal state should survive rendering root component.

andreypopp avatar Mar 30 '14 10:03 andreypopp

They're not of the same type, no. But it's weird to have getInitialStateAsync triggered twice:

  • when I click tab switching link;
  • when the model is ready and render is called on root component. 

I can't give you a test case right now but this should reproduce it:

On link click, navigate to an async component. This component's gISA should wait three seconds, then call method that re-renders root component, then call gISA callback with {} as state.

In this case, the second step (calling root render) triggers gISA again and so it goes.

Д

On Sun, Mar 30, 2014 at 2:42 PM, Andrey Popp [email protected] wrote:

The infinite loop maybe cause by the fact that your state doesn't converge. For example when you have state A you trigger update to state B and vice-versa. It's hard to say without looking at the actual code. I mean you probably have to do somewhere:

...
getInitialStateAsync: function(cb) {
  if (!this.props.model.isPrepared) {
    this.props.model.prepare(cb);
  }
},
...

Reply to this email directly or view it on GitHub: https://github.com/andreypopp/react-router-component/issues/29#issuecomment-39022488

gaearon avatar Mar 30 '14 11:03 gaearon

They're not of the same type, no.

If they are not of the same type when it getInitialStateAsync should be triggered (as well as getInitialState which is a model for getInitialStateAsync semantics).

  • when I click tab switching link;
  • when the model is ready and render is called on root component.

You should just check in getInitialStateAsync if your model is ready or not before calling any async actions. I think this is reasonable cause your handlers don't own the state and so should assume it can be in different states before performing update actions on it.

andreypopp avatar Mar 30 '14 11:03 andreypopp

Though I may miss some important details and if you have suggestions on how it should behave I'd like to hear them and improve react-router-component/react-async! But as far as I understand, in this case the decision to fire on not to fire some async action depends on some knowledge which is specific to the application — router cannot know if model is ready or not.

andreypopp avatar Mar 30 '14 11:03 andreypopp

If they are not of the same type when it getInitialStateAsync should be triggered (as well as getInitialState which is a model for getInitialStateAsync semantics). Maybe I didn't makes myself clear. gISA is triggered twice on the same (second) component. For comparison, gIS is triggered only once (right before mounting).

gaearon avatar Mar 30 '14 11:03 gaearon

So they are of the same type?

It looks like this:

var Tab = React.createClass({
  getInitialStateAsync: function(cb) { ... },
  ...
})

var App = React.createClass({
  render: function() {
    return (
      <Locations>
        <Location path="/1" handler={Tab} tabName="someTab" />
        <Location path="/2" handler={Tab} tabName="otherTab" />
      </Locations>
    )
  }
})

right? In this case getInitialStateAsync should be triggered once for a transition from /1 to /2 and vice-versa.

Sorry, I might be slow sunday mornings :-)

andreypopp avatar Mar 30 '14 11:03 andreypopp

I meant getInitialStateAsync should be triggered once and transtions from /1 to /2 and vice versa shouldn't trigger it. Just the initial one.

andreypopp avatar Mar 30 '14 11:03 andreypopp