TypeError unref of undefined when running proper-lockfile inside of react-scripts test about node-proper-lockfile HOT 7 CLOSED

Ah just read a little more and I see where it is required to be an integer.

"If previous handle was provided, let handle be previous handle; otherwise, let handle be a user-agent-defined integer that is greater than zero that will identify the timeout to be set by this call in the list of active timers."

I'm happy to go make this argument with Jest

from node-proper-lockfile.

Looking at the code, the error you reported shouldn’t be happening as setTimeout should always return something. In case of the browser, it returns a integer, while in node it returns a time ref. It seems to me that your test environment is somehow mocking setTimeout in a bad way and returning nullish, causing the issue.

Are you using any lib to mock timers?

from node-proper-lockfile.

Thanks for getting back @satazor
Looks like jest is doing something funky with setTimeout.

I've attemped using jest.useFakeTimers() but it looks like the mock timers don't solve the issue.

I was able to confirm that creating a manual mock of setTimeout which returns an empty object fixes the unref of undefined issue.

It'll take further digging to figure out exactly what jest is doing that is causing issues with setTimeout

from node-proper-lockfile.

I'm doing the same, and seeing the same error.

The PR that @mboperator submitted (#91) mitigates the issue in my case.

Given the fact that it's necessary to be defensive here for cases where setTimeout returns something specifically different (an integer), it seems reasonable to me to be generally defensive against cases where it might return something else (like in this case undefined).

The current implementation is defensive against everything that happens to be truthy but doesn't implement unref. Why not go all the way and defend against nullish values as well?

from node-proper-lockfile.

I understand that the overall issue caused a lot of frustration. If the check was in place, you wouldn’t experience this issue at all, which at first might be a good thing. But if you think about it, it would be hiding a flaw from somewhere else that would eventually bite you in the future.

The spec says that setTimeout must return a handle that can be passed to clearTimeout, see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timers

Since this is a bug with the environment you are running tests and not this lib, it seems unreasonable to implement a fix/check here.

from node-proper-lockfile.

Forgive me, I'm pretty new to reading specs like this, but it looks to me like undefined can be passed to clearTimeout.

If it really is against the spec to return undefined from setTimeout I'm 100% with you, however I don't see in the spec you linked that this is quite the case.

"The clearTimeout() and clearInterval() methods must clear the entry identified as handle from the list of active timers of the WindowOrWorkerGlobalScope object on which the method was invoked, if any, where handle is the argument passed to the method. (If handle does not identify an entry in the list of active timers of the WindowOrWorkerGlobalScope object on which the method was invoked, the method does nothing.)"
-https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timers

clearTimeout seems to be designed to accept arguments that don't identify handles and that this would include nullish values like undefined which to me seems reasonable for situations like unit testing.

from node-proper-lockfile.

This is an old issue i'm going to close this! Feel free to reopen if it needs more discussion.

Thank you.

from node-proper-lockfile.