Skip to content
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

Change in return value of setTimeout #35770

Closed
kefasb opened this issue Oct 23, 2020 · 4 comments
Closed

Change in return value of setTimeout #35770

kefasb opened this issue Oct 23, 2020 · 4 comments
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Comments

@kefasb
Copy link

kefasb commented Oct 23, 2020

  • 12.19.0:
  • all:

What steps will reproduce the bug?

In node:12.19.0-slim docker image setTimeout function returns a value for which isNaN=false
In node:12.18.4-slim docker image setTimeout function returns a value for which isNaN=true

timeoutId = setTimeout(function() {alert("Timeout")}, 1000);
console.log(isNaN(timeoutId))

image

Is this an intentional behaviour?

@PoojaDurgad PoojaDurgad added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Oct 23, 2020
@Trott
Copy link
Member

Trott commented Oct 23, 2020

Is this an intentional behaviour?

It was indeed an intentional change. #34017

I wonder if perhaps that would have ideally been treated as semver-major, but I can see why that wasn't obvious at the time.

@lundibundi @nodejs/timers

@Fishrock123
Copy link
Contributor

I don't have much to add - the change is definitely something that should have happened at some point. The current maintainers will need figure out what, if anything, to do about this situation.

@kefasb
Copy link
Author

kefasb commented Oct 26, 2020

Thank you for clarification.

@kefasb kefasb closed this as completed Oct 26, 2020
@benjamingr
Copy link
Member

@kefasb just to point out - this change should have probably been communicated better and semver-major if people "in the wild" are running into cases where code that checks for node vs. browser timers is starting to fail.

The idea was to make node timers easier to work with and this sort of usage pattern (someone using isNaN to check if the timer they got was a web browser) was missed on our part.

Sorry for breaking your code!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

No branches or pull requests

5 participants