Missing Node.js / timers definition

[phillipj]

Hi,

I'm getting check errors when using .unref() on timers in Node.js. In practise I'm using .unref() to ensure the process can shutdown properly. Without it the process will not exit by itself cause the event loop isn't empty.

I'm more than happy to create a PR with the definition, but not sure what the best approach would be to avoid breaking the current definitions of clearInterval() and clearTimeout(). Making the Node.js setInterval() return a timer instance as described in the Node.js docs, would not work with the mentioned clear-methods expecting a number: https://github.com/facebook/flow/blob/master/lib/core.js#L766.

Any thoughts?

metrics.js

setInterval(reportProcessMetrics, 10 * 1000).unref();

output

metrics.js:58
 58:   setInterval(reportProcessMetrics, 10 * 1000).unref();
                                                    ^^^^^ property `unref`. Property not found in
 58:   setInterval(reportProcessMetrics, 10 * 1000).unref();
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Number

Refs Node.js Timers docs: https://nodejs.org/api/timers.html#timers_class_timeout

[mwalkerwells]

Why does setInterval return a number in the first place? Older versions of Node?

[phillipj]

Don't know why exactly, functions on window isn't specced by ECMA so it's more up to each environment, but returning a number seems to be the consensus amongst browsers. Node.js is a little different obviously.

Refs https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Return_value

[mwalkerwells]

This is how I'd update the library definition (pending clarification on Case 3). The return type depends on the environment that's used & it needs to be specified unless flow-typed allowed definitions to be environment dependent (@jeffmo).

/* @flow */

declare class Timeout {
  ref(): this;
  unref(): this;
}

declare type SetTimeout<T: Timeout | number> =
  <U: Array<*>>(callback?: (...rest: U) => void, ms?: number, ...args: U) => T

declare var setTimeout1: SetTimeout<*>
declare var setTimeout2: SetTimeout<Timeout>
declare var setTimeout3: SetTimeout<number>

// Case 1
declare var needsNumber: number => void
needsNumber(setTimeout1()) // No Error
needsNumber(setTimeout2()) // Errors
needsNumber(setTimeout3()) // No Error
    
// Case 2
declare var needsTimeout: Timeout => void
needsTimeout(setTimeout1()) // No Error
needsTimeout(setTimeout2()) // No Error
needsTimeout(setTimeout3()) // Errors

// Case 3
declare var needsBoolean: boolean => void
needsBoolean(setTimeout1()) // No Error: Not sure why this works? The return type of SetTimeout should be bounded to "Timeout | number".
needsBoolean(setTimeout2()) // Errors
needsBoolean(setTimeout3()) // Errors

[nmn]

Why not something like this?

This way you'd be forced to check for the existence of ref and unref though.

declare class Timeout extends Number {
  +ref?: () => this;
  +unref?: () => this;
}

declare function setTimeout(fn: Function, ms?: number): Timeout;
declare function clearTimeout(timeout: Timeout): void;

[julien-f]

Any news on this?

I'm new to Flow and I have no idea how to handle the error due to setTimeout().unref().
Is there a workaround?

[aikar]

I just throw // $FlowFixMe on them, but note you do need to do if (result.unref) even for node.js if you use Jest, as apparently Jest redefines these methods and returns numbers.

[SamChou19815]

The libdef has been moved to flow-typed. If you want to make improvements, please make PR over there.