fix(clerk-js): Add push/replace props & replace state when merging fragment into url

[desiprisg]

Type of change

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

Packages affected

• @clerk/clerk-js
• @clerk/clerk-react
• @clerk/nextjs
• @clerk/remix
• @clerk/types
• @clerk/themes
• @clerk/localizations
• @clerk/clerk-expo
• @clerk/backend
• @clerk/clerk-sdk-node
• @clerk/shared
• @clerk/fastify
• @clerk/chrome-extension
• gatsby-plugin-clerk
• build/tooling/chore

Description

• npm test runs as expected.
• npm run build runs as expected.

When using path routing and clicking on a sign-in/up link, we get rerouted to a hash based url which automatically gets converted to a path based url (that's happening inside PathRouter.tsx). After that, when the back button is pressed, we arrive at the hash-based url that got converted and not at the actual previous page we were on.

Solution: Add a routerReplace prop to <ClerkProvider/> and use that for merging the fragment into the url.

Edit: This PR also includes the new routerPush prop that replaces the navigate prop for v5.

Before:

Screen.Recording.2023-05-26.at.03.39.21.mov

After:

Screen.Recording.2023-05-26.at.04.38.49.mov

[changeset-bot]

🦋 Changeset detected

Latest commit: 7f1a039

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

This PR includes changesets to release 11 packages

Name	Type
@clerk/nextjs	Major
gatsby-plugin-clerk	Major
@clerk/clerk-js	Major
@clerk/remix	Major
@clerk/types	Major
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/clerk-react	Patch
@clerk/clerk-sdk-node	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

[jit-ci]

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

[colinclerk]

This is a good change, but there's a reasonable chance it will lead to errors. The issue is we can't predict if we're setting an appropriate state variable for the framework's router.

We probably won't see an issue unless we:

• Call window.history.replaceState
• Complete the first step of sign-up
• Then click the back button, to return to the start step of the sign-up

The back button will fire the Window popstate event, which the framework's router will be listening to so it can update it's own internal state. At that point, it's possible the framework can get confused by our manual entry via window.history.replaceState and throw an error.

Passing window.history.state is a good countermeasure and I suspect it will work for navigations to the root of the component, but I'd still worry about navigations to the non-root (for example, /sign-in#/choose for afterSignOutOne).

This issue is why we don't call window.history.pushState directly, but instead use the framework's wrapper of pushState (e.g. router.push in Next.js).

The safer approach for this fix is to use the framework's wrapper around window.history.replaceState, too (e.g. router.replace in Next.js)

I've been thinking we should accept both push and replace variants to <ClerkProvider> in v5, but if there's urgency in v4 we can find a way.

[desiprisg]

@colinclerk We'll do some more tests to make sure this works, if we're going to go ahead with it. I didn't think accepting a replace variant was an option right now as well.

[BRKalow]

Implementation looks good, seems like CI is failing though.

[dimkl]

Please also add the changes to the Upgrade v4 -> v5 Guide [DRAFT].

[nikosdouvlis]

@desiprisg let's please add at least one e2e test to cover this.