fix(clerk-js): Append initial values query params after hash #1855
Conversation
🦋 Changeset detectedLatest commit: 2d72e68 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 |
desiprisg
force-pushed
the
initial_values_query_param_fix
branch
from
92809b0 to
3f9e29b
Compare
packages/clerk-js/src/core/clerk.ts
Outdated
| return this.buildUrlWithAuth( | ||
| appendUrlsAsQueryParams(appendAsQueryParams(signInOrUpUrl, options?.initialValues || {}), opts), | ||
| ); | ||
| return this.buildUrlWithAuth(appendAsQueryParams(signInOrUpUrl, { values: options?.initialValues, urls })); |
There was a problem hiding this comment.
Passing values and urls as separate paramaters here is a bit confusing to me, particularly since they're handled with a single search params abstraction on the other end
Can we strip the origin from urls before calling buildUrlWithAuth, so that we only need to pass in a single object?
There was a problem hiding this comment.
Added a refactor commit.
|
Please add a description to the PR to describe what you're changing and why. Thanks! |
| } | ||
| }); | ||
|
|
||
| return this.buildUrlWithAuth(appendAsQueryParams(signInOrUpUrl, { ...urls, ...options?.initialValues })); |
There was a problem hiding this comment.
❓Couldn't the stripSameOrigin logic be part of appendAsQueryParams ?
There was a problem hiding this comment.
Wasn't that the case before the refactor @colinclerk proposed? Please look at the first commit and lmk if that's what you mean.
There was a problem hiding this comment.
hmm, i see, yeap that covers it.
desiprisg
force-pushed
the
initial_values_query_param_fix
branch
from
0e91210 to
7b84319
Compare
desiprisg
force-pushed
the
initial_values_query_param_fix
branch
from
a5a4692 to
2d72e68
Compare
|
This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
When calling
Clerk.redirectToSignIn/Up(), the initial values query params were being added as normal query params, instead of being appended after the hash. This means that they were not being picked up in a hash routing environment. This PR addresses that issue and appends them the same way as the redirect urls.Checklist
npm testruns as expected.npm run buildruns as expected.Type of change
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-extensiongatsby-plugin-clerkbuild/tooling/chore