-
-
Notifications
You must be signed in to change notification settings - Fork 6
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.
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
Better browser support for updating urls #41
Changes from 3 commits
b0085aa
cdd8e85
e0443a7
71164ec
6eadb34
0b85421
49eb1ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { expect, test } from 'vitest' | ||
import { isBrowser } from '@/utilities/isBrowser' | ||
|
||
test('isBrowser returns true when environment is browser', () => { | ||
expect(isBrowser).toBe(true) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { expect, test } from 'vitest' | ||
import { isBrowser } from '@/utilities/isBrowser' | ||
|
||
test('isBrowser returns false when environment is node', () => { | ||
expect(isBrowser).toBe(false) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
// eslint-disable-next-line @typescript-eslint/prefer-optional-chain | ||
export const isBrowser: boolean = typeof window !== 'undefined' && typeof window.document !== 'undefined' | ||
pleek91 marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
|
||
import { expect, test, vi } from 'vitest' | ||
import { updateBrowserUrl } from '@/utilities/updateBrowserUrl' | ||
|
||
test('updates the window location', () => { | ||
vi.spyOn(window.location, 'assign').mockImplementation(() => {}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these |
||
|
||
updateBrowserUrl('http://example.com2/foo') | ||
|
||
expect(window.location.assign).toHaveBeenCalled() | ||
}) | ||
|
||
test('updates the window location using replace when options.replace is true', () => { | ||
vi.spyOn(window.location, 'replace').mockImplementation(() => {}) | ||
|
||
updateBrowserUrl('http://example.com/foo', { replace: true }) | ||
|
||
expect(window.location.replace).toHaveBeenCalled() | ||
}) | ||
|
||
test('updates the history', () => { | ||
vi.spyOn(history, 'pushState').mockImplementation(() => {}) | ||
|
||
updateBrowserUrl('/foo') | ||
|
||
expect(history.pushState).toHaveBeenCalled() | ||
}) | ||
|
||
test('updates the history using replaceState when options.replace is true', () => { | ||
vi.spyOn(history, 'replaceState').mockImplementation(() => {}) | ||
|
||
updateBrowserUrl('/foo', { replace: true }) | ||
|
||
expect(history.replaceState).toHaveBeenCalled() | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { expect, test } from 'vitest' | ||
import { isBrowser } from '@/utilities/isBrowser' | ||
import { updateBrowserUrl } from '@/utilities/updateBrowserUrl' | ||
|
||
test('does nothing when the window is not available', () => { | ||
updateBrowserUrl('http://example.com2/foo') | ||
|
||
expect(isBrowser).toBe(false) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { isBrowser } from '@/utilities/isBrowser' | ||
|
||
type UpdateBrowserUrlOptions = { | ||
replace?: boolean, | ||
} | ||
pleek91 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
export function updateBrowserUrl(url: string, options: UpdateBrowserUrlOptions = {}): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on a popstate event where the browser is telling us the user navigated outside of our router's control, would you imagine we call this function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if a popstate event is emitted the browser already has an updated url. So we wouldn't need to call this. |
||
if (!isBrowser) { | ||
return | ||
} | ||
|
||
if (isSameOrigin(url)) { | ||
return updateHistory(url, options) | ||
} | ||
|
||
return updateWindow(url, options) | ||
} | ||
|
||
function updateHistory(url: string, options: UpdateBrowserUrlOptions = {}): void { | ||
if (options.replace) { | ||
return history.replaceState({}, '', url) | ||
} | ||
|
||
history.pushState({}, '', url) | ||
} | ||
|
||
function updateWindow(url: string, options: UpdateBrowserUrlOptions = {}): void { | ||
if (options.replace) { | ||
return window.location.replace(url) | ||
} | ||
|
||
return window.location.assign(url) | ||
} | ||
|
||
function isSameOrigin(url: string): boolean { | ||
const { origin } = new URL(url, window.location.origin) | ||
|
||
return origin === window.location.origin | ||
} | ||
pleek91 marked this conversation as resolved.
Show resolved
Hide resolved
|
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggesting this as an alternative to the
globalExists
pattern. Thinking avoidingeval
is probably a good idea for a public repository.