Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

props and state passed to observe() are undefined after first state change #25

Closed
jsierles opened this issue Apr 24, 2015 · 12 comments
Closed

Comments

@jsierles
Copy link

When my component is mounted, the props are available. However, after I perform a setState, both props and state are undefined.

@gkrcode
Copy link

gkrcode commented Apr 25, 2015

I'm having the same issue.

@agnosticdev
Copy link

Where are you calling setState from?

@gkrcode
Copy link

gkrcode commented Apr 27, 2015

sample code in todo demo to add pagination

var TodoList = React.createClass({
  mixins: [ParseReact.Mixin],

  getInitialState: function() {
    return {skip:0, limit: 3}
  },

  observe: function(props, state) {
   // state is undefined after initial mount
    return {
      items: (new Parse.Query('TodoItem')).ascending('createdAt').skip(state.skip).limit(state.limit)
    };
  },

  render: function() {
    var self = this;
    // If a query is outstanding, this.props.queryPending will be true
    // We can use this to display a loading indicator
    return (
      <div className={this.pendingQueries().length ? 'todo_list loading' : 'todo_list'}>
        <a onClick={this._refresh} className="refresh">Refresh</a>
        {this.data.items.map(function(i) {
          // Loop over the objects returned by the items query, rendering them
          // with TodoItem components.
          return (
            <TodoItem key={i.id} item={i} update={self._updateItem} destroy={self._destroyItem} />
          );
        })}
        <a onClick={this._nextPage}>Next</a>
      </div>
    );
  },

  _nextPage: function() {
    this.setState({skip: this.state.skip + this.state.limit });
  },

  _refresh: function() {
    this.refreshQueries('items');
  }

...

 });

@kidwm
Copy link

kidwm commented Apr 27, 2015

I have to downgrade to 0.1.4 for avoiding this issue.

@agnosticdev
Copy link

Weird, you could always just reference the values directly as getInitialState is called before observe

observe: function(props, state) {
    //do not even use state or props but reference the values directly
    return {
      items: (new Parse.Query('TodoItem')).ascending('createdAt').skip(this.state.skip).limit(this.state.limit)
    };
},

@kidwm
Copy link

kidwm commented Apr 27, 2015

@agnosticdev Thanks for your advice. It worked, but this.data.items will be undefined in my case.

@gkrcode
Copy link

gkrcode commented Apr 27, 2015

@agnosticdev this.state gives the values before the setState call

observe(props, state) is called whenever the component will update, before it renders. At mount time, the initial props and state are passed as function parameters. Each successive time, the upcoming props and state are passed as parameters.

@kidwm
Copy link

kidwm commented Apr 27, 2015

@gkrcode Yes, you're right. No wonder it did not give any result in my case.

@agnosticdev
Copy link

@gkrcode yes, this is a good point.

@andrewimm
Copy link
Contributor

I have to say, I really appreciate the level of community discussion happening here around such a young library.

This particular bug is due to an (embarrassing) oversight, thankfully fixed by #28. My local test suite didn't have a case for changing state after initialization.
I'm going to incorporate these changes tonight, as well as the fixes to the problems in #27, and do a new npm release (0.2.1) by tomorrow. I'm also going to focus on open sourcing the more complex test suites in the near future so that they can be added to the repository.

For those interested, the discussion around the observe API on the React repo has picked up in the last few days, and a lot of ideas are being tossed around there. I think the observe lifecycle method provides an ideal, universal interface for subscribing to assorted components, but I'm beginning to question how dependent it should be on things like state. There's still a lot of discussion that needs to take place before it becomes a cemented piece of React, but all of your input is valuable.

@flovilmart
Copy link

@andrewimm actually both strategies would be valid depending on the purpose of 'observables'.

React pushes forward strong DOM elements reuse as it's pretty difficult to ask for a componenent unmount. In my case, I have an 'ObjectEditor', which is a highly reusable component that take a parseClassName and objectId as props. From there with type inference, I have a proper a nice CMS.

The issue for me is when I switch objects being rendered, (which is just the state of the parent component) that editor doesn't get unmounted, therefore, updating the props to the new objectId is what make sense. I ended up monkey patching the function to suit my needs, not responding to state changes, only props changes.

maybe there should be an exposed (not _) function so we can trigger the observe lifecycle outside the common prop update.

@andrewimm
Copy link
Contributor

Just released 0.2.1 on npm and ParseCDN. This issue should be resolved now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants