goatslacker / alt

Isomorphic flux implementation
http://alt.js.org/
3.45k stars 323 forks source link

Async: multiple requests don't update spinner #594

Open Ganasist opened 8 years ago

Ganasist commented 8 years ago

I've setup a spinner in a component to show while my Action / Store make an async request. The spinner shows as expected on the first request, but any subsequent requests do not trigger the spinner (although the console is clearly logging my request-in-progress / request-received events). It is also showing me that the loading: true and loading: false state is being set in my Store. The only way I can get the spinner to show again is to hard refresh my browser in the dev environment.

Any idea why, in my component, the async spinner only shows on first request? I'm using Webpack Hot Loader if that's any help. Here is the component in question:

import connectToStores  from 'alt/utils/connectToStores'
import React  from 'react'
import CustomerActions  from '../actions/customer-actions.js'
import CustomerStore  from '../stores/customer-store.js'
import { Col, ProgressBar } from 'react-bootstrap'

@connectToStores
export default class Greeting extends React.Component {
  constructor(props) {
    super(props)
    CustomerActions.fetchCustomers()
  }

  static getStores() {
    return [CustomerStore]
  }

  static getPropsFromStores() {
    return CustomerStore.getState()
  }

  componentWillReceiveProps(nextProps) {
    console.log('componentWillReceiveProps', nextProps)
    console.log('loading', this.props.loading)
  }

  render() {
    if (this.props.customers) {
      var customers = this.props.customers.map((customer) => {
        return (
          <li key={customer.id}>{customer.email}</li>
        )
      })
    }

    if (this.props.loading) {
      var progressBar = <ProgressBar bsStyle='info active' now={100} striped label="Loading" />
    }

    return (
      <Col>
        { progressBar }
        {this.props.children}
        <h1>Hello again, {this.props.name}!</h1>
        <ul className='list-unstyled'>
          { customers }
        </ul>
      </Col>
    )
  }
}
//
// Greeting.propTypes = { loading: React.PropTypes.bool }
// Greeting.defaultProps = { loading: true }
cezarsmpio commented 8 years ago

I've a similar issue... My getting flag doesn't get true if I don't have the timout function in getAllDone.

My action:

class EventoActions {

  getAll(page = 1) {
    API.Evento
      .getAll({
        params: {
          page: page
        },
        headers: {
          'Authorization': `token ${User.getToken()}`
        }
      })
      .then((res) => {
        this.getAllDone(res);
      });

    return page;
  }

  getAllDone(res) {
    return res;
  }

}

And my store:

export class EventoStore {

  constructor() {
    this.state = {
      eventos: {},
      getting: false,
      pageActive: 1
    };

    this.bindActions(EventoActions);
  }

  onGetAll(page) {
    console.log('on get all - getting true');
    this.setState({
      pageActive: page,
      getting: true
    });
  }

  onGetAllDone(res) {
    setTimeout(() => {
      console.log('on get all done - getting false');
    this.setState({
      eventos: {
        [this.state.pageActive]: res.data.results
      },
      getting: false
    });
    }, 2000);
  }
}
lsanwick commented 8 years ago

I have a very similar situation, where the loading action does occur and the Store has the correct data, but there doesn't appear to be an emitChange to trigger a re-render on my UI.

Here's an example: https://gist.github.com/lsanwick/577a047bd1f44a2e5919

If replace the componentWillMount with this:

setTimeout((-> OrderStore.fetchOrders()), 1)

It will not get re-rendered correctly when the loading event occurs without a timeout applied, which makes me think it's related to the React component lifecycle.

lsanwick commented 8 years ago

After lengthy experimenting and digging through the code, changing when I call my action fixes the problem.

componentWillMount: ->
  OrderStore.fetchOrders()

becomes

@componentDidConnect: ->
  OrderStore.fetchOrders()

componentDidConnect is in the connect-to-stores module, but is completely undocumented, so I'm not sure if what I'm doing is intended.

cezarsmpio commented 8 years ago

Any solution? We will continue use setTimeout for setState delays?

Thanks!

lsanwick commented 8 years ago

@CezarLuiz0, not sure about your case, if you set a debugger in onGetAll does it get caught?

@Ganasist, I think this will fix your problem:

@connectToStores
export default class Greeting extends React.Component {
  constructor(props) {
    super(props)
  }

  static getStores() {
    return [CustomerStore]
  }

  static getPropsFromStores() {
    return CustomerStore.getState()
  }

  static componentDidConnect() {
    CustomerActions.fetchCustomers()
  }

  ...

If that doesn't fix it, can you paste in what your store/actions look like?

cezarsmpio commented 8 years ago

My problem is, my first change in store, It doesn't work, I don't know, the store don't call the listen method in the View/Component.

To fix this, I need do this:

// store
onGetAll(page) {
  setTimeout(() => {
    this.setState({
      pageActive: page,
      getting: true
    });
  }, 1);
}
lsanwick commented 8 years ago

@CezarLuiz0, can you show the code for your View/Component?

cezarsmpio commented 8 years ago

Sure.

Component:

'use strict';

import React from 'react';
import { Link } from 'react-router';

import {
  Table,
  TableBody,
  TableRow,
  TableHeader,
  TableHeaderColumn,
  TableRowColumn,
  TableFooter,
  RaisedButton,
  FlatButton,
  Paper,
  IconButton,
  Toggle
} from 'material-ui';

import Loading from 'components/ui/loading/LoadingComponent';

// Actions
import EventoActions from 'actions/EventoActions';

// Store
import EventoStore from 'stores/EventoStore';

class EventosComponent extends React.Component {

  constructor(props) {
    super(props);

    this.state = {
      store: EventoStore.getState()
    };

    this.onEventoChange = this.onEventoChange.bind(this);
    this.rows = this.rows.bind(this);
  }

  componentDidMount() {
    EventoActions.getAll(1);

    EventoStore.listen(this.onEventoChange);
  }

  componentWillUnmount() {
    EventoStore.unlisten(this.onEventoChange);
  }

  onEventoChange(state) {
    this.setState({
      store: state
    });
  }

  rows() {
    let store = this.state.store;

    if (is.not.undefined(store.eventos[store.pageActive])) {
      let rows = store.eventos[store.pageActive].map((evento, i) => {
        return (
          <TableRow key={i}>
            <TableRowColumn>{evento.name}</TableRowColumn>
            <TableRowColumn>{evento.category.name}</TableRowColumn>
            <TableRowColumn>{evento.starts_at}</TableRowColumn>
            <TableRowColumn>{`${evento.city} - ${evento.state}`}</TableRowColumn>
            <TableRowColumn>{evento.status ? 'Publicado' : 'Inativo'}</TableRowColumn>
            <TableRowColumn>
              <IconButton
                containerElement={<Link to={`/dashboard/eventos/${evento.id}`} />}
                linkButton
                iconClassName="material-icons"
              >
                mode_edit
              </IconButton>
            </TableRowColumn>
          </TableRow>
        );
      });

      return rows;
    }

    return null;
  }

  render() {
    let store = this.state.store;

    return (
      <Loading when={store.getting}>
        <div className="eventos-component">
          <section className="component-section">
            <header className="row middle-xs component-section-header">
              <div className="col-xs-6">
                <h3 className="section-title">Eventos</h3>
              </div>
              <div className="col-xs-6 end-xs">
                <RaisedButton
                  label="Novo Evento"
                  containerElement={<Link to="/dashboard/eventos/novo" />}
                  linkButton
                  primary
                />
              </div>
            </header>

            <Paper>
              <Table
                selectable={false}
              >
                <TableHeader>
                  <TableRow>
                    <TableHeaderColumn>Título</TableHeaderColumn>
                    <TableHeaderColumn>Categoria</TableHeaderColumn>
                    <TableHeaderColumn>Começa em</TableHeaderColumn>
                    <TableHeaderColumn>Cidade/Estado</TableHeaderColumn>
                    <TableHeaderColumn>Status</TableHeaderColumn>
                    <TableHeaderColumn>Ações</TableHeaderColumn>
                  </TableRow>
                </TableHeader>

                <TableBody showRowHover>
                  {this.rows()}
                </TableBody>
              </Table>
            </Paper>
          </section>
        </div>
      </Loading>
    );
  }
}

EventosComponent.displayName = 'DashboardAplicativoEventosEventosComponent';

// Uncomment properties you need
// EventosComponent.propTypes = {};
// EventosComponent.defaultProps = {};

export default EventosComponent;

Actions:

import alt from 'components/Dispatcher';

import API from 'utils/API';
import User from 'actions/UserActions';
import Notification from 'actions/NotificationActions';

class EventoActions {

  getAll(page = 1) {
    API.Eventos
      .getAll({
        params: {
          page: page
        },
        headers: {
          'Authorization': `token ${User.getToken()}`
        }
      })
      .then((res) => {
        this.getAllDone(res);
      });

    return page;
  }

  getAllDone(res) {
    return res;
  }

  /* Create */
  create(evento) {
    API.Eventos
      .create({
        data: evento,
        headers: {
          'Authorization': `token ${User.getToken()}`
        }
      })
      .then(res => {
        this.createDone(res);
      })

    return evento;
  }

  createDone(res) {
    Notification.success({
      message: 'Criado com sucesso!'
    });

    return res;
  }

}

export default alt.createActions(EventoActions);

Store:

import alt from 'components/Dispatcher';
import update from 'react-addons-update';

import EventoActions from 'actions/EventoActions';

export class EventoStore {

  constructor() {
    this.state = {
      eventos: [],
      getting: true,
      pageActive: 1
    };

    this.bindActions(EventoActions);
  }

  onGetAll(page) {
    // doesn't work well...
    this.setState({
      pageActive: page,
      getting: true
    });

    /*
      So it works well...

      setTimeout(() => {
        this.setState({
          pageActive: page,
          getting: true
        });
      }, 1)

     */
  }

  onGetAllDone(res) {
    let state = this.state;

    let eventos = update(state.eventos, {
      [state.pageActive]: {
        $set: res.data.results
      }
    });

    this.setState({
      eventos: eventos,
      getting: false
    });
  }
}

export default alt.createStore(EventoStore, 'EventoStore');
lsanwick commented 8 years ago

@CezarLuiz0 try making this change.

Instead of:

componentDidMount() {
  EventoActions.getAll(1);
  EventoStore.listen(this.onEventoChange);
}

Move it to componentDidConnect:

static componentDidConnect() {
  EventoActions.getAll(1);
}

Also, having the component directly invoke against the store with onEventoChange is a bad pattern. Just add another action to EventoActions and a matching handler in the store, like this:

Component:

...
onEventoChange() {
  EventoActions.changeEvento(state)
}
...

Store:

...
onChangeEvento(state) {
  // Do something
}
...
cezarsmpio commented 8 years ago

@lsanwick I tried:

static componentDidConnect() {
    EventoActions.getAll();

    console.log('connected');
  }

But it did not work. -- EDIT Sorry, I had not used getStores and getPropsFromStores. componentDidConnect works fine!

lsanwick commented 8 years ago

Sorry, should have clarified, componentDidConnect receives two arguments, so if you need props to fire an action, you have them.

For example:

static componentDidConnect(props, context) {
  EventoActions.getEvent(props.id);
}