Add lolexFromGlobal export#164
Conversation
| @@ -2,800 +2,828 @@ | |||
| (function (global){ | |||
There was a problem hiding this comment.
should the diff from this file be excluded?
There was a problem hiding this comment.
Yes. It's a build artifact we check in when releasing new versions. We should, of course, try to get this built and added to the sinonjs page release page automatically somehow, but until then this serves as the only non-NPM distribution method.
There was a problem hiding this comment.
ok, cool. removed the changes then 🙂
|
|
||
| ### `lolex.lolexFromGlobal` | ||
|
|
||
| In order to support creating clocks based on fake environments (such as JSDOM), Lolex exports a factory method which takes single argument `global`, which it inspects to figure out what to mock and what features to support. When invoking this function with a global, you will get back an object with `timers`, `createClock` and `install` - same as the regular Lolex exports only based on the apssed in global instead if the global environment. |
There was a problem hiding this comment.
On the last line, "apssed" should presumably be "passed" and the "if" replaced with "of".
Could you perhaps refer to "separate environments" instead of "fake environments"? The latter description is not particularly accurate for jsdom.
There was a problem hiding this comment.
Good call! and thanks for the typo callouts 🙂
|
|
||
| Implements the `now` method of the [`Performance`](https://developer.mozilla.org/en-US/docs/Web/API/Performance/now) object but using the clock to provide the correct time. Only available in environments that support the Performance object (browsers mostly). | ||
|
|
||
| ### `lolex.lolexFromGlobal` |
There was a problem hiding this comment.
Any reason this has lolex in the name (rather than just fromGlobal or withGlobal?
There was a problem hiding this comment.
mostly as it kinda made sense in my head to do const lolex = lolexFromGlobal(myGlobal). But I'm totally fine with another name. Any preferences?
There was a problem hiding this comment.
I'd like withGlobal or fromGlobal. Looks better IMHO lolex.withGlobal(myJsDom)
If people do static imports and think the contextless names are confusing they can name it whatever they like anyway:
import {withGlobal as lolexFromGlobal} from 'lolex'.
benjamingr
left a comment
benjamingr
left a comment
There was a problem hiding this comment.
Changes look pretty straightforward and LGTM.
|
I've tested with this PR in Jest now (jestjs/jest#5171) and it works pretty good! |
|
@fatso83 while I realise I can merge stuff - I think this change is big enough to ask for your opinion :) |
fatso83
left a comment
fatso83
left a comment
There was a problem hiding this comment.
This looks fine to me! The factory name strikes me as a bit odd, though.
|
|
||
| Implements the `now` method of the [`Performance`](https://developer.mozilla.org/en-US/docs/Web/API/Performance/now) object but using the clock to provide the correct time. Only available in environments that support the Performance object (browsers mostly). | ||
|
|
||
| ### `lolex.lolexFromGlobal` |
There was a problem hiding this comment.
I'd like withGlobal or fromGlobal. Looks better IMHO lolex.withGlobal(myJsDom)
If people do static imports and think the contextless names are confusing they can name it whatever they like anyway:
import {withGlobal as lolexFromGlobal} from 'lolex'.
| }; | ||
| } | ||
|
|
||
| var defaultImplementation = factory(global || window); |
|
Renamed to |
|
Merged as da5505b (had to fix the commit msgs that didn't reflect the naming changes). |
|
Released as 2.4.0 |
|
Oh, forgot to change the commit message when squashing. sorry about that! |
4 participants
The first commit wraps the entire source inside of a factory function, and replaces global references with checks on the passed in
_global. It then invokes that factory withglobal || windowand exports the result from that call, to make the API stay the same for consumers. Highly recommended to view that diff without whitespace changes (https://github.com/sinonjs/lolex/pull/164/files?w=1).The second commit just exports that factory function, and adds docs.
The name is horrible, suggestions more than welcome.
Fixes #146