chore(tests): don't rely on jest fake timers scheduling real timers

[SimenB]

I'm currently working on replacing Jest's custom fake timers implementation with Lolex (jestjs/jest#5171). As part of that, I wanted to test that React's tests still work. They didn't...

A quirk of Jest's current implementation is that we schedule process.nextTick and setImmediate on the real timers, even though they're faked. Then we just make sure to do nothing if Jest invoked the function when advancing time, or vice versa if Node triggered it before us. That behavior is intentionally not re-implemented in the new version.

This PR changes from using setImmediate to Promise.resolve().then() to "wait 1 tick" which behaves the same.

[SimenB]

Using Jest 24, the test passes without faking global.requestAnimationFrame. So whenever the upgrade lands, just remove the implementation here

[SimenB]

Lolex will fake this.

The test also passed when removing it when using Jest 23, not sure what it was supposed to do?

[nhunzaker]

This is curious! I'm not sure what this was for. I'm guessing that this was just an artifact copy/pasting a snippet from ReactDOMRoot-test. It's quite a bit more sophisticated:

react/packages/react-dom/src/__tests__/ReactDOMRoot-test.js

Lines 23 to 45 in c84b9bf

	container = document.createElement('div');
	// TODO pull this into helper method, reduce repetition.
	// mock the browser APIs which are used in schedule:
	// - requestAnimationFrame should pass the DOMHighResTimeStamp argument
	// - calling 'window.postMessage' should actually fire postmessage handlers
	// - must allow artificially changing time returned by Date.now
	// Performance.now is not supported in the test environment
	const originalDateNow = Date.now;
	let advancedTime = null;
	global.Date.now = function() {
	if (advancedTime) {
	return originalDateNow() + advancedTime;
	}
	return originalDateNow();
	};
	advanceCurrentTime = function(amount) {
	advancedTime = amount;
	};
	global.requestAnimationFrame = function(cb) {
	return setTimeout(() => {
	cb(Date.now());
	});
	};

I think this is safe to remove.

[sizebot]

Details of bundled changes.

Comparing: ae4f3f0...5029cba

scheduler

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
scheduler.development.js	n/a	n/a	0 B	19.17 KB	0 B	5.74 KB	UMD_DEV
scheduler.production.min.js	n/a	n/a	0 B	3.16 KB	0 B	1.53 KB	UMD_PROD

Generated by 🚫 dangerJS

[nhunzaker]

I kind of wish this could be re-used, but it's unclear where something like that could go. .

[SimenB]

Could do an await, then you're guaranteed a tick even if promises are faked

[nhunzaker]

Naw, I think this is fine.

[nhunzaker]

This looks good to me. What does the timing of this look like? Should we keep this open until the change in Jest publishes?

[SimenB]

Seeing as the tests still pass, there's no reason not to merge, imo :)
Timeline is fuzzy, personally I hope for e.g. December 1st

[nhunzaker]

Sure. Let's run with it. Thanks!

[acdlite]

@nhunzaker In the future, please at least tag the person who wrote the code that's being changed. Brian or me, in this case. Unless it's to unbreak master. Sometimes even small changes like this could have unforeseen consequences.

[nhunzaker]

Sorry 😥. Sure thing!

[SimenB]

Did it break anything, or is that just a precaution?