Skip to content

Conversation

@panteliselef
Copy link
Member

@panteliselef panteliselef commented Feb 4, 2025

Description

General Problem

Since the deprecation of setActive({ beforeEmit }), our UI components have been using Clerk.navigate in order to navigate when setActive({ redirectUrl }) is set. Instead our UI components should be using useRouter().navigate in every occasion since it is has some extra handy capabilities.

Specific use-case

This buggy behaviour is clearly visible, when you attempt to delete a user from UserProfile rendered in a modal. Upon deletion, setActive({redirectUrl}) was called, but when a soft navigation occurred (using framework router) the modal would not close, and the backdrop was clearly visible.

Solution

This solution in this PR fixes the above issues while maintaining setActive simple to use for developers. We also still discourage the explicit usage of setActive({beforeEmit})

Fixes the e2e test case that fails in #5071

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:

…Active and Clerk.navigate inside UI components
…Active

This way we do not need to change the public signature of setActive, or remove the deprecation of beforeEmit, since developers that consume the sdk should still use `redirectUrl`.
@vercel
Copy link

vercel bot commented Feb 4, 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 Feb 4, 2025 1:15pm

@changeset-bot
Copy link

changeset-bot bot commented Feb 4, 2025

🦋 Changeset detected

Latest commit: 5bc5a88

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

This PR includes changesets to release 22 packages
Name Type
@clerk/clerk-js Patch
@clerk/clerk-react Patch
@clerk/types Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/elements Patch
@clerk/nextjs Patch
@clerk/react-router Patch
@clerk/remix Patch
@clerk/tanstack-start Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/localizations Patch
@clerk/nuxt Patch
@clerk/shared Patch
@clerk/testing Patch
@clerk/themes Patch
@clerk/ui 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

@panteliselef panteliselef changed the title Elef/sdki 856 userprofile modal backdrop is still mounted after delete chore(clerk-js): Enable UI Components to use setActive with router.navigate Feb 4, 2025
@panteliselef
Copy link
Member Author

Currently linting is expected to fail, until all setActive({redirectUrl}) usages are updated within our UI components.

@panteliselef panteliselef requested a review from a team February 4, 2025 13:18
@panteliselef panteliselef self-assigned this Feb 4, 2025
@dstaley
Copy link
Member

dstaley commented Feb 4, 2025

Why was setActive({ beforeEmit }) deprecated originally? This feels like we're remaking that API, only more convoluted with the introduction of the scoped context. I'd love to understand why it was removed originally because as it's shown here I feel like setActive({ __internal_beforeEmit }) would be a cleaner alternative API.

Also, I wonder if there's a better expression of this idea that we could safely expose as a public API?

@panteliselef
Copy link
Member Author

Alternative solution #5092 got approval, so I will be closing this

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.

4 participants