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

chore(clerk-js,types): Load tasks based on environment settings #5422

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

LauraBeatris
Copy link
Member

@LauraBeatris LauraBeatris commented Mar 21, 2025

Description

This PR removes the previous experimental.withSessionTasks flag in favor of preloading SessionTasks component based on the current environment settings, to avoid downloading its chunk of JS for instances that aren't using this feature.

Breakdown

Main changes:

  • Introduce a new root path (/tasks) so that SessionTasks can declare inner routes to be reused across SignIn/SignUp
  • With the above, SessionTasks can now be used as standalone, outside of SignIn/SignUp paths, such as rendering it on a catchall (/onboarding/[[...index]].tsx) -> coming in a next PR
  • Also with the root path, allows to redirect from @clerk/backend when the incoming JWT claims contain sts pending -> coming in a next PR

Misc:

  • Introduce webpack chunk for SessionTasks on bundlewatch to track size across PRs
  • Fix nomenclature to be consistent across tasks on plural, since that component can have multiple tasks routes / render multiple tasks

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@LauraBeatris LauraBeatris self-assigned this Mar 21, 2025
Copy link

vercel bot commented Mar 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 24, 2025 11:56am

Copy link

changeset-bot bot commented Mar 21, 2025

🦋 Changeset detected

Latest commit: 4dde647

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 21 packages
Name Type
@clerk/clerk-js Patch
@clerk/types Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/agent-toolkit Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/elements Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/localizations Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/shared Patch
@clerk/testing Patch
@clerk/themes Patch
@clerk/vue Patch

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

@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from a20ae6a to e147a5f Compare March 21, 2025 22:24
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from e147a5f to a8a6526 Compare March 21, 2025 23:15
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from ff45aea to 336604a Compare March 23, 2025 17:06
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from 336604a to 8ffa4d7 Compare March 23, 2025 17:24
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from 8ffa4d7 to dfe49ef Compare March 23, 2025 17:39
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from dfe49ef to fd9a6f0 Compare March 23, 2025 17:42
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from fd9a6f0 to 2633070 Compare March 23, 2025 17:45
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from 2633070 to b8abacc Compare March 23, 2025 18:04
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from b8abacc to 25ca39e Compare March 23, 2025 18:53
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from 25ca39e to 9b5b6c0 Compare March 23, 2025 19:02
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from 9b5b6c0 to d50c67c Compare March 23, 2025 22:14
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from d50c67c to 7ced9f8 Compare March 23, 2025 22:39
@LauraBeatris LauraBeatris changed the title [wip] chore(clerk-js,types): Lazy load tasks based on environment settings chore(clerk-js,types): Lazy load tasks based on environment settings Mar 23, 2025
@LauraBeatris LauraBeatris force-pushed the laura/lazy-load-tasks-env branch from 7ced9f8 to 7246270 Compare March 23, 2025 22:44
@LauraBeatris LauraBeatris requested a review from a team March 23, 2025 22:45
@LauraBeatris LauraBeatris changed the title chore(clerk-js,types): Lazy load tasks based on environment settings chore(clerk-js,types): Display tasks based on environment settings Mar 23, 2025
@LauraBeatris LauraBeatris changed the title chore(clerk-js,types): Display tasks based on environment settings chore(clerk-js,types): Load tasks based on environment settings Mar 23, 2025
@LauraBeatris LauraBeatris marked this pull request as ready for review March 23, 2025 22:56
@LauraBeatris
Copy link
Member Author

Here are a few considerations when it comes to optimizations:

  • It's not possible to prefetch FAPI organization memberships ahead of navigating to a task. By the time the SignIn/SignUp component loads, the user hasn't had a session yet.
  • Lazy loading is being applied to the entire SessionTasks within SignIn/SignUp
    • I've experimented with lazy loading the inner routes to SessionTasks based on the environment settings, eg: the upcoming billing task wouldn't get downloaded if there's only a force org task, however, it'd require dealing with Suspense and possibility leading to a waterfall of loading -> it's a tradeoff tho, we can revisit this later

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

Successfully merging this pull request may close these issues.

2 participants