') + ')', '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('^' + ".*" + ', '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" + ', '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('^' + ".*" + ', '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); } })(); })(); fix(clerk-js): Remove internal useCore hooks by LekoArts · Pull Request #2111 · clerk/javascript · GitHub
Skip to content

fix(clerk-js): Remove internal useCore hooks - #2111

Merged
panteliselef merged 8 commits into
mainfrom
lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our
Nov 28, 2023
Merged

fix(clerk-js): Remove internal useCore hooks#2111
panteliselef merged 8 commits into
mainfrom
lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our

Conversation

@LekoArts

@LekoArtsLekoArts commented Nov 10, 2023•
edited by panteliselef
Loading

Copy link
Copy Markdown
Contributor

Description

This PR:

  • Merges useCoreSession (clerk-js) and useSession(clerk-react) and moves it to @clerk/shared/react as useSession.
  • Merges useCoreUser (clerk-js) and useUser(clerk-react) and moves it to @clerk/shared/react as useUser.
  • Merges useCoreClerk (clerk-js) and useClerk(clerk-react) and moves it to @clerk/shared/react as useClerk.
  • Renames useCoreOrganization(List) hooks to useOrganization(List) inside clerk-js.
  • Improves parity between the hooks we are exposing and the hooks we use internally.

The changes for the clerk-js package are applied inside the ui.retheme and the ui directory

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

@changeset-bot

changeset-botBot commented Nov 10, 2023•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ee24072

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

This PR includes changesets to release 11 packages
NameType
@clerk/clerk-jsMinor
@clerk/sharedMinor
@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

@panteliselef
panteliselefforce-pushed the lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our branch 2 times, most recently from 3a08302 to 37e8513CompareNovember 12, 2023 17:18
@panteliselef
panteliselef marked this pull request as ready for review November 12, 2023 18:03
@panteliselef
panteliselef requested a review from a team as a code ownerNovember 12, 2023 18:03
@panteliselef
panteliselefforce-pushed the lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our branch from 37e8513 to e9d23e0CompareNovember 13, 2023 12:21
@panteliselef
panteliselefforce-pushed the lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our branch 4 times, most recently from dde7159 to 9ad4505CompareNovember 23, 2023 09:16
dimkl
dimkl approved these changes Nov 23, 2023
View reviewed changes

@dimkldimkl left a comment

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.

❓ Instead of replacing user. with user?. in the components where useUser() hook is used, shouldn't we add a guard if (!user) return null;?
❓ Instead of adding the if (!user) return null; guard could we move that up the hierarchy to avoid repeating this guard to all the components of eg UserProfile / OrganizationSwitcher? (If so, let's verify that all components that need user have the guard and add a separate PR to make that improvement)

Comment threadpackages/clerk-js/src/ui.retheme/components/OrganizationList/UserMembershipList.tsx

if (!user) {
return null;
}

@dimkldimklNov 23, 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.

❓ is this a fix to avoid rendering PersonalAccountPreview when user is not available or the useCoreUser() has different behaviour from the useUser() ?

@panteliselefpanteliselefNov 23, 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.

OrganizationList is wrapped in withCoreUserGuard so we are safe. It doesn't really matter if we do user?.something or return null.

Here we did that because it was helping with the deconstruction.

Comment threadpackages/clerk-js/src/ui.retheme/components/OrganizationProfile/ActionConfirmationPage.tsx
const { user } = useUser();

if (!organization) {
if (!organization || !user) {

@dimkldimklNov 23, 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.

same as above

Comment threadpackages/clerk-js/src/ui.retheme/components/OrganizationProfile/ActiveMembersList.tsx
const { localizeCustomRole } = useLocalizeCustomRoles();
const card = useCardState();
const user = useCoreUser();
const { user } = useUser();

@dimkldimklNov 23, 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.

❓ Don't we need if (!user) return null; guard here?

@panteliselefpanteliselefNov 23, 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.

OrganizationProfile is wrapped in withCoreUserGuard so we are safe.

Comment threadpackages/react/src/contexts/ClientContext.tsx Outdated

@dimkldimklNov 23, 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.

🔧 remove this empty file.

@panteliselef
panteliselefforce-pushed the lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our branch 2 times, most recently from b46658d to 41fed9eCompareNovember 28, 2023 08:59
@panteliselef
panteliselefforce-pushed the lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our branch from dc7b7a4 to ee24072CompareNovember 28, 2023 18:15
@panteliselef
panteliselef added this pull request to the merge queue Nov 28, 2023
Merged via the queue into main with commit 4b8bedcNov 28, 2023
@panteliselef
panteliselef deleted the lekoarts/sdk-660-remove-usecorex-hooks-and-instead-call-usex-directly-for-our branch November 28, 2023 21:42
@clerk-cookieclerk-cookie mentioned this pull request Nov 28, 2023
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brkalowbrkalowbrkalow approved these changes
+2 more reviewers
@panteliselefpanteliselefpanteliselef left review comments
@dimkldimkldimkl approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@LekoArtsLekoArts

@panteliselefpanteliselef

Labels

clerk-jsreact

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@LekoArts@brkalow@dimkl@panteliselef@clerk-cookie