Skip to content

fix(clerk-js): Set eTLD+1 as the domain for client_uat#3318

Merged
nikosdouvlis merged 8 commits intomainfrom
brk.fix/client-uat-domain
May 8, 2024
Merged

fix(clerk-js): Set eTLD+1 as the domain for client_uat#3318
nikosdouvlis merged 8 commits intomainfrom
brk.fix/client-uat-domain

Conversation

@brkalow
Copy link
Copy Markdown
Member

@brkalow brkalow commented May 3, 2024

Description

Set an explicit domain value for our clientUat cookie. This mirrors logic in FAPI. Without this, we were seeing duplicate __client_uat cookies, which causes issues when they get out of sync. I also removed an unused helper method, getAllETLDs(), that I stumbled across.

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:

@brkalow brkalow requested review from colinclerk and nikosdouvlis May 3, 2024 03:48
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 3, 2024

🦋 Changeset detected

Latest commit: b6f9e58

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

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@brkalow brkalow changed the title fix(clerk-js): Set eTLD+1 as the domain for client_uat and clerk_db_jwt fix(clerk-js): Set eTLD+1 as the domain for client_uat May 3, 2024
Comment on lines -46 to -57
export function getAllETLDs(hostname: string = window.location.hostname): string[] {
const parts = hostname.split('.');
const memo = [];
const domains = [];

for (let i = parts.length - 1; i > 0; i--) {
memo.unshift(parts[i]);
domains.push(memo.join('.'));
}

return domains;
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Stumbled on this, and turns out it's unused, so removed 🧹

@brkalow brkalow force-pushed the brk.fix/client-uat-domain branch from bdb0b58 to 0ba197e Compare May 3, 2024 04:11
Comment thread packages/clerk-js/src/utils/cookies/getCookieDomain.ts
@brkalow brkalow requested a review from a team May 7, 2024 18:54
Comment thread packages/clerk-js/src/utils/cookies/getCookieDomain.ts
@nikosdouvlis nikosdouvlis requested a review from LekoArts May 8, 2024 13:48
@nikosdouvlis
Copy link
Copy Markdown
Member

nikosdouvlis commented May 8, 2024

I've added unit tests and made 2 minor improvements:

  • handle common addresses like 0.0.0.0 that usually point to localhost
  • make use of the module-level cache value

Please take another look @brkalow @LekoArts @dimkl

const eTLDCookie = createCookieHandler('__clerk_test_etld');

export function getCookieDomain(hostname = window.location.hostname, cookieHandler = eTLDCookie) {
// only compute it once per session to avoid unnecessary cookie ops
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

👍

@nikosdouvlis nikosdouvlis merged commit 27d6126 into main May 8, 2024
@nikosdouvlis nikosdouvlis deleted the brk.fix/client-uat-domain branch May 8, 2024 14:50
@clerk-cookie clerk-cookie mentioned this pull request May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants