When running in Jest, code assumes `setImmediate` and `clearImmediate`

[SimenB]

• @testing-library/dom version: 7.29.6
• Testing Framework and version: Jest@fix(env-jsdom): remove setImmediate and clearImmediate jestjs/jest#11222
• DOM Environment: JSDOM@16.4.0

Relevant code or config:

require('@testing-libray/react')

What you did:

Ran the test without global setImmediate and clearImmediate defined

What happened:

Reproduction:

See jestjs/jest#11222. Essentially, delete setImmediate and clearImmediate from global in the JSDOM env Jest sets up.

Problem description:

It throws runtime errors at require/import-time

Suggested solution:

Do a typeof setImmediate !== 'undefined' before using it.

[kentcdodds]

If I'm not mistaken, the only place we rely on a global setImmediate is here:

dom-testing-library/src/helpers.js

Lines 18 to 25 in 300bfe2

	const timerAPI = {
	clearImmediate,
	clearInterval,
	clearTimeout,
	setImmediate,
	setInterval,
	setTimeout,
	}

That could probably be changed to use the setImmediatePolyfill and we could probably create a clearImmediate pollyfill as well.

[craigkovatch]

What is it used for? setImmediate is only supported by IE10/11, and not on any kind of standards plan. To me it seems an odd choice to polyfill something that won’t ever exist natively in a modern browser.

[gaearon]

So it seems like the place that uses this (linked above) is just this helper which is meant to runWithJestRealTimers. But if Jest removes setImmediate in jsdom (which is the plan), then there's no need for setImmediate to be in that list.

Therefore, the fix is just to change this code to be resilient to setImmediate not existing. I don't think any polyfilling is needed there.

[renatoalencar]

What is it used for? setImmediate is only supported by IE10/11, and not on any kind of standards plan. To me it seems an odd choice to polyfill something that won’t ever exist natively in a modern browser.

setImmediate is supported by Node.js and it looks like is used to wait for changes in the DOM. There's already a polyfill in the code but used in other part of the code. It looks like this issue comes from this function runWithJestRealTimers, which really doesn't make sense to patch with a polyfill.

[gaearon]

Proposed fix:

  const timerAPI = {
    clearInterval,
    clearTimeout,
    setInterval,
    setTimeout,
  }
  if (typeof setImmediate === 'function') {
    timerAPI.setImmediate = setImmediate;
  }
  if (typeof clearImmediate === 'function') {
    timerAPI.clearImmediate = clearImmediate;
  }
  // ...

[renatoalencar]

Proposed fix:

  const timerAPI = {
    clearInterval,
    clearTimeout,
    setInterval,
    setTimeout,
  }
  if (typeof setImmediate === 'function') {
    timerAPI.setImmediate = setImmediate;
  }
  if (typeof clearImmediate === 'function') {
    timerAPI.clearImmediate = clearImmediate;
  }
  // ...

This function depends on setImmediate to work properly in order to work, I think this could probably use the polyfill declared below in the same file.

function waitForDomChange({
  container = getDocument(),
  timeout = getConfig().asyncUtilTimeout,
  mutationObserverOptions = {
    subtree: true,
    childList: true,
    attributes: true,
    characterData: true,
  },
} = {}) {
  if (!hasWarned) {
    hasWarned = true
    console.warn(
      `\`waitForDomChange\` has been deprecated. Use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.`,
    )
  }
  return new Promise((resolve, reject) => {
    const timer = setTimeout(onTimeout, timeout)
    const {MutationObserver} = getWindowFromNode(container)
    const observer = new MutationObserver(onMutation)
    runWithRealTimers(() =>
      observer.observe(container, mutationObserverOptions),
    )

    function onDone(error, result) {
      clearTimeout(timer)
      setImmediate(() => observer.disconnect())
      if (error) {
        reject(error)
      } else {
        resolve(result)
      }
    }

    function onMutation(mutationsList) {
      onDone(null, mutationsList)
    }

    function onTimeout() {
      onDone(new Error('Timed out in waitForDomChange.'), null)
    }
  })
}

[facundop3]

Proposed fix:

  const timerAPI = {
    clearInterval,
    clearTimeout,
    setInterval,
    setTimeout,
  }
  if (typeof setImmediate === 'function') {
    timerAPI.setImmediate = setImmediate;
  }
  if (typeof clearImmediate === 'function') {
    timerAPI.clearImmediate = clearImmediate;
  }
  // ...

This function depends on setImmediate to work properly in order to work, I think this could probably use the polyfill declared below in the same file.

function waitForDomChange({
  container = getDocument(),
  timeout = getConfig().asyncUtilTimeout,
  mutationObserverOptions = {
    subtree: true,
    childList: true,
    attributes: true,
    characterData: true,
  },
} = {}) {
  if (!hasWarned) {
    hasWarned = true
    console.warn(
      `\`waitForDomChange\` has been deprecated. Use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.`,
    )
  }
  return new Promise((resolve, reject) => {
    const timer = setTimeout(onTimeout, timeout)
    const {MutationObserver} = getWindowFromNode(container)
    const observer = new MutationObserver(onMutation)
    runWithRealTimers(() =>
      observer.observe(container, mutationObserverOptions),
    )

    function onDone(error, result) {
      clearTimeout(timer)
      setImmediate(() => observer.disconnect())
      if (error) {
        reject(error)
      } else {
        resolve(result)
      }
    }

    function onMutation(mutationsList) {
      onDone(null, mutationsList)
    }

    function onTimeout() {
      onDone(new Error('Timed out in waitForDomChange.'), null)
    }
  })
}

@renatoalencar is it possible to switch the setImmediate in favour of setTimeout(()=> observer.disconnect(), 0) in waitForDomChange implementation ?

[renatoalencar]

Proposed fix:

  const timerAPI = {
    clearInterval,
    clearTimeout,
    setInterval,
    setTimeout,
  }
  if (typeof setImmediate === 'function') {
    timerAPI.setImmediate = setImmediate;
  }
  if (typeof clearImmediate === 'function') {
    timerAPI.clearImmediate = clearImmediate;
  }
  // ...

This function depends on setImmediate to work properly in order to work, I think this could probably use the polyfill declared below in the same file.

function waitForDomChange({
  container = getDocument(),
  timeout = getConfig().asyncUtilTimeout,
  mutationObserverOptions = {
    subtree: true,
    childList: true,
    attributes: true,
    characterData: true,
  },
} = {}) {
  if (!hasWarned) {
    hasWarned = true
    console.warn(
      `\`waitForDomChange\` has been deprecated. Use \`waitFor\` instead: https://testing-library.com/docs/dom-testing-library/api-async#waitfor.`,
    )
  }
  return new Promise((resolve, reject) => {
    const timer = setTimeout(onTimeout, timeout)
    const {MutationObserver} = getWindowFromNode(container)
    const observer = new MutationObserver(onMutation)
    runWithRealTimers(() =>
      observer.observe(container, mutationObserverOptions),
    )

    function onDone(error, result) {
      clearTimeout(timer)
      setImmediate(() => observer.disconnect())
      if (error) {
        reject(error)
      } else {
        resolve(result)
      }
    }

    function onMutation(mutationsList) {
      onDone(null, mutationsList)
    }

    function onTimeout() {
      onDone(new Error('Timed out in waitForDomChange.'), null)
    }
  })
}

@renatoalencar is it possible to switch the setImmediate in favour of setTimeout(()=> observer.disconnect(), 0) in waitForDomChange implementation ?

I'm currently trying to see if there's any way to break this, and actually works just fine. Let me do some more tests and see.