Skip to content
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

Sidebar: Unable to write unit test: groupToDisplayedElements[group].findLastIndex is not a function #5954

Closed
hans-ts opened this issue Feb 13, 2024 · 11 comments · Fixed by #5968
Assignees
Labels
Component: Test Issue or pull request is related to Component Testing
Milestone

Comments

@hans-ts
Copy link
Contributor

hans-ts commented Feb 13, 2024

Describe the bug

I have written some wrapper components around Primereact and updating from 10.3.0 to 10.3.1 causes my vi-test / testing-library tests for the sidebar to fail with the following error:

TypeError: groupToDisplayedElements[group].findLastIndex is not a function
 ❯ node_modules/primereact/hooks/hooks.cjs.js:279:57
 ❯ safelyCallDestroy node_modules/react-dom/cjs/react-dom.development.js:22932:5
 ❯ commitHookEffectListUnmount node_modules/react-dom/cjs/react-dom.development.js:23100:11
 ❯ commitPassiveUnmountInsideDeletedTreeOnFiber node_modules/react-dom/cjs/react-dom.development.js:25101:11
 ❯ commitPassiveUnmountEffectsInsideOfDeletedTree_begin node_modules/react-dom/cjs/react-dom.development.js:25048:5
 ❯ commitPassiveUnmountEffects_begin node_modules/react-dom/cjs/react-dom.development.js:24956:11
 ❯ commitPassiveUnmountEffects node_modules/react-dom/cjs/react-dom.development.js:24941:3
 ❯ flushPassiveEffectsImpl node_modules/react-dom/cjs/react-dom.development.js:27038:3
 ❯ flushPassiveEffects node_modules/react-dom/cjs/react-dom.development.js:26984:14
 ❯ commitRootImpl node_modules/react-dom/cjs/react-dom.development.js:26935:5

Here is a basic test using the Primereact sidebar component straight from the box that shows the problem:

describe(
  'Sidebar',
  () => {
    it(
      'Should be possible to show the sidebar immediately',
      () => {
        const onHideCallback = vi.fn();
        render(<Sidebar visible={true} onHide={onHideCallback} />);

        const sidebar = screen.queryByRole('complementary');
        expect(sidebar).not.toBeInTheDocument();
      }
    );
);

As far as I can tell, all the assertions in the test complete successfully, but then there is a problem unmounting the component.

Reproducer

No response

PrimeReact version

10.3.1

React version

17.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

No response

Steps to reproduce the behavior

No response

Expected behavior

No response

@hans-ts hans-ts added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Feb 13, 2024
@melloware melloware added Component: Test Issue or pull request is related to Component Testing and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Feb 13, 2024
@melloware
Copy link
Member

Its because of useDisplayOrder

https://github.com/primefaces/primereact/blob/master/components/lib/hooks/useDisplayOrder.js

It looks like something needs to be initialized?

@hans-ts
Copy link
Contributor Author

hans-ts commented Feb 13, 2024

Before submitting the bug I tried wrapping the test in <PrimeReactProvider />, but that did not make a difference.

@melloware
Copy link
Member

Can you submit a PR with test showing the issue?

@hans-ts
Copy link
Contributor Author

hans-ts commented Feb 13, 2024

I can for sure try. Is there a guide on where to start?

@melloware
Copy link
Member

We have other tests. You just need to create a spec.js file similar to this test of the Chip component

components/lib/chip/Chip.spec.js

@hans-ts
Copy link
Contributor Author

hans-ts commented Feb 14, 2024

Thank you for your help. I was able to reproduce with the following:

import '@testing-library/jest-dom';
import { render, screen } from '@testing-library/react';
import { Sidebar } from './sidebar';

describe('Sidebar', () => {
    test(
        'can be made visible',
        () => {
            const hideOn = jest.fn();
            render(<Sidebar visible={true} onHide={hideOn} />);

            const sidebar = screen.queryByRole('complementary');
            expect(sidebar).toBeVisible();
        }
    );
});

I am not at my work station that is set up with all my git keys, but I will create a PR tomorrow.

@melloware melloware added this to the 10.5.2 milestone Feb 14, 2024
@hans-ts
Copy link
Contributor Author

hans-ts commented Feb 14, 2024

@melloware I figured out the problem. The issue was not with the implementation of the hook, rather, that I was on NodeJS v16. findLastIndex is not available until at least v18. I upgraded to 20.11.0 and the test now completes successfully.

I am happy to still create a PR with the .spec file if you like, but other than that, I consider this resolved.

@melloware
Copy link
Member

Yes i would still love a spec file!

@hans-ts
Copy link
Contributor Author

hans-ts commented Feb 14, 2024

PR created, but I am having a hard time linking it to this issue correctly.

melloware added a commit that referenced this issue Feb 14, 2024
* Add a test for the sidebar component.

* Update Sidebar.spec.js

---------

Co-authored-by: Melloware <[email protected]>
@hans-ts
Copy link
Contributor Author

hans-ts commented Feb 14, 2024

@melloware Thank you for all your help!

@melloware
Copy link
Member

thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Test Issue or pull request is related to Component Testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants