Allow configuring if lolex should return DOM or node timers

[benjamingr]

Fixes (possibly) #146

CC @SimenB - let me know if this addresses your environment configuration issue with global or if a more general workaround (passing the library) is still required - this just felt less flakey to me.

CC @fatso83

[SimenB]

This won't be enough for Jest's use case as for instance requestAnimationFrame is not on global. It will be on target, though. It will also incorrectly detect process.nextTick even though it won't be available in the runtime of tests (or vice versa if testEnvironment: node rather than the default jsdom)

[benjamingr]

Interesting, I'm still leaning towards exposing the detection outside as configuration rather than accepting a global.

I figured jest wouldn't be interested in things like whether or not requestAnimationFrame is shimmed by default since it picks which timers to shim itself. Would exposing the rest of the flags help?

[SimenB]

Jest cares that it is shimmed at all, which it won't be without having lolex do it's detections either on target passed to lolex.install or through some other way of inspecting an object I give it.

if lolex.install({ target }) did all of its sniffing for existent timer functions on target it would work. Now performancePresent will be false meaning performance.now will be gone, whilst hrtimePresent will be true, meaning it's exposed even if it shouldn't be. We'll also get requestAnimationFrame into the node env, which we don't want.

I suppose we could send the whitelist ourselves, but being able to say to Lolex "fake the timer functions on this passed in object" is a much nicer API

[benjamingr]

@SimenB what about passing Object.keys(window) for the JSDOM window or Object.keys(global) in Node as toFake to lolex.install?

If you want hrtime you can also pass Object.keys(global).concat(Object.keys(process)

[SimenB]

That might work, yeah. Same with performance I guess?

I can apply these changes locally and see if it's enough for our use case 🙂

[benjamingr]

@SimenB right, but not with whether or not timers should or shouldn't return objects - which requires this PR. Would you be OK with that?

I'm still willing to allow passing target more "aggressively" but I'm not really sure that's particularly good API behavior and this lets us do things explicitly.

[SimenB]

Doesn't work to pass any old string to toFake because of this line: https://github.com/sinonjs/lolex/blob/444c9384ace3853761d76aa31fa4ac3648247c9d/src/lolex-src.js#L409

    TypeError: Cannot set property 'hadOwnProperty' of undefined

      at hijackMethod (node_modules/lolex/src/lolex-src.js:409:34)

It's also impossible to fake performance.now as lolex thinks it doesn't exist. Unless I'm doing something weird, it seems impossible to make lolex detect the correct globals when it's self-detecting at require-time instead of inspecting some passed in object.

[fatso83]

Any thoughts on how to proceed with the issues @SimenB mentions? Is that a separate issue?

[SimenB]

Any news here? We're gearing up for Jest 23, and switching will probably be pretty breaking, so I'd love to be able to include Lolex in that release 🙏

I'm happy to put together a PR if we can figure out the correct API. The issue boils down to the fact that lolex inspects globals when it's required, but Jest has a separate global object which is used in tests, and that's the thing that needs to be inspected.

[benjamingr]

I'm happy to put together a PR if we can figure out the correct API. The issue boils down to the fact that lolex inspects globals when it's required, but Jest has a separate global object which is used in tests, and that's the thing that needs to be inspected.

To be honest I'm not really 100% sure what a solution would be - so a PR would be great :)

[SimenB]

@benjamingr cool, opened up #164

[benjamingr]

Thanks, let's continue the discussion there, I'm +1 on landing the changes in time for the next jest major