-
Notifications
You must be signed in to change notification settings - Fork 345
fix(clerk-js): Fix act output in tests [SDK-1054] #2289
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
Conversation
🦋 Changeset detectedLatest commit: 3fa0fe9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 No more "act" errors 🎉
packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/InviteMembersPage.test.tsx
Outdated
Show resolved
Hide resolved
await waitFor(() => { | ||
expect(getByRole('tab', { name: 'Requests' })).toBeInTheDocument(); | ||
}); | ||
await waitForLoadingCompleted(container); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
await waitForLoadingCompleted(container); | ||
|
||
const pagination = getByText(/displaying/i).closest('p'); | ||
expect(pagination?.textContent).toEqual('Displaying 1 – 5 of 5'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
const { wrapper } = await createFixtures(f => { | ||
f.withOrganizations(); | ||
f.withUser({ email_addresses: ['[email protected]'] }); | ||
}); | ||
const { queryByRole } = await act(() => render(<OrganizationSwitcher />, { wrapper })); | ||
|
||
jest.advanceTimersByTime(15000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do u remember what was the issue here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this, but a lot of the issues boil down to useDelayedVisibility
and its use of setTimeout
.
@@ -11,25 +11,28 @@ import { createFakeMember, createFakeOrganizationInvitation, createFakeOrganizat | |||
|
|||
const { createFixtures } = bindCreateFixtures('OrganizationProfile'); | |||
|
|||
async function waitForLoadingCompleted(container: HTMLElement) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Is it possible that the issue is related to using getBy...
instead of findBy...
which is a combination of waitFor
and getBy...
?
I would prefer we avoid mingling the logic to wait for a loader to be removed with the usage of waitForLoadingCompleted
and depend on findBy...
instead.
* fix(clerk-js): Fix unhandled act errors [SDK-1054] * chore(*): Add changeset * chore(clerk-js): Add missing awaits * chore(clerk-js): Minor updates
Description
Fixes
act
(and oneconsole.log
) output in Jest tests forclerk-js
.SDK-1054
Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change
Packages affected
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/clerk-expo
@clerk/fastify
gatsby-plugin-clerk
@clerk/localizations
@clerk/nextjs
@clerk/clerk-react
@clerk/remix
@clerk/clerk-sdk-node
@clerk/shared
@clerk/themes
@clerk/types
build/tooling/chore