Skip to content

fix(clerk-js): Handle removing origin in non-browser environments #6737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 9, 2025
Merged

fix(clerk-js): Handle removing origin in non-browser environments #6737

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
686f2f0
fix(clerk-js): Handle removing origin in non-browser environments
bratsos Sep 9, 2025
1f622c0
chore: Bump bundlewatch size
bratsos Sep 9, 2025
49f8a60
chore: add changesets
bratsos Sep 9, 2025
ff7748e
fixup! fix(clerk-js): Handle removing origin in non-browser environments
bratsos Sep 9, 2025
a78ffd7
Merge branch 'main' into alexbratsos/user-3415-clerkclerk-expo-cannot…
bratsos Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/clerk-js/src/utils/__tests__/url.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
relativeToAbsoluteUrl,
requiresUserInput,
sanitizeHref,
stripOrigin,
trimLeadingSlash,
trimTrailingSlash,
} from '../url';
Expand Down Expand Up @@ -641,3 +642,32 @@ describe('relativeToAbsoluteUrl', () => {
expect(relativeToAbsoluteUrl(relative, origin)).toEqual(new URL(expected));
});
});

describe('stripOrigin(url)', () => {
it('should strip origin when window.location is available', () => {
const originalLocation = window.location;
Object.defineProperty(window, 'location', {
value: { origin: 'https://example.com' },
writable: true,
});

expect(stripOrigin('https://example.com/test?param=1')).toBe('/test?param=1');
expect(stripOrigin('/test')).toBe('/test');

Object.defineProperty(window, 'location', { value: originalLocation });
});

it('should handle undefined window.location gracefully', () => {
const originalLocation = window.location;
Object.defineProperty(window, 'location', {
value: undefined,
writable: true,
});

expect(() => stripOrigin('/test')).not.toThrow();
expect(stripOrigin('/test')).toBe('/test');
expect(stripOrigin('https://example.com/test')).toBe('https://example.com/test');

Object.defineProperty(window, 'location', { value: originalLocation });
});
});
6 changes: 6 additions & 0 deletions packages/clerk-js/src/utils/url.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ export function toURL(url: string | URL): URL {
* @returns {string} Returns the URL href without the origin
*/
export function stripOrigin(url: URL | string): string {
// In non-browser environments `window.location.origin` might not be available
// if not polyfilled, so we can't construct a URL object with the `url` string
if (typeof window.location === 'undefined' && typeof url === 'string') {
Copy link
Member

Choose a reason for hiding this comment

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

Sorry if Im missing something obvious, but would this be safer?

Suggested change
if (typeof window.location === 'undefined' && typeof url === 'string') {
if ((typeof window === 'undefined' || !window.location) && typeof url === 'string') {

Copy link
Contributor Author

@bratsos bratsos Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is intentional as we're expecting that at least a window global object will be somehow declared. If it's not I prefer to get an error here, plus we'll have many other places that would break as well. WDYT?

Edit: Happy to make this more defensive though if we think it's valuable

Copy link
Member

Choose a reason for hiding this comment

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

window is defined in react-native 😬

See my comment here https://github.com/clerk/javascript/pull/4095/files#r1742755342

return url;
}
Copy link
Member

Choose a reason for hiding this comment

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

❓ should we instead handle this in toURL() where the direct window.location.origin usage is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my initial thought, but I think stripOrigin is better suited for making sure origin is accessible, and as it accepts both URL and string, essentially the code looks like this:

  • if you somehow managed to create a URL object, I'll pass it down to toURL
  • if you passed me a string, and I cannot safely detect origin to remove it, I'll just return the string

Happy to change this if you think moving it to toURL is easier to understand though.

Copy link
Member

Choose a reason for hiding this comment

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

@bratsos I don't think this covers the case of passing a fully qualified URL as a string, e.g.

stripOrigin("https://clerk.com/blog") // -> /blog

In this case, I would still expect it to strip the origin. toURL() only uses window.location.origin as the fallback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case, if location is available (essentially in all browser environments) this will still work. It won't work if we detect that location is no available, so we're in an exotic runtime that isn't polyfilled.

The thing is that toURL uses URL directly, and even though it's polyfilled in react native (source), the implementation is non-standard - and for example .origin which is accessed by stripOrigin is not even implemented. So we cannot really assume that if URL exists, new URL('https://clerk.com/blog', undefined) would work as expected. I think it's too late if we reach to toURL when we detect that something is wrong in the runtime APIs.

An even better solution would probably be to try and detect if the required APIs are available, and work as expected, or detect the runtime and decide what to do based on that, but I think that's an overkill as in most cases you're either in a browser/polyfilled* environment (so window.location and URL exist and we can safely use them), or not, so in that case we cannot do much on a plain string, unless we want to implement a pathname parser in-house.

*: If you polyfilled the environment in a weird way this can also throw us off.

WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense! Maybe we can adjust the JSDoc comment here to make it explicit that origin won't be stripped in certain non-browser environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure: ff7748e


url = toURL(url);
return url.href.replace(url.origin, '');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Fix: guard must not access window.location when window is undefined; also avoid toURL() (it dereferences window.location.origin).

Current check typeof window.location === 'undefined' will throw in SSR/Node where window is not defined. Additionally, the subsequent toURL(url) uses window.location.origin and can still throw in non-browser contexts or when url is a URL instance. Harden the function and remove the dependency on toURL() here.

Apply:

 export function stripOrigin(url: URL | string): string {
-  // In non-browser environments `window.location.origin` might not be available
-  // if not polyfilled, so we can't construct a URL object with the `url` string
-  if (typeof window.location === 'undefined' && typeof url === 'string') {
-    return url;
-  }
-
-  url = toURL(url);
-  return url.href.replace(url.origin, '');
+  // In non-browser environments `window` or `window.location` might not exist.
+  const hasWindowOrigin =
+    typeof window !== 'undefined' && !!window.location && typeof window.location.origin === 'string';
+  if (!hasWindowOrigin) {
+    // Without a reliable current origin, return input unchanged.
+    return typeof url === 'string' ? url : url.href;
+  }
+  const u = typeof url === 'string' ? new URL(url, window.location.origin) : url;
+  return u.href.replace(u.origin, '');
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function stripOrigin(url: URL | string): string {
// In non-browser environments `window.location.origin` might not be available
// if not polyfilled, so we can't construct a URL object with the `url` string
if (typeof window.location === 'undefined' && typeof url === 'string') {
return url;
}
url = toURL(url);
return url.href.replace(url.origin, '');
}
export function stripOrigin(url: URL | string): string {
// In non-browser environments `window` or `window.location` might not exist.
const hasWindowOrigin =
typeof window !== 'undefined' && !!window.location && typeof window.location.origin === 'string';
if (!hasWindowOrigin) {
// Without a reliable current origin, return input unchanged.
return typeof url === 'string' ? url : url.href;
}
const u = typeof url === 'string' ? new URL(url, window.location.origin) : url;
return u.href.replace(u.origin, '');
}
🤖 Prompt for AI Agents 
In packages/clerk-js/src/utils/url.ts around lines 176-185, the current guard
uses typeof window.location which throws when window is undefined and then calls
toURL() which itself accesses window.location.origin; change the logic to first
check typeof window !== 'undefined' before touching window at all, avoid calling
toURL(), and handle both URL and string inputs explicitly: if the input is a URL
instance use it directly; if it's a string and window is undefined simply return
the string unchanged; otherwise (in a browser or when window exists) construct a
URL safely using window.location.origin as the base (or a fallback base like
'http://localhost' if needed) and return href with origin stripped; ensure no
access to window.location happens unless typeof window !== 'undefined'.

Expand Down
Loading