What are the expectations around setTimeout/setImmediate after environment is torn down?

[gaearon]

We're running into some difficulties while making changes to React. I can explain them in more depth later, but the tldr is we tried to replace setTimeout with setImmediate in one place, and this led to a bunch of issues. Before diving deeper into that rabbit hole, I'm curious if there's any well-defined thinking about what should happen to timeouts/immediates after the Jest test environment is torn down.

In particular, I'm thinking about a case where:

• A test uses real setTimeout or setImmediate (no fake timers)
• It runs in a Jest jsdom environment
• The code inside the callback might access the DOM

it('test', () => {
  setTimeout(() => {
    // do something with window
  }, 10);

  setImmediate(() => {
    // do something with window
  });
})

I'm getting the impression is that Jest doesn't do anything special for real timers. Which would explain why, for example, setTimeout simply won't fire if this is the only test file, but would fire if the process is still running (e.g. due to async tests or the worker being reused for another test suite). Or why setImmediates run after jsdom is already torn down, causing an error if we use DOM there.

I want to understand if my explanation above is correct, and whether this behavior actually makes sense. Conceptually, a "real" Node environment is not isolated. E.g. module system isolation or jsdom teardown/setup is a Jest feature. It's not something built-in. By this logic, wouldn't it make sense to also isolate any "real" setTimeout and setImmediate between test files? For example, by consistently clearing them when the test environment is torn down instead of allowing "zombie" timeouts and immediates to execute. Or by forcing all immediates to run before jsdom is torn down.

Any thoughts? I can prepare more detailed examples if this doesn't make sense.

[gaearon]

cc @kentcdodds in case you've run into this with React Testing Library

[kentcdodds]

Thanks for the mention. I've run into these kinds of problems for sure. I like the proposed solution and it's what I do when I'm mocking timers and I would do it with real timers if I could. For example: https://github.com/kentcdodds/bookshelf/blob/main/src/setupTests.js#L53

[gaearon]

Here's a few examples (all are reproducible with the default repl.it)

This test suite passes:

test('hello', () => {
  setTimeout(() => {
    throw Error('hi');
  }, 0)
});

It's because the process exits before we've had a chance to run timeouts.

However, this other test suite fails:

test('hello', () => {
  setTimeout(() => {
    throw Error('hi');
  }, 0)
});

// I just added this to make it fail:
test('oh no', async () => {
  await new Promise(resolve => setTimeout(resolve, 1000));
});

This is because the first timeout actually got a chance to run now, causing the crash. This seems like a source of non-determinism.

Curiously, I can't reproduce this problem if I move these tests apart into different files, even if they run in band. I'm not sure why. Does it mean there's already some kind of a cleanup mechanism for setTimeout?

For example, if I set setTimeout.foo = true in one file, I don't see setTimeout.foo in other file. Why?

[gaearon]

Now regarding setImmediate.

This seems to bring down the Worker entirely:

test('hello', () => {
  setImmediate(() => {
    throw Error('hi');
  })
});

I get exit status 1 instead of the normal passed/failed summary.

This code also crashes Jest:

test('hello', () => {
  document.createElement('div');
  setImmediate(() => {
    document.createElement('div'); // Cannot read property 'createElement' of null
  })
});

However, if I run multiple test files, of course, document exists in each of them, which means it's this particular environment that got torn down by the time setImmediate runs.

Note how setTimeout does not have the same issue because it simply won't run after teardown:

test('hello', () => {
  document.createElement('div');
  setTimeout(() => {
    document.createElement('div') // Works fine
  }, 0)
});

[gaearon]

Here's another fun curiosity.

This code passes fine in Jest:

/** @jest-environment jsdom */

test('hello', () => {
  setTimeout(() => {
    throw Error('hi')
  }, 0);
});

However, if we switch to node environment, it will crash the Jest runner:

/** @jest-environment node */

test('hello', () => {
  setTimeout(() => {
    throw Error('hi')
  }, 0)
});

This gives me exit status 1, just like setImmediate in jsdom environment. It also crashes the next test file (if it exists).

This makes me think that maybe jsdom Jest environment does something special with timeouts in particular?

[gaearon]

👀

https://github.com/facebook/jest/blob/75e94cc26714a9f23722dc127381c8e869ba42d8/packages/jest-util/src/installCommonGlobals.ts#L65-L68

[gaearon]

Proposal to remove the inconsistency: #11204

[SimenB]

Most of my reply at #11204 (comment) probably applies here instead 😅

I think long term, the ideal way for Jest to handle this is to disallow scheduling any work after the test environment has been torn down (or is in the process if being torn down) or run any previously scheduled work. Once a test has completed, no more of its code should run (mostly as stack traces and figuring out where this work was scheduled is confusing as we cannot connect it to. any specific test or test suite). This should probably happen for both "real" and "fake" timers, which'd mean we'd need to override all the timer functions manually to ensure we can track them.

Whether or not that means to throw when it happens or just make it into a no-op is a separate discussion, but it can also be up to the environment itself (i.e. the envs shipped by Jest could throw, but some env testEnvironmentOptions could disable it or something).

---

For example, if I set setTimeout.foo = true in one file, I don't see setTimeout.foo in other file. Why?

When you say "other file" does that mean another file loaded for the same test, or in a separate test? If the latter it's just because they get fresh globals each time. If the former... that sounds weird 😅

---

/cc @thymikee @jeysal thoughts?

[gaearon]

When you say "other file" does that mean another file loaded for the same test, or in a separate test? If the latter it's just because they get fresh globals each time. If the former... that sounds weird 😅

Yes, what I meant is that two different files "see" different setTimeout globals but they see the same setImmediate global. But the source code that shows it's directly lifted from the top-level environment explains it.

[github-actions]

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.