fix(browser/react/vue): pass url.search to hasAuthParamsInUrl instead of falling back to window.location.search - #1
Open
Sdnsoumy wants to merge 2 commits into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
… of falling back to window.location.search hasAuthParams(url, afterSignInUrl) accepted a URL argument but called hasAuthParamsInUrl() with no argument, silently ignoring url.search and always checking window.location.search instead. This makes the helpers untestable in isolation and incorrect whenever the provided URL differs from the current window location. Fix all call sites to pass url.search explicitly. Also correct the JSDoc on hasAuthParamsInUrl: the function only checks for 'code', not both 'code' and 'session_state' as the old description claimed.
Copy link
Copy Markdown
There was a problem hiding this comment.
Pull request overview
Fixes browser auth-param detection to use the caller-provided URL consistently (instead of implicitly reading window.location.search), improving correctness in non-window scenarios (tests, navigation, SSR hydration) and aligning documentation with actual behavior.
Changes:
- Pass url.search explicitly to hasAuthParamsInUrl(url.search) from the React hook and Vue provider call sites.
- Update hasAuthParamsInUrl JSDoc to reflect that it checks for code (not code + session_state).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/vue/src/providers/ThunderIDProvider.ts | Passes url.search to hasAuthParamsInUrl to ensure correct URL evaluation. |
| packages/react/src/hooks/useBrowserUrl.ts | Passes url.search to hasAuthParamsInUrl to avoid falling back to window.location.search. |
| packages/browser/src/utils/hasAuthParamsInUrl.ts | Updates JSDoc to match the function’s actual detection logic (code). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment thread
packages/react/src/hooks/useBrowserUrl.ts
Comment thread
packages/browser/src/utils/hasAuthParamsInUrl.ts
Outdated
…wserUrl - Clarify hasAuthParamsInUrl @PARAM docs: function expects a raw URL.search string (with leading ? or &), not a URLSearchParams.toString() result - Support relative afterSignInUrl in hasAuthParams by resolving against url.origin via new URL(afterSignInUrl, url.origin) - Update JSDoc example to note both absolute and relative forms are accepted
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
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.
Purpose
hasAuthParams(url, afterSignInUrl) accepts a URL argument but both the React hook (useBrowserUrl) and the Vue provider (ThunderIDProvider) called hasAuthParamsInUrl() with no argument, silently ignoring url.search and always falling back to window.location.search.
This makes the helpers untestable in isolation (you cannot pass an arbitrary URL) and would produce incorrect results in any scenario where the caller's url differs from the current window location (e.g. during navigation events, SSR hydration, or unit tests).
Also fixes a JSDoc mismatch on hasAuthParamsInUrl: the description claimed it checks for both code and session_state, but the implementation only checks for code.
Approach
No behaviour change in production (both callers already pass the current window URL), but the helpers are now correct by construction and fully testable.
Related Issues