Flow: 0.63.x regression: setTimeout() & setInterval() return types broken

[jaredh159]

Error does not exist in 0.62.0

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBjOA7AzgFzCjjgC4xsBXAWwCMBTAJzAF4xd78AVAS2vriV8ACmEBKVgD4wAbwC+AGjABmAAyqxAbkw4CYWgENG5KnSat2nAJLZ8TAG4GYoiS2nylajZqA

// @flow
const foo: number = setTimeout(() => {}, 300);
const bar: number = setInterval(() => {}, 300);

3: const foo: number = setTimeout(() => {}, 300);
                       ^ TimeoutID. This type is incompatible with
3: const foo: number = setTimeout(() => {}, 300);
              ^ number
4: const bar: number = setInterval(() => {}, 300);
                       ^ IntervalID. This type is incompatible with
4: const bar: number = setInterval(() => {}, 300);
              ^ number

[TrySound]

@jaredh159 Not a regression. Just not documented. As you can see here number was replaced with opaque type to prevent passing incorrect values to clearTimeout/clearInterval. Try to infer opaque types.

const foo: * = setTimeout(() => {}, 300);
const bar: * = setInterval(() => {}, 300);

[jaredh159]

@TrySound Thanks for the info. I'm not sure why the team took this approach then. setTimeout and setInterval return numbers always. So it's fair that lots of devs might add types or timeout ids and interval ids. This change basically enforces that you can't ever type variables that hold these variables and that you have to infer. It's weird to me. It's like you changed the core api of a global javascript function. They return numbers, but you aren't allowed to type them as numbers now. I could see a lot of people getting tripped up by this even if it was documented as a gotcha somewhere.

[jaredh159]

I looked at the commit that made the change and I think I understand better. You're trying to enforce the fact that you should only pass to clearInterval something that came from setInterval and so on.

That makes a certain amount of sense -- but I still think this might cause a lot of head-scratching and // $FlowFixMe because it seems broken not to be able to type the return of one of these functions as what they are -- a number. Maybe a specific error message? Is that even possible?

[TrySound]

It's private value which works only with pair. Passing wrong value may have same effect as passing wrong type. So semantically it's a good direction if you upgrade major version.

[TrySound]

I mean private type.

[TrySound]

Yeah, there should be a global type.

[jcready]

There are global types for these: (Try Flow)

const foo: TimeoutID = setTimeout(() => {}, 300);
const bar: IntervalID = setInterval(() => {}, 300);

[jaredh159]

@jcready oh great, thanks!

[lvdang]

does TimeoutID have a default type?

const foo: TimeoutID = ?; // number, string, boolean fails

[apsavin]

@lvdang if you need to initiate a constant, you can get TimeoutID only from setTimeout function.

[MrLoh]

@TrySound Ok I understand the reasoning of the introduction of TimeoutID type, but I don't understand what I should declare in my react components now. I usually do

class MyComponent extends React.Component<{}> {
  timeout: number;
  componentDidMount() {
    this.timeout = setTimeout(doSomethingLater, 1000)
  }
  componentWillUnmount() {
    clearTimeout(this.timeout)
  }
}

I’d expect to declar timeout now as type TimeoutID instead of number, but do I understand correctly, that I should declare it as any?

[TrySound]

@MrLoh any is unsound. You should use TimeoutID.

[lll000111]

@jaredh159, @MrLoh

setTimeout and setInterval return numbers always.

No they don't. On node.js they return an object: https://nodejs.org/dist/latest-v10.x/docs/api/timers.html#timers_class_timeout and Flow also has to work for the node.js latform...

[msh2050]

where I should define timerID

`import React, { Component } from "react";
import ReactDOM from "react-dom";
import WebFont from "webfontloader";

WebFont.load({
google: {
families: ["Titillium Web:300,400,700", "sans-serif"]
}
});

type Props = {};
type State = {
date: Date
};

class Clock extends Component {
constructor(props) {
super(props);
this.state = { date: new Date() };
}

componentDidMount() {
this.timerID = setInterval(() => this.tick(), 1000);
}

componentWillUnmount() {
clearInterval(this.timerID);
}

tick() {
this.setState({
date: new Date()
});
}
render() {
return (

Hello, world!

It is {this.state.date.toLocaleTimeString()}.

);
}
}

const rootelement = document.getElementById("root");
if (rootelement == null) {
throw new Error("no pad element");
}`

[kdrich]

I don't understand what I should declare in my react components now.

@MrLoh, depends on your setup. I'm doing this. Same idea, I just wait to initialize it. What did you end up doing?

export default class Header extends React.Component<{}, State> {
  state = { query: '' };
  timerId: TimeoutID;

  doSearch = (): void => {
    if (typeof this.timerId === 'number') {
      clearInterval(this.timerId);
    }

    this.timerId = setTimeout(() => {
      const { query } = this.state;

      if (query.trim().length > 0) {
        console.log(`Fake searching for "${query}" with timerId ${String(this.timerId)}`);
      }

    }, 300);
  }

  // https://flow.org/en/docs/react/events/
  setQuery = (e: SyntheticEvent<HTMLInputElement>): void => {
    if (typeof e.currentTarget.value === 'string') {
      this.setState({query: e.currentTarget.value }, this.doSearch);
    }
  }

...