I still question legitimacy of that ticket, we didn't receive any real issues about that and it seems only one reason why we need to do that is

However, some environments provide their own window.setTimeout etc.

Those environments should create browser-like conditions, not that other way around. If you think about it, it seems really weird, for us to do this, it is like for Firefox to change some their behaviour to keep consistency with the jsdom, which, actually should create Firefox like environment. Sounds like absurd, i bet no browser lib would do that.

In other words, jsdom should create hacks and not us, and again to emphasize - we didn't received any issues which should provoke this PR.

Also, @domenic could you please check it out, since it seems, we doing that just for you.

Otherwise LGTM :-).

There's no reason to do this. Scripts that run in jsdom can use setTimeout and friends just fine, with no prefix.

If you run into any other "do this to support jsdom" issues, let me know and we can fix jsdom.

@markelog

However, some environments provide their own window.setTimeout etc.

Those environments should create browser-like conditions, not that other way around.

The thing is those environments do provide those conditions. The current situation is that we support both scenarios:

1. The global behaves like a browser window (web browsers, browserify): we return the jQuery object.
2. The global doesn't behave like a browser window: we then return a function to which you can provide your own window instance and it'll return the jQuery object using that window.

In the (2) case we're cheating a little as we generally take stuff off this window but with exception of setTimeout & friends which we always take from the global.

We could decide that this is an edge case that's not worth supporting but it will mean a little divergence from the (2) use case.

@domenic This mostly isn't about jsdom but about running in an environment where you can provide most needed stuff via your own window instance but without assuming you have anything on a global. Though if you were creating & closing (window.close()) a lot of windows in jsdom and using setTimeout etc. in them you'd leak memory, wouldn't you? jsdom has its own setTimeout etc. for a reason, doesn't it?

we didn't received any issues which should provoke this PR

We did, the exact one I mentioned in the description of this PR... i.e. #2177.

Oh, I don't really know anything about the use case of pasing in a window to a factory function. jsdom is geared around the use case of running jquery inside the jsdom environment itself, not that thing.

@mzgol well, in there, owner of that ticket tries to use Deferreds (in the environment that doesn't have setTimeout!) which is better to do through https://github.com/jaubourg/jquery-deferred-for-node package or through custom build, so that is really out there which doesn't worth 40 bytes and code complication from my perspective.

So i as i see it, since jsdom could work with jQuery just fine, the reason to do it is to be more consistent but a lot more hacky?

Thank you for the replay @domenic, you helped a lot.

@domenic Hm. It was all added in 1f67d07. You even posted some comments under the commit. ;)

Well, if we just want to assume that window is available then the whole factory function with all this complicated logic when to return the object and when the function accepting window as a parameter is unnecessary... So how did we end up here? :P @timmywil?

Also, if we want to go full-in in requiring a proper window to be present
then I'd prefer to remove the JSHint exception for setTimeout etc. and just
use window.setTimeout without a fallback to the global one like we do with
most browser globals.

Actually, I'd be fine with requiring a proper window and moving to window.setTimeout everywhere. The problems that would cause in Node would be mitigated by using tools like jsdom along with jQuery, which is probably the only way jQuery should be used in Node anyway.

Side note: we probably shouldn't recommend https://github.com/jaubourg/jquery-deferred-for-node after 3.0 comes out. It hasn't received updates in a long time and does not support Promises/A+. I know that was the point of packaging jquery deferreds separately, but it would no longer be consistent with jQuery 3.0 Deferreds.

@timmywil @markelog OK, I created yet another attempt: #2397.

Side note: we probably shouldn't recommend https://github.com/jaubourg/jquery-deferred-for-node after

Let's ask @jaubourg, i thought he is still commited to supporting this module

I'd be fine with requiring a proper window and moving to window.setTimeout everywhere

Why we need a window "prefix" for global vars?

Why we need a window "prefix" for global vars?

For the same reason we prefix getComputedStyle, document, document.documentElement etc. - so that we're consistent in taking them from the provided window instance instead of the global. If we e.g. assumed a global getComputedStyle, jsdom wouldn't work. It would work without scoping setTimeout but there are other use cases. I think we should be consistent.

It took me a while to catch up. In this particular case on #2177 it seems like the methods would normally be on global but are only on window for this unusual Firefox env, so we reference them from there?

@dmethvin This comment of mine is a good summary: #2394 (comment)

We take most globals from window but we take setTimeout etc. from the global which works for jsdom but not necessarily for other envs and is inconsistent. This PR tries to support both but the simpler one (the 4th my PR trying to fix this issue ;)) that just takes them from window is at #2397.

Now i remember, okay, i agree, but i still would just write window.setTimeout

Closing in favor of #2397.