-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0.63.x regression: setTimeout() & setInterval() return types broken #5627
Comments
@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); |
@TrySound Thanks for the info. I'm not sure why the team took this approach then. |
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 That makes a certain amount of sense -- but I still think this might cause a lot of head-scratching and |
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. |
I mean private type. |
Yeah, there should be a global type. |
There are global types for these: (Try Flow) const foo: TimeoutID = setTimeout(() => {}, 300);
const bar: IntervalID = setInterval(() => {}, 300); |
@jcready oh great, thanks! |
does TimeoutID have a default type? const foo: TimeoutID = ?; // number, string, boolean fails |
@lvdang if you need to initiate a constant, you can get |
@TrySound Ok I understand the reasoning of the introduction of class MyComponent extends React.Component<{}> {
timeout: number;
componentDidMount() {
this.timeout = setTimeout(doSomethingLater, 1000)
}
componentWillUnmount() {
clearTimeout(this.timeout)
}
} I’d expect to declar |
@MrLoh |
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... |
where I should define timerID`import React, { Component } from "react"; WebFont.load({ type Props = {}; class Clock extends Component<Props, State> { componentDidMount() { componentWillUnmount() { tick() { Hello, world!
|
@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);
}
}
... |
Error does not exist in
0.62.0
https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBjOA7AzgFzCjjgC4xsBXAWwCMBTAJzAF4xd78AVAS2vriV8ACmEBKVgD4wAbwC+AGjABmAAyqxAbkw4CYWgENG5KnSat2nAJLZ8TAG4GYoiS2nylajZqA
The text was updated successfully, but these errors were encountered: