Bring config.target into the light

[fatso83]

Purpose (TL;DR) - mandatory

Remove unneeded branch logic and improve docs.

Background (Problem in detail) - optional

When looking into a user question (#2155) it became obvious that the
it's not obvious at all that the docs for Lolex install config can be
used when trying to configure useFakeTimers.

For one, we added a prop, config.global, to Sinon's useFakeTimers config in #1935 that
was quite superfluous: it supported a use case that was already there using config.target!

In this PR, I remove some extra branching logic and also update the docs
to highlight the lolex docs and the target prop.

How to verify - mandatory

1. npm test
2. npm run build-docs

[fatso83]

Basically fails due to sinonjs/fake-timers#276. One option I highlight there is to basically remove the target prop and just have one way of configuring Lolex for a different context. If so, I'd remove (or at least redo) these changes.

[mroderick]

This looks good 👍

Needs a bit of love to get the tests passing.

Once this is in, I think we should put out the next semver:major with a good changelog update.

[fatso83]

@mroderick This kind of stalled because I don't know how to act around @SimenB's comment in Lolex, as this turned out to have a little catch: sinonjs/fake-timers#276 (comment): Is there a reason one might want to use the timers from a second environment and "pass them" into a third? That's basically the use case the current behavior supports that simplifying this in Lolex would remove.

[SimenB]

FWIW, I don't think it's necessarily a valid use case. Maybe you want a working queueMicrotask into some other env that doesn't support it, but that's essentially polyfilling and not Lolex's responsibility at all

[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.

[mroderick]

@fatso83 @SimenB what do you think we should do with this PR?

[SimenB]

I think that for now this can land as is, and if anything changes in lolex we can revisit. But the current implementation matches what's documented in this PR

[fatso83]

Sorry for keeping this hanging. Will redo once sinonjs/fake-timers#318 is merged.

[fatso83]

How would we like the API to look here? Keep the config.target (which is now proposed to be removed in the fake timers project) or expose withGlobal somehow? 😕

Might be worth considering that nise could also use such a thing.

[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.

[mroderick]

@fatso83 I think there's nothing left blocking this ... is there?

[fatso83]

As sinonjs/fake-timers#318 removed config.target (my doing), this PR no longer makes sense. What makes sense is document the global property introduced in #1935 and highlight that in our docs (missing) to avoid stuff like #2155 (comment).