Skip to content

Commit

Permalink
Support passing a relative string to pushState/replaceState (#58438)
Browse files Browse the repository at this point in the history
Follow-up to #58335.

Fixes
#48110 (reply in thread)

As reported in the discussion passing a origin-relative string didn't
work as `new URL` will throw an error. This ensures the `origin`
parameter is provided to `new URL`.
Added tests for the string behavior.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Zack Tanner <[email protected]>
  • Loading branch information
3 people authored Nov 16, 2023
1 parent 4ba64e2 commit 97ba910
Show file tree
Hide file tree
Showing 5 changed files with 330 additions and 5 deletions.
15 changes: 10 additions & 5 deletions packages/next/src/client/components/app-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ const originalReplaceState =
: null

function copyNextJsInternalHistoryState(data: any) {
if (data == null) data = {}
const currentState = window.history.state
const __NA = currentState?.__NA
if (__NA) {
Expand All @@ -242,6 +243,8 @@ function copyNextJsInternalHistoryState(data: any) {
if (__PRIVATE_NEXTJS_INTERNALS_TREE) {
data.__PRIVATE_NEXTJS_INTERNALS_TREE = __PRIVATE_NEXTJS_INTERNALS_TREE
}

return data
}

/**
Expand Down Expand Up @@ -314,7 +317,7 @@ function Router({
) {
return
}
const url = new URL(addBasePath(href), location.href)
const url = new URL(addBasePath(href), window.location.href)
// External urls can't be prefetched in the same way.
if (isExternalURL(url)) {
return
Expand Down Expand Up @@ -404,8 +407,9 @@ function Router({
if (
!event.persisted ||
!window.history.state?.__PRIVATE_NEXTJS_INTERNALS_TREE
)
) {
return
}

dispatch({
type: ACTION_RESTORE,
Expand Down Expand Up @@ -456,10 +460,11 @@ function Router({
const applyUrlFromHistoryPushReplace = (
url: string | URL | null | undefined
) => {
const href = window.location.href
startTransition(() => {
dispatch({
type: ACTION_RESTORE,
url: new URL(url ?? window.location.href),
url: new URL(url ?? href, href),
tree: window.history.state.__PRIVATE_NEXTJS_INTERNALS_TREE,
})
})
Expand All @@ -476,7 +481,7 @@ function Router({
_unused: string,
url?: string | URL | null
): void {
copyNextJsInternalHistoryState(data)
data = copyNextJsInternalHistoryState(data)

applyUrlFromHistoryPushReplace(url)

Expand All @@ -494,7 +499,7 @@ function Router({
_unused: string,
url?: string | URL | null
): void {
copyNextJsInternalHistoryState(data)
data = copyNextJsInternalHistoryState(data)

if (url) {
applyUrlFromHistoryPushReplace(url)
Expand Down
10 changes: 10 additions & 0 deletions test/e2e/app-dir/shallow-routing/app/(shallow)/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ export default function ShallowLayout({ children }) {
To PushState new pathname
</Link>
</div>
<div>
<Link href="/pushstate-string-url" id="to-pushstate-string-url">
To PushState String Url
</Link>
</div>
<div>
<Link href="/replacestate-data" id="to-replacestate-data">
To ReplaceState Data
Expand All @@ -74,6 +79,11 @@ export default function ShallowLayout({ children }) {
To ReplaceState new pathname
</Link>
</div>
<div>
<Link href="/replacestate-string-url" id="to-replacestate-string-url">
To ReplaceState String Url
</Link>
</div>
</div>
{children}
</>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'use client'
import { useSearchParams } from 'next/navigation'

export default function Page() {
const searchParams = useSearchParams()
return (
<>
<h1 id="pushstate-string-url">PushState String Url</h1>
<pre id="my-data">{searchParams.get('query')}</pre>
<button
onClick={() => {
const previousQuery = new URL(window.location.href).searchParams.get(
'query'
)
const url = `?query=${
previousQuery ? previousQuery + '-added' : 'foo'
}`

window.history.pushState({}, '', url)
}}
id="push-string-url"
>
Push searchParam using string url
</button>

<button
onClick={() => {
const previousQuery = new URL(window.location.href).searchParams.get(
'query'
)
const url = `?query=${
previousQuery ? previousQuery + '-added' : 'foo'
}`

window.history.replaceState(null, '', url)
}}
id="push-string-url-null"
>
Push searchParam with null data param
</button>

<button
onClick={() => {
const previousQuery = new URL(window.location.href).searchParams.get(
'query'
)
const url = `?query=${
previousQuery ? previousQuery + '-added' : 'foo'
}`

window.history.replaceState(undefined, '', url)
}}
id="push-string-url-undefined"
>
Push searchParam with undefined data param
</button>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
'use client'
import { useSearchParams } from 'next/navigation'

export default function Page() {
const searchParams = useSearchParams()
return (
<>
<h1 id="replacestate-string-url">ReplaceState String Url</h1>
<pre id="my-data">{searchParams.get('query')}</pre>
<button
onClick={() => {
const previousQuery = new URL(window.location.href).searchParams.get(
'query'
)
const url = `?query=${
previousQuery ? previousQuery + '-added' : 'foo'
}`

window.history.replaceState({}, '', url)
}}
id="replace-string-url"
>
Replace searchParam using string url
</button>

<button
onClick={() => {
const previousQuery = new URL(window.location.href).searchParams.get(
'query'
)
const url = `?query=${
previousQuery ? previousQuery + '-added' : 'foo'
}`

window.history.replaceState(null, '', url)
}}
id="replace-string-url-null"
>
Replace searchParam with null data param
</button>

<button
onClick={() => {
const previousQuery = new URL(window.location.href).searchParams.get(
'query'
)
const url = `?query=${
previousQuery ? previousQuery + '-added' : 'foo'
}`

window.history.replaceState(undefined, '', url)
}}
id="replace-string-url-undefined"
>
Replace searchParam with undefined data param
</button>
</>
)
}
Loading

0 comments on commit 97ba910

Please sign in to comment.