Skip to content

feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020

Merged
octoper merged 22 commits into
mainfrom
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged

feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

Conversation

@octoper

@octoperoctoper commented Nov 2, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR changes the default behaviour of the following

  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.

Checklist

  • npm test runs as expected.
  • npm run 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:

Packages affected

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

@octoper
octoper requested a review from a team as a code ownerNovember 2, 2023 21:08
@changeset-bot

changeset-botBot commented Nov 2, 2023•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 59ab8ae

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

This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch

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

@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch from 2e81ce1 to 91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper changed the title feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef reviewed Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts Outdated
Comment on lines +143 to +134
afterSignInUrl: '/',
afterSignUpUrl: '/',

@anagstefanagstefNov 3, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?

@octoperoctoperNov 7, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.

@dimkl
dimklforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch from 7aecc46 to eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch 2 times, most recently from 35db2db to 0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch 2 times, most recently from 42e46a1 to 320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis requested changes Nov 9, 2023
View reviewed changes

@nikosdouvlisnikosdouvlis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day

@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch 2 times, most recently from 74cc3f8 to 9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch 3 times, most recently from 5c30f34 to 15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef reviewed Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md Outdated
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts Outdated
@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch from af58073 to 021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis requested changes Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts Outdated
Comment on lines +133 to +134
afterSignInUrl: '/',
afterSignUpUrl: '/',

@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.

What we want to do instead, is to change the behavior of Clerk IF these are undefined

@octoperoctoperNov 16, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.

dimkl
dimkl reviewed Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts Outdated
@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch from bf04a09 to 2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch from 2c10bf3 to 59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper added this pull request to the merge queue Dec 6, 2023
Merged via the queue into main with commit d30ea1fDec 6, 2023
@octoper
octoper deleted the vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton branch December 6, 2023 09:53
@clerk-cookieclerk-cookie mentioned this pull request Dec 5, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes
+5 more reviewers
@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp by octoper · Pull Request #2020 · clerk/javascript · GitHub
Skip to content
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp - #2020
Merged
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Dec 6, 2023
Merged
feat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUp#2020
octoper merged 22 commits into
mainfrom 
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
Conversation
@octoper
@octoperoctoper
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

Contributor
Description
This PR changes the default behaviour of the following
  • Change the default behaviour of after afterSignOutUrl prop in <UserButton /> to be redirected to / instead of the Account Portal defined url.
  • Change the default behaviour of after afterSignIn and afterSignUp props to be redirected to / instead of the Account Portal defined url.
  • Use the default options for the Clerk singleton as right now they get overwritten from undefined values.
  • Added a utility function to @clerk/shared package, that receives an object and returns it back without the properties undefined value.
Checklist
  • npm test runs as expected.
  • npm run 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:
Packages affected
  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore
@octoper
octoper
requested a review
from a team
as a code ownerNovember 2, 2023 21:08
@changeset-bot
changeset-botBot
commented
Nov 2, 2023•

edited

Loading

Copy link


Copy Markdown

🦋 Changeset detected
Latest commit: 59ab8ae
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMajor
@clerk/sharedPatch
@clerk/chrome-extensionPatch
@clerk/clerk-expoPatch
@clerk/backendPatch
@clerk/fastifyPatch
@clerk/nextjsPatch
@clerk/clerk-reactPatch
@clerk/remixPatch
@clerk/clerk-sdk-nodePatch
gatsby-plugin-clerkPatch
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
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2e81ce1 to
91fe24dCompareNovember 2, 2023 21:08
@octoperoctoper
changed the title
feat(clerk-js): Change the default behavior of afterSignOutUrl, afterSignIn and afterSignUpfeat(clerk-js): Change the default behaviour of afterSignOutUrl, afterSignIn and afterSignUpNov 2, 2023
anagstef
anagstef
reviewed
Nov 3, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+143
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@anagstefanagstefNov 3, 2023

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more places in our codebase where we use displayConfig.afterSignInUrl and displayConfig.afterSignUpUrl. Are those also considered?
@octoperoctoperNov 7, 2023•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the changes I made the default values will be directly taken from the defaultOptions so the displayConfig.afterSignInUrl or displayConfig.afterSignUpUrl are never used.
@dimkl
dimklforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
7aecc46 to
eae95f2CompareNovember 6, 2023 11:29
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
35db2db to
0ffd8bfCompareNovember 7, 2023 11:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
42e46a1 to
320b770CompareNovember 8, 2023 11:05
nikosdouvlis
nikosdouvlis
requested changes
Nov 9, 2023
View reviewed changes
@nikosdouvlisnikosdouvlis
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octoper You will need to apply the changes to the ui-retheme dir. I will share more details within the day
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
2 times, most recently
from
74cc3f8 to
9997c00CompareNovember 10, 2023 10:02
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
3 times, most recently
from
5c30f34 to
15b3a32CompareNovember 15, 2023 08:47
anagstef
anagstef
reviewed
Nov 15, 2023
View reviewed changes
Comment thread.changeset/spotty-boxes-do.md
Outdated

Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
af58073 to
021108bCompareNovember 15, 2023 14:46
nikosdouvlis
nikosdouvlis
requested changes
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/core/clerk.ts
Outdated


Comment on lines

+133
 to 
+134

 afterSignInUrl: '/',
 afterSignUpUrl: '/',
@nikosdouvlisnikosdouvlisNov 16, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to take another look, but my instinct tells me that this is not the direction we want to go as this will cause these to have actual values so, by default, the URLs will include the query params.
What we want to do instead, is to change the behavior of Clerk IF these are undefined
@octoperoctoperNov 16, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes to only fallback to / and the values will not be included to the query params.
dimkl
dimkl
reviewed
Nov 16, 2023
View reviewed changes
Comment threadpackages/clerk-js/src/utils/pagesToOffset.ts
Comment threadpackages/clerk-js/src/core/clerk.redirects.test.ts
Outdated

@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
bf04a09 to
2c10bf3CompareDecember 4, 2023 15:34
@octoper
feat(clerk-js): Change the default path of afterSignOutUrl
5801a06
@octoper
feat(clerk-js,shared): Use default options if nothing passed when loa…
ac94a63
…ding Clerk client
@octoper
chore(repo): Update NextJS playground
bb62c3c
@octoper
chore(clerk-js): Fix typo
3255cf4
@octoper
chore(repo): Added Changeset
b7775b3
@octoper
chore(shared): Fixed types for getNonUndefinedValues and added tests
0d4204e
@octoper
fix(remix): Utility function getEnvVariable return undefined instead …
6e4ef19
…of empty string
@octoper
fix(clerk-js): Give empty object when options are undefined
c856132
@octoper
test(shared): Fixed test for getNonUndefinedValues utility function
936fdf2
@octoper
fix(clerk-js): Apply changes to ui.retheme
060401b
@octoper
fix(clerk-js): Add missing import
016da40
@octoper
fix(clerk-js): Add missing import
6c23963
@octoper
test(clerk-js): Update Clerk redirect tests
2260eb9
@octoper
chore(repo): Update Changeset
c58c6f0
@octoper
chore(remix): Update return type comment
e34efdd
@octoper
fix(clerk-js): Use the defaults for afterSignInUrl and afterSignUpUrl…
09ae7be
… as fallbacks
@octoper
chore(remix): Update return type of getEnvVariable utility function
58aba6a
@octoper
test(clerk-js): Update test for SignInStart component
4fb3201
@octoper
test(clerk-js): Update test for SignInStart component in ui.retheme
b47a5c0
@octoper
feat(clerk-js): Removed display config values
e8ebc75
@octoper
chore(repo): Apply Prettier formating
74b0d29
@octoper
chore(shared): Remove unused import from tests
59ab8ae
@octoper
octoperforce-pushed
the
vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton
branch
from
2c10bf3 to
59ab8aeCompareDecember 6, 2023 09:10
@octoper
octoper
added this pull request to the merge queue
Dec 6, 2023

Merged
via the queue into
main
with commit d30ea1fDec 6, 2023
@octoper
octoper
deleted the

vaggelis/sdk-831-change-the-default-behavior-of-aftersignouturl-of-userbutton

branch
December 6, 2023 09:53
@clerk-cookieclerk-cookie
mentioned this pull request
Dec 5, 2023
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@nikosdouvlisnikosdouvlisnikosdouvlis approved these changes

+5 more reviewers

@anagstefanagstefanagstef approved these changes
@SokratisVidrosSokratisVidrosSokratisVidros approved these changes
@dimkldimkldimkl approved these changes
@panteliselefpanteliselefpanteliselef approved these changes
@LekoArtsLekoArtsLekoArts left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@octoperoctoper

Labels

clerk-jsplaygroundremix

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

8 participants

@octoper@dimkl@SokratisVidros@nikosdouvlis@anagstef@LekoArts@panteliselef@clerk-cookie