Create fake timers based on passed in object

[SimenB]

Bad title, so I'll try to describe my use case.

If I create a JSDOM instance in node, it has requestAnimationFrame, but lolex is unable to discover this (unless I use some fancy vm code) as is inspects global at require-time. This means that I have to use lolex inside of the script I execute in JSDOM, instead of managing it from the outside by controlling the injected globals. I need this as I'm looking to use Lolex directly in Jest.

I'm thinking something like const clockFactory = lolex.createClockFactoryFrom(global) or something, which can be separately used later in for instance const clock = clockFactory.install({ target: myTarget });

[jdalton]

For context, @SimenB is a core dev for the Jest testing solution. Jest is looking to adopt lolex for their mocks to simplify their support and defer to a specialized lib like sinonjs/lolex.

[benjamingr]

Sure @jdalton @SimenB I think we can do that.

The way I see it - we'd either pass the object to .install or expose a new method that does it to global.

Alternatively, we can expose a factory on lolex:

• Wrap the entire code in lolex-src.js in a factory function
• Invoke that when we export lolex with global from the outside (from browserify)
• Expose the factory a second time as lolex.initializeFromGlobal.

That should let you pass global in. As an alternatively, jest could create a new clock and then override the globals itself - but that doesn't sound ergonomic and I think it's fine for us to expose it (especially since the JSDOM in Node use case is entirely legitimate and should be supported IMO)

@fatso83 if this sounds good to you - I can open a PR for this if you'd like - shouldn't be too much work to add.

[SimenB]

As an alternatively, jest could create a new clock and then override the globals itself

Yeah, that's the alternative, but I was hoping to offload all of that logic onto Lolex.

One issue with manually setting the globals is that set{Timeout,Interval,Immediate} returns Timer instances with an unref function in Node which we don't want with JSDOM. I assume from the docs that Lolex does the right thing:tm: based on globals, which breaks in the JSDOM case.

^{Just to clarify, I'm a collaborator on Jest, but I don't work for FB 🙂}

[benjamingr]

@SimenB

One issue with manually setting the globals is that set{Timeout,Interval,Immediate} returns Timer instances with an unref function in Node which we don't want with JSDOM

You're right:

var timeoutResult = setTimeout(NOOP, 0);
var addTimerReturnsObject = typeof timeoutResult === "object";

If we extract things to a factory this should work out of the box - although it should probably be changed to global.setTimeout and we should add a test for it.

[fatso83]

I thought this was the intent of using var clock = lolex.install({target: context});, but I see that specifying the target doesn't change use of global. Changing that would probably break a lot of assumptions, and hence tests, so I think a PR for a factory function makes sense. I guess it would basically have the same signature as install, just with an added option of global, or perhaps an arity of two, like (global, installOptions)?

[SimenB]

What about const clock = lolex.createClock({base: myGlobal}) and being able to say clock.installInto(myGlobal) or something? Would still need to move around all the current global usage, but reusing createClock seems nice (at least from an outsider's perspective 🙂)

[fatso83]

Seems ok, but the API looks a tad bit weird/inconsistent when considering that we return a clock object from install. So

var clock = lolex.install(); // installs into global
clock.installInto(myGlobal); // possible, but not meaningful

[benjamingr]

var clock = lolex.install({global: window});

can also be done (API wise) if you prefer

[fatso83]

I think that is preferable, if possible? Seems consistent. Other views?

[SimenB]

I like it.

That means we do

var clock = lolex.install({global: this._global, target: this._global});

right?

One thing is that in Jest we want the ability to install and uninstall often, and it's wasteful to calculate from globals every time. But I doubt it's measurable

[SimenB]

Any news here? Would love this to land so I can start testing this within Jest. I've started a tiny bit (jestjs/jest#5171) and the amount of code deletion (and extra features) is absolutely lovely!

[benjamingr]

@SimenB I'm currently traveling in Ecuador, I'll only be back in Feb 6th and will be able to work on this afterwards - you or @fatso83 can take this or we can wait - sorry!

[SimenB]

No need to apologize, a timeline is more than enough for me. Enjoy your travels! 🙂

[benjamingr]

Going to take a look

[benjamingr]

Looking at this, it seems like it's just a bug with target - so rather than introducing a new parameter we should make timer references deference the target every time.

[SimenB]

I don't think so. See the following lines: https://github.com/sinonjs/lolex/blob/444c9384ace3853761d76aa31fa4ac3648247c9d/lolex.js#L31-L37

They look at the global global. Jest needs it to look at some passed in global as we won't require lolex from within the context of jsdom - we manage it outside of it

[benjamingr]

@SimenB thinking about this further, I think the issue is just whether or not we should return object or DOM timers - so I think I'll just PR that functionality instead.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[SimenB]

Discussion still ongoing in the PR

[SimenB]

For people following along, I've opened up a PR containing my suggestion from the OP - a factory function. #164