prefer-global-this should allow window.setTimeout() and setInterval()

[n0099]

Description

as it's a common workaround: 1, 2, for globalThis.setTimeout() and setInterval() returning NodeJS.Timeout in @types/node

Fail

const id: number = window.setTimeout(console.log, 100);
//                 ^^^^^^
// Prefer `globalThis` over `window`. eslint(unicorn/prefer-global-this)

Pass

const id: number = window.setTimeout(console.log, 100);

const id: NodeJS.Timeout | number = globalThis.setTimeout(console.log, 100);

Additional Info

No response

[sindresorhus]

I disagree. The correct way to handle it is:

const id: ReturnType<typeof setTimeout> = window.setTimeout(console.log, 100);

[fregante]

I had this exact issue yesterday 😅

I wish there was a way to instruct the user on how to proceed in cases like this. Should this specific scenario be handled differently? For example:

• disable auto-fix in TS files
• extend the message to specify how to specify the type
• mention it in the rule's readme

I also had issues regarding globalThis types after running this rule. "Nobody knows" how to make globals available as properties on globalThis (the solution seems to be declare var someGlobal: Type, see xojs/eslint-config-xo-typescript#91)

[n0099]

In my scenario I'm storing the id returned by setTimeout() for latter cancelation via clearTimeout().
If I have to make it compatible with both node and browser enviornment, the initial value of id before its first assign from setTimeout() should be conditionally set to:

let id = import.meta.client ? 0 : import.meta.server ? new NodeJS.Timeout() : ...;

for preventing let TDZ and a violation of eslint rule init-declarations.
Or with the fact that clearTimeout() accepts undefined on both node and browser:

let id: ReturnType<typeof setTimeout> | undefined = undefined;

but this will also violates the eslint rule no-undef-init and unicorn/no-useless-undefined.

[fregante]

You don't need any of that. Just call setTimeout normally.

As for the let, the lint rule is right. This is correct:

let id: ReturnType<typeof setTimeout> | undefined;

// and then

id = setTimeout()
clearTimeout(id)

[n0099]

This is correct:

that requires // eslint-disable-next-line init-declarations for let id;

[fregante]

That rules isn't enabled in any recommended config: https://eslint.org/docs/latest/rules/init-declarations

As you said, it's completely at odds with unicorn/no-useless-undefined so pick one or the other.

[xsjcTony]

I disagree. The correct way to handle it is:

const id: ReturnType<typeof setTimeout> = window.setTimeout(console.log, 100);

It won't work. It will always default to NodeJS's version unless you explicitly specify window.setXxx

[fregante]

That was a typo, drop window as the rule suggests

[n0099]

typeof setTimeout is referring typeof globalThis.setTimeout, so it's not the same with your window.setTimeout

[xsjcTony]

I've tried all the different combinations, it will always default to Node.js's version🫠
eslint-disable for now I guess

[fregante]

typeof setTimeout is referring typeof globalThis.setTimeout, so it's not the same with your window.setTimeout

It doesn't matter as long as you don't use any specific features from node's.

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAcgHYQCYFMEG4BQODGESAzvMKgFxwBK6MArlEgCoCeY6APDO+hAGZxidZqD70YAPjgBeISLEQJACkIkIAG3QA6DRADmAGjgBGAAxmAlLji24AentwAegH4gA

In short, use the setTimeout function as you've always done: without window

[xsjcTony]

Yes it will work if the only usage is for clearInterval. However my function here requires a number type.

Guess I need to change number to ReturnType<typeof setTimeout>, but that's just so weird🫠

[n0099]

[...]it will always default to Node.js's version🫠[...]
[...]

window.setTimeout is the one for browser enviorment.

Guess I need to change number to ReturnType<typeof setTimeout>

Only when that function has to work on both node and browser env.

It doesn't matter as long as you don't use any specific features from node's.

[xsjcTony]

Honestly, I agree with adding this special case to the rule, as it's so classical and common that both versions will be used in a project.
The most straightforward way to differentiate is to use window.setTimeout for the browser and globalThis.setTimeout for the node.

[fregante]

window.setTimeout is the one for browser enviorment.

That's completely irrelevant. This is a type-only issue, window === globalThis in real life.

I'm closing this for now because the discussion is going nowhere. This is a documentation issue/misunderstanding. I'll open a dedicated issue to improve the DX as described earlier

[xsjcTony]

window.setTimeout is the one for browser enviorment.

That's completely irrelevant. This is a type-only issue, window === globalThis in real life.

I'm closing this for now because the discussion is going nowhere. This is a documentation issue/misunderstanding. I'll open a dedicated issue to improve the DX as described earlier

Indeed, I ran in to no-var as well today.😂 Yeah let's do a separate issue for those things, I'll just eslint-disable for now.

@n0099 Yes I do understand Web API and Node.js API and that's exactly why I'm here. I still think using window.setTimeout is the most clear way, but anyways.

[n0099]

That's completely irrelevant. This is a type-only issue, window === globalThis in real life.

I know that both window.setTimeout and globalThis.setTimeout will return NodeJS.Timeout on node env(or number on browser env).
But only the typing for globalThis.setTimeout is defined in @types/node, that's why sticking with window.setTimeout will use the one from lib.dom.d.ts which comes with typescript itself and cannot get removed like @types/node (yes, npm uninstall @types/node is also a workaround to prevent a literal setTimeout(without window. or globalThis.) returning NodeJS.Timeout).
This also means if you assume the returned value of window.setTimeout will always be an number as lib.dom.d.ts says and doing +-*/ on it, it will throw TypeError on node runtime but not the browser, and typescript won't help you to detect this.