testing-library / testing-library/dom-testing-library
`waitFor` doesn't advance time when using `jest` fake timers without injecting globals
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 3.3k
- Forks
- 474
- PR merge metrics
- No merged PRs in 30d
Description
@testing-library/domversion:9.3.4- Testing Framework and version:
jest@29.7.0 - DOM Environment:
jest-environment-jsdom@29.7.0
Relevant Code or Config:
A minimal sandbox to reproduce the issue is available on codesandbox. You can execute tests by opening a terminal and running the yarn test command.
Actions Undertaken:
- Configured
jestto not inject globals (--injectGlobals=false); refer to thepackage.jsoncommand from the sandbox above. - Created tests that rely on
waitFororwaitForElementToBeRemoved.
Outcome:
waitFornever advances timers by time (the default50msinterval of the loop or any other interval), causing the tests to exceed their timeout.
Reproduction:
You can reproduce this issue via the following codesandbox link.
Problem Description:
When using jest, but without injecting globals, the waitFor logic:
- Fails to identify
jest's environment in thejestFakeTimersAreEnabledpredicate. - Assuming the above was passing correctly, it would reference the
jestglobal object, which is undefined when not injecting globals. This would lead to an error when attempting to invokejest.advanceTimersByTime.
On a broader scope (although I want to keep this issue's focus narrow for the sake of actionability), there might be other unintended implicit assumptions about depending on (a) jest and / or (b) a test environment that injects globals. The following issues are related, although they focus on different manifested problems:
- https://github.com/testing-library/react-testing-library/issues/1240
- https://github.com/testing-library/react-testing-library/issues/1257
Suggested Solution:
- The first part of the issue (i.e.,
jestenvironment detection) can be resolved by adding anORcondition to thejestFakeTimersAreEnabledpredicate. In my experience, we should add a check forprocess.env.JEST_WORKER_IDto not beundefined. - The second issue could be addressed by dynamically attempting to import
jestfrom@jest/globalswhen (a) we detect aJEST_WORKER_IDand (b) we know fake timers are enabled.- I don't believe
dom-testing-libraryshould depend on@jest/globals. - I'm not completely satisfied with the tentative dynamic import of an optional dependency
@jest/globalsin this case, but I think it might provide the best developer experience for end users.
- I don't believe
Depending on the appetite for this issue to be fixed, I might work on a PR to address the above. I am somewhat surprised this hasn't come up before, but I guess not many people tend to set up jest without injecting globals.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the jestFakeTimersAreEnabled predicate in src/helpers.ts and the timer handling around line 81 of src/wait-for.js. Run the linked CodeSandbox tests with Jest 29.7.0 and --injectGlobals=false to observe the timeout. Done means waitFor and waitForElementToBeRemoved advance fake timers correctly without injected Jest globals, with coverage for the reproduced setup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100