Skip to content

Incorrect behavior after set injectGlobals: false in jest config #1240

Not planned
@krutoo

Description

@krutoo
"@testing-library/react": "^14.0.0",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",

Relevant code or config:

I'm trying to switch from injected globals to import needed functions from @jest/globals

i just add insectGlobals: false in jest.config.ts

What you did:

I added needed imports to test file and tests fails with errors like:

expect(received).toHaveLength(expected)

    Expected length: 0
    Received length: 1

What happened:

errors are reproduced only if there are two or more tests

it looks like the render result is not reset and they are rendered into one document

Reproduction:

Here an repo with example of incorrect behavior

https://github.com/krutoo/testing-library-react-jest-no-inject-globals

Activity

krutoo

krutoo commented on Oct 10, 2023

@krutoo
Author

if injectGlobas == undefined | true there is no problem

krutoo

krutoo commented on Oct 10, 2023

@krutoo
Author

it has same behavior in Bun test

leonadler

leonadler commented on Oct 11, 2023

@leonadler

I encountered the same problem with vitest (config.test.globals true/false)

This seems to solve it in vitest - please check if something similar works in Jest / Bun test:

// vitest.config.ts
export default defineConfig({
  // ...
  setupFiles: ['src/testing/setupTests.tsx')]
});
// setupTests.tsx
import { cleanup } from '@testing-library/react';
import { afterEach } from 'vitest';
afterEach(() => cleanup());

Jest has setupFiles and setupFilesAfterEnv, I'm not sure which one you would need.

Bun has bun test --preload ./setup.ts (1).


The reason this happens: with injectGlobals: false, there is no global beforeEach (duh), so react-testing-library would not know which beforeEach to call, as it is testing framework agnostic (i.e. you can use it without jest).

krutoo

krutoo commented on Oct 12, 2023

@krutoo
Author

@leonadler thank you for workaround

i will try to use this solution but

i think that it is need to be fixed in testing library

leonadler

leonadler commented on Oct 12, 2023

@leonadler

It can't really be, is what I'm saying - You might be using jest, or Bun (so you yourself are already using 2 test runners!), I am using vitest, someone else might be using mocha, ava, ...
React-Testing-Library would have to probe every test runner that exists, and would have to know which test runner you are using, which it can't - unless there is a beforeEach global, which you are disabling with the injectGlobals: false.

They could add a section about it in the readme, I would agree - but they can't detect your testing framework if you basically configure it to canBeDetected = false.

MatanBobi

MatanBobi commented on Oct 12, 2023

@MatanBobi
Member

Hi @krutoo, thanks for opening this.
As written in the docs for cleanup:

Please note that this is done automatically if the testing framework you're using supports the afterEach global and it is injected to your testing environment (like mocha, Jest, and Jasmine). If not, you will need to do manual cleanups after each test.

Since there's no afterEach, no cleanup is being called. If you believe this phrasing isn't clear enough, we're open for PR's in our docs repo.

I'm closing this one because as already written here by @leonadler, we can't have dedicated code for each test runner nor we wan't to inject an afterEach if it's not injected because that's too invasive.

leonadler

leonadler commented on Oct 12, 2023

@leonadler

@MatanBobi Sorry if this entirely the wrong place to ask, but next to the code that does beforeEach, you have this part:

    // This matches the behavior of React < 18.
    let previousIsReactActEnvironment = getIsReactActEnvironment()
    beforeAll(() => {
      previousIsReactActEnvironment = getIsReactActEnvironment()
      setReactActEnvironment(true)
    })

    afterAll(() => {
      setReactActEnvironment(previousIsReactActEnvironment)
    })

Do I understand correctly that with React >= 18 this is not necessary anymore? Or do consumers with setGlobals: false have to also do this beforeAll()+afterAll() logic?

MatanBobi

MatanBobi commented on Oct 12, 2023

@MatanBobi
Member

@leonadler AFAIR, IS_REACT_ACT_ENVIRONMENT wasn't available before React 18, what React did before was testing if typeof jest === 'undefined' so this code is 100% relevant for React >=18. If consumers use setGlobals: false`, they should do this on their own. I'm hesitating if we should write this in the docs too.

leonadler

leonadler commented on Oct 12, 2023

@leonadler

Ok, thanks - just FYI, with TypeScript this gets a bit unwieldly and awkward, so maybe the module could export getIsReactActEnvironment, setIsReactActEnvironment? 🤔 : (I can do the PR)

// my-project/external-modules.d.ts

declare module '@testing-library/react/dist/act-compat' {
  export function getIsReactActEnvironment(): unknown;
  export function setReactActEnvironment(env: unknown): void;
}
// my-project/setup-tests.ts
import { cleanup } from '@testing-library/react';
import { getIsReactActEnvironment, setReactActEnvironment } from '@testing-library/react/dist/act-compat';
// ^ meh 🙁
import { afterAll, afterEach, beforeAll, beforeEach, vitest } from 'vitest';

let previousIsReactActEnvironment = getIsReactActEnvironment();
beforeAll(() => {
  previousIsReactActEnvironment = getIsReactActEnvironment();
  setReactActEnvironment(true);
});
afterEach(() => cleanup());
afterAll(() => {
  setReactActEnvironment(previousIsReactActEnvironment);
});
MatanBobi

MatanBobi commented on Oct 12, 2023

@MatanBobi
Member

I understand.
Can you please open a feature request for this one so we'll be able to discuss it first? This is really an implementation detail that I'm not sure we want to be exported as part of our public API (or maybe not in this specific form).

krutoo

krutoo commented on Oct 12, 2023

@krutoo
Author

can package log some warning message about inability to apply automatic cleanup when afterEach is undefined?

MatanBobi

MatanBobi commented on Oct 13, 2023

@MatanBobi
Member

@krutoo I've created a PR to add a warning in that case.

MatanBobi

MatanBobi commented on Nov 17, 2023

@MatanBobi
Member

@krutoo - This PR was reverted because it created too much problems for our users. We'll re-circle it to see if there's a different approach to do it.

MattyBalaam

MattyBalaam commented on Nov 20, 2023

@MattyBalaam

@MatanBobi Is there a new issue to track this?

MatanBobi

MatanBobi commented on Nov 26, 2023

@MatanBobi
Member

@MattyBalaam - I just opened one: #1257

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @MattyBalaam@leonadler@MatanBobi@krutoo

        Issue actions

          Incorrect behavior after set injectGlobals: false in jest config · Issue #1240 · testing-library/react-testing-library