fix(clerk-js,ui): Preload component chunks in parallel during mount#7901
Open
jacekradko wants to merge 2 commits intomainfrom
Open
fix(clerk-js,ui): Preload component chunks in parallel during mount#7901jacekradko wants to merge 2 commits intomainfrom
jacekradko wants to merge 2 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: c5bd5b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
Member
Author
|
!snapshot |
Collaborator
|
Hey @jacekradko - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.3.0-snapshot.v20260221025222 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/fastify@2.7.0-snapshot.v20260221025222 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260221025222 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260221025222 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260221025222 --save-exact |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
📝 WalkthroughWalkthroughCalls to 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Replace the keepMounted Popover approach with chunk preloading. Pass preloadHint to ensureMounted() from all mount methods so component chunks download in parallel with the common chunk, eliminating the sequential waterfall on slow connections. Move preloadComponent() call outside the first-call guard so it fires on every ensureMounted() call.
jacekradko
force-pushed
the
jacek/user-4747-ui-delay-opening-user-menu-and-org-switcher-on-slow-internet
branch
from
9b06f86 to
cdc7d44
Compare
The __mocks__ directory in src/elements/ was not detected by Vitest for node_module mocks, so auto-animate ran in tests and leaked timers that fired after jsdom teardown, causing requestAnimationFrame errors.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/clerk-js/vitest.setup.mts`:
- Around line 265-270: Add unit tests that exercise the new preload/mount flow
and assert the component lifecycle now uses the explicit auto-animate mock:
write tests that simulate the preload step and then mount/render the component
and verify expected post-preload behavior (e.g., DOM state, callbacks invoked,
side effects) and that '@formkit/auto-animate/react' is mocked via
useAutoAnimate returning [null]; update or add tests in the clerk-js test suite
to cover both the preload->mount transition and that useAutoAnimate is not
causing timers or animations to run during teardown to prevent regressions.
Comment on lines
+265
to
+270
| // Mock @formkit/auto-animate to prevent timers leaking after test teardown. | ||
| // The __mocks__ directory in src/elements/ is not detected by Vitest for | ||
| // node_module mocks, so we need an explicit vi.mock here. | ||
| vi.mock('@formkit/auto-animate/react', () => ({ | ||
| useAutoAnimate: () => [null], | ||
| })); |
Contributor
There was a problem hiding this comment.
Add tests covering the behavioral change introduced in this PR.
No tests were added or updated to cover the new preload/mount behavior and the explicit auto-animate mock. Please add/adjust tests to validate the changed flow and guard against regressions.
As per coding guidelines “If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/clerk-js/vitest.setup.mts` around lines 265 - 270, Add unit tests
that exercise the new preload/mount flow and assert the component lifecycle now
uses the explicit auto-animate mock: write tests that simulate the preload step
and then mount/render the component and verify expected post-preload behavior
(e.g., DOM state, callbacks invoked, side effects) and that
'@formkit/auto-animate/react' is mocked via useAutoAnimate returning [null];
update or add tests in the clerk-js test suite to cover both the preload->mount
transition and that useAutoAnimate is not causing timers or animations to run
during teardown to prevent regressions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mount*()calls, eliminating the sequential download waterfall that caused slow first-render on poor connectionspreloadHintfrom all 16mount*methods toensureMounted()so the component-specific chunk starts downloading immediately alongside the common chunkpreloadComponent()outside the first-call guard inensureMounted()so it fires on every call, not just the first — enabling preloading even when a different component was mounted firstkeepMountedPopover approach (hidden container rendering) in favor of this simpler chunk preloading strategyResolves USER-4747
Testing PR: https://github.com/clerk/dashboard/pull/8496
How it works
Before:
mount*()→ common chunk downloads → React renders →React.lazy()triggers component chunk download (sequential waterfall)After:
mount*()→ common chunk and component chunk download in parallel → React renders →React.lazy()finds module already loaded (no wait)preloadComponentis idempotent (returns a cached promise on repeat calls), so this is safe to call on everyensureMounted()invocation.Test plan
pnpm buildpassespnpm testpasses (1616 tests, 111 test files)git diff main --statshows only intended changes (2 files for preloading + 3 files reverting keepMounted)Summary by CodeRabbit