Use globally defined setImmediate on server side for test suites like jest that have mock timers

[robframpton]

Jest leverages Node's setImmediate function with it's mock timers. After updating metal-clay-components to metal 2.16.0, there were a few test failures due to this.

My proposal is that we use the globally defined setImmediate only when it's defined on the server.

The reason for not using Metal's isServerSide method here, is that it also checks if the NODE_ENV variable is equal to "test", which is the case in metal-clay-components and other projects which use jest.

[robframpton]

Tests are not running due to TravisCI outage https://www.traviscistatus.com/incidents/c1s4dlyxd4lf

[jbalsas]

Re-ran the tests and all seems good...

Why not updating isServerSide to either remove the NODE_ENV check or provide some variant? That way we don't need to repeat this logic here and remember what it means...

[robframpton]

Hey @jbalsas, makes sense.

Before I bother writing tests and pushing up, how does this look to you?

export function isServerSide(checkEnv = true) {
	let serverSide = typeof process !== 'undefined' && !process.browser;
	if (serverSide && checkEnv) {
		serverSide =
			typeof process.env !== 'undefined' &&
			process.env.NODE_ENV !== 'test';
	}
	return serverSide;
}

Then in the case of nextTick, we would call isServerSide(false).

[jbalsas]

Hey @Robert-Frampton, yeah that should be fine, I'm just not a huge fan of Boolean Traps... it makes it hard to read the user code when you see isServerSide(true) you're pretty much always forced to look at the implementation or at least the method signature...

I know it's more verbose, but maybe better to have something like:

export function isServerSide(options = {checkEnv: true}) {
    console.log(options.checkEnv);
}

That could be called like:

isServerSide({checkEnv: false});

const checkEnv = false;
isServerSide({checkEnv});

[robframpton]

Sure let's do it that way.