[SDK] Fix SiteLink and SiteEmbed stripping URL hash fragments#8663
Merged
joaquim-verges merged 1 commit intomainfrom Feb 5, 2026
Merged
[SDK] Fix SiteLink and SiteEmbed stripping URL hash fragments#8663joaquim-verges merged 1 commit intomainfrom
joaquim-verges merged 1 commit intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Contributor
WalkthroughgetUrlToken now reads auth params from both the query string and the hash fragment, removes those tokens while preserving the original hash path when updating history, and tests were added to verify hash-fragment preservation and parsing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/thirdweb/src/react/web/ui/SiteLink.tsx`:
- Around line 84-96: SiteLink.tsx currently appends auth params into the URL
hash for hash-routed URLs which makes them invisible to the existing
getUrlToken() logic (used by autoConnectCore) that only reads
window.location.search; fix by either updating getUrlToken() to also parse URL
hash parameters (split window.location.hash to extract a query string and merge
those params with search) or change SiteLink.tsx to place auth params back into
the search portion for non-history routers; reference getUrlToken() and
autoConnectCore.ts for the parser update, or modify SiteLink.tsx where finalUrl
is constructed to keep params in window.location.search for compatibility.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8663 +/- ##
==========================================
+ Coverage 52.72% 52.73% +0.01%
==========================================
Files 934 934
Lines 62945 62975 +30
Branches 4117 4137 +20
==========================================
+ Hits 33185 33210 +25
- Misses 29660 29665 +5
Partials 100 100
🚀 New features to boost your workflow:
|
eabdelmoneim
force-pushed
the
fix/sitelink-hash-fragment-preservation
branch
from
78389ef to
57b0912
Compare
SiteLink/SiteEmbed already preserve hash fragments via the URL API (searchParams.set keeps the hash at end). The real bug was in getUrlToken() which dropped window.location.hash when cleaning up auth params via pushState. Also updated getUrlToken() to parse auth params from inside the hash fragment as a fallback, supporting hash-routed apps where params may appear after #/route?params. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
eabdelmoneim
force-pushed
the
fix/sitelink-hash-fragment-preservation
branch
from
57b0912 to
105db79
Compare
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+37
to
+66
| hashParams?.get("walletId") ?? | ||
| undefined) as WalletId | undefined; | ||
| const authResultString = | ||
| params.get("authResult") ?? hashParams?.get("authResult") ?? undefined; | ||
| const authProvider = (params.get("authProvider") ?? | ||
| hashParams?.get("authProvider") ?? | ||
| undefined) as AuthOption | undefined; | ||
| const authCookie = (params.get("authCookie") ?? | ||
| hashParams?.get("authCookie") ?? | ||
| undefined) as string | undefined; | ||
| const authFlow = (params.get("authFlow") ?? | ||
| hashParams?.get("authFlow") ?? | ||
| undefined) as "connect" | "link" | undefined; | ||
|
|
||
| if ((authCookie || authResultString) && walletId) { | ||
| const authResult = (() => { | ||
| if (authResultString) { | ||
| params.delete("authResult"); | ||
| hashParams?.delete("authResult"); | ||
| return JSON.parse(decodeURIComponent(authResultString)); | ||
| } | ||
| })(); | ||
| params.delete("walletId"); | ||
| params.delete("authProvider"); | ||
| params.delete("authCookie"); | ||
| params.delete("authFlow"); | ||
| hashParams?.delete("walletId"); | ||
| hashParams?.delete("authProvider"); | ||
| hashParams?.delete("authCookie"); | ||
| hashParams?.delete("authFlow"); |
Contributor
There was a problem hiding this comment.
Missing params.delete("walletId") in the cleanup logic. The code deletes walletId from hashParams (line 63) but not from the standard params. If walletId is in the standard query string, it will remain in the reconstructed URL after authentication.
Impact: walletId will not be cleaned up from the query string, potentially causing it to persist in the URL after auth completion.
Fix: Add params.delete("walletId"); before line 63:
params.delete("authProvider");
params.delete("authCookie");
params.delete("authFlow");
params.delete("walletId"); // Add this line
hashParams?.delete("walletId");
Suggested change
| hashParams?.get("walletId") ?? | |
| undefined) as WalletId | undefined; | |
| const authResultString = | |
| params.get("authResult") ?? hashParams?.get("authResult") ?? undefined; | |
| const authProvider = (params.get("authProvider") ?? | |
| hashParams?.get("authProvider") ?? | |
| undefined) as AuthOption | undefined; | |
| const authCookie = (params.get("authCookie") ?? | |
| hashParams?.get("authCookie") ?? | |
| undefined) as string | undefined; | |
| const authFlow = (params.get("authFlow") ?? | |
| hashParams?.get("authFlow") ?? | |
| undefined) as "connect" | "link" | undefined; | |
| if ((authCookie || authResultString) && walletId) { | |
| const authResult = (() => { | |
| if (authResultString) { | |
| params.delete("authResult"); | |
| hashParams?.delete("authResult"); | |
| return JSON.parse(decodeURIComponent(authResultString)); | |
| } | |
| })(); | |
| params.delete("walletId"); | |
| params.delete("authProvider"); | |
| params.delete("authCookie"); | |
| params.delete("authFlow"); | |
| hashParams?.delete("walletId"); | |
| hashParams?.delete("authProvider"); | |
| hashParams?.delete("authCookie"); | |
| hashParams?.delete("authFlow"); | |
| hashParams?.get("walletId") ?? | |
| undefined) as WalletId | undefined; | |
| const authResultString = | |
| params.get("authResult") ?? hashParams?.get("authResult") ?? undefined; | |
| const authProvider = (params.get("authProvider") ?? | |
| hashParams?.get("authProvider") ?? | |
| undefined) as AuthOption | undefined; | |
| const authCookie = (params.get("authCookie") ?? | |
| hashParams?.get("authCookie") ?? | |
| undefined) as string | undefined; | |
| const authFlow = (params.get("authFlow") ?? | |
| hashParams?.get("authFlow") ?? | |
| undefined) as "connect" | "link" | undefined; | |
| if ((authCookie || authResultString) && walletId) { | |
| const authResult = (() => { | |
| if (authResultString) { | |
| params.delete("authResult"); | |
| hashParams?.delete("authResult"); | |
| return JSON.parse(decodeURIComponent(authResultString)); | |
| } | |
| })(); | |
| params.delete("authProvider"); | |
| params.delete("authCookie"); | |
| params.delete("authFlow"); | |
| params.delete("walletId"); | |
| hashParams?.delete("walletId"); | |
| hashParams?.delete("authProvider"); | |
| hashParams?.delete("authCookie"); | |
| hashParams?.delete("authFlow"); |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://linear.app/thirdweb/issue/PRO-186/bug-in-sitelink-and-siteembed-for-urls
SiteLink/SiteEmbed now append wallet params inside the hash fragment for hash-routed URLs (e.g. https://snapshot.org/#/s:wampei.eth) instead of placing them in the standard query string where they get separated from the hash router. Also fix getUrlToken dropping window.location.hash when cleaning up auth params via pushState.
PR-Codex overview
This PR focuses on enhancing the handling of URL parameters in hash-routed applications, ensuring that both query parameters and hash fragments are processed correctly. It includes tests to validate the preservation of hash fragments and modifies the
getUrlTokenfunction to accommodate these changes.Detailed summary
SiteLink.test.tsxto verify preservation of hash fragments in URLs.get-url-token.test.tsxto:authFlowandauthProviderreturnundefinedinstead ofnull.get-url-token.tsto:pushState.Summary by CodeRabbit
Bug Fixes
Tests