petyosi / react-virtuoso

The most powerful virtual list component for React
https://virtuoso.dev
MIT License
5.25k stars 301 forks source link

Empty list when testing with react-testing-library #26

Closed brapifra closed 5 years ago

brapifra commented 5 years ago

Hi, first of all, congrats for your awesome library! I've been trying to test some components where I use it, but the list never gets rendered :disappointed:

import * as React from 'react';
import { Virtuoso } from 'react-virtuoso';
import {  render } from '@testing-library/react';

function Component () {
     return (
         <Virtuoso
                totalCount={100}
                item={index => <div>{index}</div>}
          />
     );
}

describe('Test', () => {
   it('should render list of items', () => {
        const { debug } = render(<Component />);
        debug(); // Prints an empty list
   });
});
petyosi commented 5 years ago

Hi,

I am not familiar with the testing-library internals, but my guess is that it is executed in a node environment, bypassing the hooks, effects, etc. If this is so, you can probably make it work by setting the initialItemCount prop, available in v0.10 (released yesterday). Check the demo: https://stackblitz.com/edit/react-virtuoso-server-side-rendering?file=example.js It is also mentioned in the API reference: https://virtuoso.dev/virtuoso-api-reference/

Please let me know if this works for you since it is a new feature.

brapifra commented 5 years ago

That works! :tada: Ty

Zache commented 4 years ago

This works for me also, only with the added problem that with it initialItemCount set React re-renders the component endlessly. The profiler says because of hooks changed. I'm dealing with this by only setting initialItemCount during tests, but it would be nice to not have to do that.

sallyhill commented 3 years ago

Hi there, is there a way to programmatically scroll in unit tests that don't define refs? Enzyme testing strips out refs on components, so scrollToIndex and anything attached to that Virtuoso ref don't seem to be accessible. Given I'm using window scrolling, I'm not sure how else to trigger the visible rendered items to shift. I've tried window.scrollTo and scrollToElement on nodes. Sorry this maybe more an enzyme or jsdom question, but it might be helped if someone has a better understanding of where window scroll listeners are being attached so I can work out how to trigger the right ones.

wjureczka commented 3 years ago

Hi, is there any solution for that? initialItemCount seems not to work in my case. Scenario is:

  1. Component has an empty array from prop so no list items are rendered - good
  2. That arrays changes and has one element, but no list item is rendered - bad
kyle-sawatsky commented 3 years ago

@wjureczka I tried that solution as well in a jest test. Oddly I swear the first time I ran the test after putting in initialItemCount it worked and then subsequently it never worked again.

I get it because the js-dom environment doesn't really give anything true dimensions. So the checks that Virtuoso makes to figure out how much to render never result in anything.

gustavo-depaula commented 3 years ago

@wjureczka I have the same problem

OzzieOrca commented 3 years ago

It seems like initialItemCount has to be manually set for tests to work. If it's set to 0, I can get the EmptyPlaceholder to show. If it's set to a value higher than the length of the data, it throws errors trying to render undefined items. Since I'm loading my data from GraphQL, the length initially is 0 but afterwards it is set to a number. But If I set initialItemCount to the length of the data, it doesn't update on subsequent render.

Is there a better way to test this component? Guess I might end up mocking it.

petyosi commented 3 years ago

@OzzieOrca Honestly, I'm not sure what you are trying to test here. Can you describe the overall idea?

stazrad commented 3 years ago

SOLVED- same problem working with @testing-library/react...the data is correct but the list item is never rendered to the "dom" (or whatever equivalent this testing library is using to generate a "document"). It seems that itemContent is never fired at all when mounting in the test environment.

solved by setting initialItemCount to data.length

NuclleaR commented 3 years ago

Hi, is there any solution for that? initialItemCount seems not to work in my case. Scenario is:

  1. Component has an empty array from prop so no list items are rendered - good
  2. That arrays changes and has one element, but no list item is rendered - bad

Any ideas how to check this case?

MaxboDev commented 2 years ago

I'm still having problems with this. I managed to get it working by setting initialItemCount to the size of the list but then I had to test some functionality which changed the size of the list and react-virtuoso wouldn't render the updated list even if I updated the initialItemCount.

My test environment isn't anything unusual, its the jest with jsdom setup offered out of the box by create-react-app. Is there a suggested way of getting this to work in that test environment?

Currently I've had to mock out react-virtuoso in my tests, but I'd really rather be running the real code in my tests. Any help would be super appreciated. Thanks!

NordlingDev commented 2 years ago

@M4sterShake - One way to bypass this is to assign a key to the Virtuoso component if the node environment is test. This will of course render a new virtuoso instance if the items count changes, but in a test environment I don't think it's such a big case to deal with.

So, if you set initialItemCount to be the length of the data, set it to key as well:

const totalCount = data.length;

<Virtuoso
  totalCount={totalCount}
  {...someProps}
  {...(process.env.NODE_ENV === 'test'
    ? {
      initialItemCount: totalCount,
      key: totalCount
    }
    : {}
  )}
/>
tanato-cit commented 2 years ago

The two provided solutions didn't seemed great to me,

Setting the initialItemCount fixed in the code when not in SSR is quite dangerous, as it'll basically render all elements thus removing the purpose of Virtualization and also can lead to multiple renders.

The second solution to override the initialItemCount on the component code that uses Virtuoso according to the environment that's running (test vs production) doesn't seem great either as the component code will be messy.

A better solution in my opinion is to override the Virtuoso implementation by creating a mocked version with a HOC that set this attribute, like this: (in this example I'm setting the initialItemCount to the same value as totalCount so it render all elements)

import { ElementType, Component as mockComponent } from 'react'

// ...

jest.mock('react-virtuoso', () => {
  const { Virtuoso } = jest.requireActual('react-virtuoso')

  const mockVirtuoso = (WrappedVirtuoso: ElementType) =>
    class extends mockComponent<{ totalCount: number }, unknown> {
      render() {
        return <WrappedVirtuoso initialItemCount={this.props?.totalCount} {...this.props} />
      }
    }
  return { Virtuoso: mockVirtuoso(Virtuoso) }
})
StephenTHendrix commented 2 years ago

The example mocking Virtuoso above is excellent!

tanato-cit commented 2 years ago

I'm still having problems with this. I managed to get it working by setting initialItemCount to the size of the list but then I had to test some functionality which changed the size of the list and react-virtuoso wouldn't render the updated list even if I updated the initialItemCount.

My test environment isn't anything unusual, its the jest with jsdom setup offered out of the box by create-react-app. Is there a suggested way of getting this to work in that test environment?

Currently I've had to mock out react-virtuoso in my tests, but I'd really rather be running the real code in my tests. Any help would be super appreciated. Thanks!

You can mock AND still run the real code, as in the example above, it just override one property

philmeta0 commented 2 years ago

Here's an alternative approach that mocks out the Virtuoso component to render the entire list without any wrapper in our test suite. We just added this to our setupTests.tsx file.

jest.mock('react-virtuoso', () => {
  function Virtuoso(props: VirtuosoProps<unknown, unknown>) {
    return <>{props.data?.map((value, index) => props.itemContent?.(index, value, undefined))}</>;
  }

  return { ...jest.requireActual('react-virtuoso'), Virtuoso };
});
stassiek02 commented 2 years ago

@aimad-majdou Did you import mockComponent like this? import { ElementType, Component as mockComponent } from 'react'

mstykow commented 2 years ago

If you're getting "Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?" do this:

jest.mock('react-virtuoso', () => {
  const Virtuoso = forwardRef((props: VirtuosoProps<unknown, unknown>, _) => {
    return <>{props.data?.map((value, index) => props.itemContent?.(index, value, undefined))}</>;
  })

  return { ...jest.requireActual('react-virtuoso'), Virtuoso };
});
petyosi commented 2 years ago

For everyone finding this issue - there's a new recommended approach, check this improved approach: https://virtuoso.dev/mocking-in-tests/.

jonbri commented 1 year ago

Thanks for the above solution @petyosi , it's mostly working for me. Something that seems to make it break however is when the initialTopMostItemIndex prop is used.

Here is my unit test, forked from the example given in the doc:

import { createElement } from 'react';
import { render, screen } from '@testing-library/react';
import { Virtuoso, VirtuosoMockContext } from 'react-virtuoso';

describe('Virtuoso', () => {
  const totalCount = 100;
  type Item = { id: string; value: string };
  const data: Item[] = [...Array(totalCount).keys()].map((i) => ({
    id: i.toString(),
    value: `Item ${i}`,
  }));

  it('correctly renders items starting at index', () => {
    render(<Virtuoso data={data} initialTopMostItemIndex={50} />, {
      wrapper: ({ children }) => (
        <VirtuosoMockContext.Provider value={{ viewportHeight: 300, itemHeight: 100 }}>
          {children}
        </VirtuosoMockContext.Provider>
      ),
    });

    screen.getByText('Item 50'); // this fails
  });
});

Here are the results:

  ● Virtuoso › correctly renders items starting at index

    TestingLibraryElementError: Unable to find an element with the text: Item 50. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

    Ignored nodes: comments, <script />, <style />
    <body>
      <div>
        <div
          data-test-id="virtuoso-scroller"
          data-virtuoso-scroller="true"
          style="height: 100%; outline: none; overflow-y: auto; position: relative;"
          tabindex="0"
        >
          <div
            data-viewport-type="element"
            style="width: 100%; height: 100%; position: absolute; top: 0px;"
          >
            <div
              data-test-id="virtuoso-item-list"
              style="box-sizing: border-box; padding-top: 0px; padding-bottom: 10000px; margin-top: 0px;"
            />
          </div>
        </div>
      </div>
    </body>

      20 |     });
      21 |
    > 22 |     screen.getByText('Item 50'); // this fails
         |            ^
      23 |   });
      24 | });
      25 |

      at Object.getElementError (../node_modules/@testing-library/dom/dist/config.js:38:19)
      at ../node_modules/@testing-library/dom/dist/query-helpers.js:90:38
      at ../node_modules/@testing-library/dom/dist/query-helpers.js:62:17
      at ../node_modules/@testing-library/dom/dist/query-helpers.js:111:19
      at Object.<anonymous> (components/List/__tests__/Virtuoso.spec.tsx:22:12)

For some reason, it's not rendering any items when the initialTopMostItemIndex prop is used. If I simply remove initialTopMostItemIndex the items are rendered per-usual.

I tried adding different combinations of initialItemCount (initialItemCount={totalCount}) and key (key={totalCount}) but they didn't seem to have any affect.

Has anyone else come across this?

petyosi commented 1 year ago

@jonbri that's logical given how initialTopMostItemIndex works - it 1) scrolls to a specific location 2) waits for the scrollTop to be reported to render.

https://github.com/petyosi/react-virtuoso/blob/master/src/initialTopMostItemIndexSystem.ts#L31-L48 https://github.com/petyosi/react-virtuoso/blob/master/src/listStateSystem.ts#L225-L232

If you're up for it, I am happy to accept a PR.

alvarogfn commented 1 year ago

If anyone is having the same problem as @jonbri, and can't modify the code directly, my temporary solution was:

jest.mock('react-virtuoso', () => ({
  ...jest.requireActual<typeof VirtuosoModule>('react-virtuoso'),
  Virtuoso:
    // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any
    forwardRef<VirtuosoHandle, VirtuosoProps<any, any>>(({ initialTopMostItemIndex, ...props }, ref) => {
      const { Virtuoso } = jest.requireActual<typeof VirtuosoModule>('react-virtuoso')
      return React.createElement(Virtuoso, { ref, ...props })
    })
}))