Skip to content

Commit

Permalink
Ensure rewrites are resolved while prefetching (vercel#22442)
Browse files Browse the repository at this point in the history
This ensures we handle resolve rewrites during prefetching the same way we do during a client-transition. Previously if a rewritten source was used in an `href` neither the page bundle or SSG data if needed would be prefetched although would work correctly on a client transition. 


Fixes: vercel#22441
  • Loading branch information
ijjk authored Feb 24, 2021
1 parent a78e904 commit 9d2b0fc
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 37 deletions.
92 changes: 59 additions & 33 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,34 @@ function prepareUrlAs(router: NextRouter, url: Url, as?: Url) {
}
}

function resolveDynamicRoute(
parsedHref: UrlObject,
pages: string[],
applyBasePath = true
) {
const { pathname } = parsedHref
const cleanPathname = removePathTrailingSlash(
denormalizePagePath(applyBasePath ? delBasePath(pathname!) : pathname!)
)

if (cleanPathname === '/404' || cleanPathname === '/_error') {
return parsedHref
}

// handle resolving href for dynamic routes
if (!pages.includes(cleanPathname!)) {
// eslint-disable-next-line array-callback-return
pages.some((page) => {
if (isDynamicRoute(page) && getRouteRegex(page).re.test(cleanPathname!)) {
parsedHref.pathname = applyBasePath ? addBasePath(page) : page
return true
}
})
}
parsedHref.pathname = removePathTrailingSlash(parsedHref.pathname!)
return parsedHref
}

export type BaseRouter = {
route: string
pathname: string
Expand Down Expand Up @@ -915,7 +943,7 @@ export default class Router implements BaseRouter {
return false
}

parsed = this._resolveHref(parsed, pages) as typeof parsed
parsed = resolveDynamicRoute(parsed, pages) as typeof parsed

if (parsed.pathname !== pathname) {
pathname = parsed.pathname
Expand Down Expand Up @@ -950,7 +978,7 @@ export default class Router implements BaseRouter {
pages,
rewrites,
query,
(p: string) => this._resolveHref({ pathname: p }, pages).pathname!,
(p: string) => resolveDynamicRoute({ pathname: p }, pages).pathname!,
this.locales
)
resolvedAs = rewritesResult.asPath
Expand Down Expand Up @@ -1058,7 +1086,7 @@ export default class Router implements BaseRouter {
// it's not
if (destination.startsWith('/')) {
const parsedHref = parseRelativeUrl(destination)
this._resolveHref(parsedHref, pages, false)
resolveDynamicRoute(parsedHref, pages, false)

if (pages.includes(parsedHref.pathname)) {
const { url: newUrl, as: newAs } = prepareUrlAs(
Expand Down Expand Up @@ -1419,33 +1447,6 @@ export default class Router implements BaseRouter {
return this.asPath !== asPath
}

_resolveHref(parsedHref: UrlObject, pages: string[], applyBasePath = true) {
const { pathname } = parsedHref
const cleanPathname = removePathTrailingSlash(
denormalizePagePath(applyBasePath ? delBasePath(pathname!) : pathname!)
)

if (cleanPathname === '/404' || cleanPathname === '/_error') {
return parsedHref
}

// handle resolving href for dynamic routes
if (!pages.includes(cleanPathname!)) {
// eslint-disable-next-line array-callback-return
pages.some((page) => {
if (
isDynamicRoute(page) &&
getRouteRegex(page).re.test(cleanPathname!)
) {
parsedHref.pathname = applyBasePath ? addBasePath(page) : page
return true
}
})
}
parsedHref.pathname = removePathTrailingSlash(parsedHref.pathname!)
return parsedHref
}

/**
* Prefetch page code, you may wait for the data during page rendering.
* This feature only works in production!
Expand Down Expand Up @@ -1480,26 +1481,51 @@ export default class Router implements BaseRouter {

const pages = await this.pageLoader.getPageList()

parsed = this._resolveHref(parsed, pages, false) as typeof parsed
parsed = resolveDynamicRoute(parsed, pages, false) as typeof parsed

if (parsed.pathname !== pathname) {
pathname = parsed.pathname
url = formatWithValidation(parsed)
}
let route = removePathTrailingSlash(pathname)
let resolvedAs = asPath

if (process.env.__NEXT_HAS_REWRITES && asPath.startsWith('/')) {
let rewrites: any[]
;({ __rewrites: rewrites } = await getClientBuildManifest())

const rewritesResult = resolveRewrites(
addBasePath(addLocale(delBasePath(asPath), this.locale)),
pages,
rewrites,
parsed.query,
(p: string) => resolveDynamicRoute({ pathname: p }, pages).pathname!,
this.locales
)

if (rewritesResult.matchedPage && rewritesResult.resolvedHref) {
// if this directly matches a page we need to update the href to
// allow the correct page chunk to be loaded
route = rewritesResult.resolvedHref
pathname = rewritesResult.resolvedHref
parsed.pathname = pathname
url = formatWithValidation(parsed)
resolvedAs = rewritesResult.asPath
}
}

// Prefetch is not supported in development mode because it would trigger on-demand-entries
if (process.env.NODE_ENV !== 'production') {
return
}

const route = removePathTrailingSlash(pathname)
await Promise.all([
this.pageLoader._isSsg(url).then((isSsg: boolean) => {
return isSsg
? this._getStaticData(
this.pageLoader.getDataHref(
url,
asPath,
resolvedAs,
true,
typeof options.locale !== 'undefined'
? options.locale
Expand Down
6 changes: 3 additions & 3 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('Build Output', () => {
expect(indexSize.endsWith('B')).toBe(true)

// should be no bigger than 63.9 kb
expect(parseFloat(indexFirstLoad)).toBeCloseTo(64, 1)
expect(parseFloat(indexFirstLoad)).toBeCloseTo(64.1, 1)
expect(indexFirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(err404Size) - 3.7).toBeLessThanOrEqual(0)
Expand All @@ -104,7 +104,7 @@ describe('Build Output', () => {
expect(parseFloat(err404FirstLoad)).toBeCloseTo(67.1, 0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll)).toBeCloseTo(63.7, 1)
expect(parseFloat(sharedByAll)).toBeCloseTo(63.8, 1)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down Expand Up @@ -168,7 +168,7 @@ describe('Build Output', () => {
expect(parseFloat(indexSize)).toBeGreaterThanOrEqual(2)
expect(indexSize.endsWith('kB')).toBe(true)

expect(parseFloat(indexFirstLoad)).toBeLessThanOrEqual(66.8)
expect(parseFloat(indexFirstLoad)).toBeLessThanOrEqual(66.9)
expect(parseFloat(indexFirstLoad)).toBeGreaterThanOrEqual(60)
expect(indexFirstLoad.endsWith('kB')).toBe(true)
})
Expand Down
8 changes: 8 additions & 0 deletions test/integration/preload-viewport/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,12 @@ module.exports = {
generateBuildId() {
return 'test-build'
},
rewrites() {
return [
{
source: '/rewrite-me',
destination: '/ssg/dynamic/one',
},
]
},
}
16 changes: 16 additions & 0 deletions test/integration/preload-viewport/pages/[...rest].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
export const getStaticProps = () => {
return {
props: {},
}
}

export const getStaticPaths = () => {
return {
paths: [{ params: { rest: ['one'] } }],
fallback: false,
}
}

export default function Page() {
return <p id="top-level-rest">Hello from [...rest]</p>
}
9 changes: 9 additions & 0 deletions test/integration/preload-viewport/pages/rewrite-prefetch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Link from 'next/link'

export default function Page() {
return (
<Link href="/rewrite-me">
<a>to /rewrite-me</a>
</Link>
)
}
26 changes: 26 additions & 0 deletions test/integration/preload-viewport/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,31 @@ describe('Prefetching Links in viewport', () => {
}
})

it('should prefetch rewritten href with link in viewport onload', async () => {
let browser
try {
browser = await webdriver(appPort, '/rewrite-prefetch')
const links = await browser.elementsByCss('link[rel=prefetch]')
let found = false

for (const link of links) {
const href = await link.getAttribute('href')
if (href.includes('%5Bslug%5D')) {
found = true
break
}
}
expect(found).toBe(true)

const hrefs = await browser.eval(`Object.keys(window.next.router.sdc)`)
expect(hrefs.map((href) => new URL(href).pathname)).toEqual([
'/_next/data/test-build/ssg/dynamic/one.json',
])
} finally {
if (browser) await browser.close()
}
})

it('should prefetch with link in viewport when href changes', async () => {
let browser
try {
Expand Down Expand Up @@ -337,6 +362,7 @@ describe('Prefetching Links in viewport', () => {
eval(content)
expect([...self.__SSG_MANIFEST].sort()).toMatchInlineSnapshot(`
Array [
"/[...rest]",
"/ssg/basic",
"/ssg/catch-all/[...slug]",
"/ssg/dynamic-nested/[slug1]/[slug2]",
Expand Down
2 changes: 1 addition & 1 deletion test/integration/size-limit/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ describe('Production response size', () => {
const delta = responseSizesBytes / 1024

// Expected difference: < 0.5
expect(delta).toBeCloseTo(284.7, 0)
expect(delta).toBeCloseTo(285.3, 0)
})
})

0 comments on commit 9d2b0fc

Please sign in to comment.