Skip to content

bug: inconsistent React.lazy behavior when running single and multiple tests #788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pahan35 opened this issue Sep 17, 2020 · 2 comments
Closed

Comments

@pahan35
Copy link

pahan35 commented Sep 17, 2020

  • @testing-library/react version: 11.0.4
  • Testing Framework and version:
    jest: 26.4.2
  • DOM Environment:
    jsdom: 16.4.0

Relevant code or config:

Parent.test.js

import {render} from '@testing-library/react'
import Parent from 'Components/LazyLoadInconsistency/Parent'
import React from 'react'

test('single test 1', async () => {
  const {findByText} = render(<Parent />)
  expect(await findByText(/Count is 2/i)).toBeInTheDocument()
})

test('single test 2', async () => {
  const {findByText} = render(<Parent />)
  expect(await findByText(/Count is 2/i)).toBeInTheDocument()
})

Parent.js

import React, {Suspense, useEffect, useRef, useState} from 'react'

const LazyChild = React.lazy(() =>
  import('Components/LazyLoadInconsistency/LazyChild'),
)

class Api {
  constructor(onSearch) {
    this.onSearch = onSearch
  }

  onReady() {
    this.ready = true
    this.search()
  }

  search() {
    if (!this.ready) {
      return
    }
    this.onSearch()
  }
}

export default function Parent() {
  const [count, setCount] = useState(0)
  const apiRef = useRef(new Api(() => setCount(prevCount => prevCount + 1)))

  useEffect(() => {
    apiRef.current.onReady()
  }, [])

  return (
    <Suspense fallback={null}>
      <LazyChild apiRef={apiRef} />
      <span>Count is {count}</span>
    </Suspense>
  )
}

LazyChild.js

import React, {useEffect} from 'react'

export default function LazyChild({apiRef}) {
  useEffect(() => {
    apiRef.current.search()
  }, [apiRef])
  return <div>I&apos;m lazy</div>
}

What you did:

I'm running multiple tests from Parent.test.js during a single run.

What happened:

The second test fails even if it is the same as the first one, which passed successfully.

Reproduction:

Please use this repo https://github.com/pahan35/web-ui-boilerplate and branch bug/lazy-inconsistency to reproduce the described problem.

  1. Clone the mentioned repo.
  2. Switch to branch bug/lazy-inconsistency.
  3. npm i.
  4. Run single tests and check that they work as expected
$ jest -t "single test 1"
$ jest -t "single test 2"
  1. Try to run the whole test file
$ jest Parent.test.js

It fails :(

Here is a failure on Travis CI.

Problem description:

When I debugged this test run inconsistency, I noticed that React.lazy() is called only once. Because of it, useEffect from LazyChild is fired faster than useEffect from Parent, as it was on the first run, and it is in the real environment, and we see inconsistency when running tests in different modes. Recorded screencast with debugging logging

Suggested solution:

I don't know which side it should be fixed, but I think React.lazy should be fired on each test run to be as close to the real environment as possible. Here is only one call to React.lazy() recorded.

@eps1lon
Copy link
Member

eps1lon commented Sep 18, 2020

There is no guarantee for the effect call order in React which changes depending on whether React.lazy is pending or already resolved. Only cleanups have stable order. See facebook/react#16728 and facebook/react#15281 for more information.

This is unfortunately how React.lazy behaves: In the first test the Child won't resolve immediately so you call onReady() in the parent and later, once it resolves, search() in the child. However, in the second test Child will already be resolved and mount immediately. In that case child effects run before parent effects and search() will be called while this.ready is still false (because the parent didn't call onReady yet).

For a different scenario with a similar issue see #716. It also includes a workaround: #716 (comment)

@eps1lon eps1lon closed this as completed Sep 18, 2020
@pahan35
Copy link
Author

pahan35 commented Sep 18, 2020

@eps1lon it works with the approach proposed in the workaround. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants