Skip to content

Commit

Permalink
fix: use a client-side navigation when redirecting to a rewriten URL (#…
Browse files Browse the repository at this point in the history
…25990)

This PR is an attempt to fix #25943. The implementation is only a proposal and I'll be happy to take any feedback about it :)

Fixes: #25943

## Bug

- [X] Related issues linked using `fixes #number`
- [x] Integration tests added

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.

## Documentation / Examples

- [ ] Make sure the linting passes
  • Loading branch information
antoinechalifour authored Jun 16, 2021
1 parent 8610ba4 commit 70dffd4
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 11 deletions.
14 changes: 6 additions & 8 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1112,14 +1112,12 @@ export default class Router implements BaseRouter {
pages
)

if (pages.includes(parsedHref.pathname)) {
const { url: newUrl, as: newAs } = prepareUrlAs(
this,
destination,
destination
)
return this.change(method, newUrl, newAs, options)
}
const { url: newUrl, as: newAs } = prepareUrlAs(
this,
destination,
destination
)
return this.change(method, newUrl, newAs, options)
}

window.location.href = destination
Expand Down
4 changes: 2 additions & 2 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ describe('Build Output', () => {
expect(parseFloat(err404Size)).toBeCloseTo(gz ? 3.17 : 8.51, 1)
expect(err404Size.endsWith('kB')).toBe(true)

expect(parseFloat(err404FirstLoad)).toBeCloseTo(gz ? 66.9 : 205, 1)
expect(parseFloat(err404FirstLoad)).toBeCloseTo(gz ? 66.9 : 204, 1)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll)).toBeCloseTo(gz ? 63.7 : 196, 1)
Expand All @@ -149,7 +149,7 @@ describe('Build Output', () => {
true
)

expect(parseFloat(mainSize)).toBeCloseTo(gz ? 20.1 : 62.8, 1)
expect(parseFloat(mainSize)).toBeCloseTo(gz ? 20.1 : 62.7, 1)
expect(mainSize.endsWith('kB')).toBe(true)

expect(parseFloat(frameworkSize)).toBeCloseTo(gz ? 42.0 : 130, 1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ const runTests = (isDev) => {

const curUrl = await browser.url()
const { pathname } = url.parse(curUrl)
expect(pathname).toBe('/missing')
expect(pathname).toBe('/docs/missing')
})

it('should apply redirect when fallback GSP page is visited directly (external domain)', async () => {
Expand Down
10 changes: 10 additions & 0 deletions test/integration/gssp-redirect-with-rewrites/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = {
async rewrites() {
return [
{
source: '/alias-to-main-content',
destination: '/main-content',
},
]
},
}
35 changes: 35 additions & 0 deletions test/integration/gssp-redirect-with-rewrites/pages/main-content.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import Link from 'next/link'

export default function MainContent({ message }) {
return (
<main>
<h1>Hello {message}</h1>

<ul>
<li>
<Link href="/redirector?redirect=/alias-to-main-content&message=refreshed">
<a id="link-with-rewritten-url" className={message}>
Link with rewritten target url
</a>
</Link>
</li>

<li>
<Link href="/redirector?redirect=/main-content&message=refreshWithClientSideNavigation">
<a>Link with client side navigation</a>
</Link>
</li>

<li>
<Link href="/redirector?redirect=/unknown-route">
<a id="link-unknown-url">Link to unknown internal navigation</a>
</Link>
</li>
</ul>
</main>
)
}

export const getServerSideProps = ({ query }) => ({
props: { message: query.message || 'World ' },
})
16 changes: 16 additions & 0 deletions test/integration/gssp-redirect-with-rewrites/pages/redirector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export default function Redirector() {
return (
<main>
<h1>Hello world</h1>
</main>
)
}

export const getServerSideProps = ({ query }) => {
return {
redirect: {
destination: `${query.redirect}?message=${query.message}`,
permanent: false,
},
}
}
62 changes: 62 additions & 0 deletions test/integration/gssp-redirect-with-rewrites/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/* eslint-env jest */

import { join } from 'path'
import {
renderViaHTTP,
findPort,
launchApp,
killApp,
check,
} from 'next-test-utils'
import webdriver from 'next-webdriver'

// test suites

const context = {}
jest.setTimeout(1000 * 60 * 5)

describe('getServerSideProps redirects', () => {
beforeAll(async () => {
context.appPort = await findPort()
context.server = await launchApp(join(__dirname, '../'), context.appPort, {
env: { __NEXT_TEST_WITH_DEVTOOL: 1 },
})

// pre-build all pages at the start
await Promise.all([
renderViaHTTP(context.appPort, '/alias-to-main-content'),
renderViaHTTP(context.appPort, '/main-content'),
])
})
afterAll(() => killApp(context.server))

it('should use a client-side navigation for a rewritten URL', async () => {
const browser = await webdriver(context.appPort, '/alias-to-main-content')

// During a browser navigation global variables are reset,
// So by checking that the __SAME_PAGE variable is still defined
// then the client-side navigation has happened
await browser.eval('window.__SAME_PAGE = true')

await browser.elementByCss('#link-with-rewritten-url').click()

// Wait until the new props are rendered
await browser.waitForElementByCss('.refreshed')

expect(await browser.eval('window.__SAME_PAGE')).toBe(true)
})

it('should fallback to browser navigation for an unknown URL', async () => {
const browser = await webdriver(context.appPort, '/alias-to-main-content')

// then the client-side navigation has happened
await browser.eval('window.__SAME_PAGE = true')
await browser.elementByCss('#link-unknown-url').click()

// Wait until the page has be reloaded
await check(async () => {
const val = await browser.eval('window.__SAME_PAGE')
return val ? 'fail' : 'success'
}, 'success')
})
})

0 comments on commit 70dffd4

Please sign in to comment.