fixed delay updating parameters;

[MicheleBertoli]

Hello @zetta83, thanks for your PR. Can you please provide more context on this?

[zetta83]

for the setState callback you should actually use this.props since it’s called after the setState is performed, the props from params may be old

[MicheleBertoli]

@zetta83 was this causing any real problem? I don't see any open issue around this. Is it possibile for you to provide a failing test or a minimal repro example? Thanks!

[zetta83]

The start function is being called on componentWillReceiveProps, which means this.props relates to the previous component props, not the next props. If you set the from prop to 5 and then set it to 10, the start value of the counter will be set to 5, not 10. That means that the counter will always go back a little if we update the from prop. Please check this pen showing the bug: https://codesandbox.io/s/PNjwWL5n2. This pen will run the counter from 0 to 10, then from 10 to 50, and then from 50 to 100.

[MicheleBertoli]

I see, thanks! Shouldn't be enough to change the lifecycle method from componentWillUpdate to componentDidUpdate?

[zetta83]

that would work too, but it will make the component re-render every time it updates :slightly_smiling_face: calling setState on componentWillReceiveProps won’t trigger another render

[MicheleBertoli]

Cool @zetta83, can you please replace the current "when receive new props" test block with this before I merge?

describe('when receive new props', () => {
  class Parent extends Component {
    constructor() {
      super();
      this.state = {
        from: 0,
        to: 1,
      };
    }
    render() {
      return <CountTo speed={1} {...this.state} />;
    }
  }

  it('starts from 0, restarts from 0', () => {
    const parent = TestUtils.renderIntoDocument(
      <Parent />
    );
    const span = TestUtils.findRenderedDOMComponentWithTag(parent, 'span');
    expect(findDOMNode(span).textContent).toEqual('0');
    jest.runAllTimers();
    expect(findDOMNode(span).textContent).toEqual('1');
    parent.setState({
      to: 2,
    });
    expect(findDOMNode(span).textContent).toEqual('0');
    jest.runAllTimers();
    expect(findDOMNode(span).textContent).toEqual('2');
  });

  it('starts from 0, restarts from 2', () => {
    const parent = TestUtils.renderIntoDocument(
      <Parent />
    );
    const span = TestUtils.findRenderedDOMComponentWithTag(parent, 'span');
    expect(findDOMNode(span).textContent).toEqual('0');
    jest.runAllTimers();
    expect(findDOMNode(span).textContent).toEqual('1');
    parent.setState({
      from: 2,
      to: 3,
    });
    expect(findDOMNode(span).textContent).toEqual('2');
    jest.runAllTimers();
    expect(findDOMNode(span).textContent).toEqual('3');
  });
});

The second fails on master and passes on your branch. It's important to cover the issues with tests when they are fixed to make sure they don't happen again.