Skip to content

Commit

Permalink
Ensure stalled CSS triggers fallback navigation (#24488)
Browse files Browse the repository at this point in the history
This ensures when CSS requests stall that they are included in the route load timeout so that stalled CSS requests don't block us from falling back to a hard navigation. This handles a rare case noticed by @pacocoursey where a transition did not complete while attempting to load CSS assets. 

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
  • Loading branch information
ijjk authored Apr 27, 2021
1 parent fff183c commit efa58ef
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 32 deletions.
66 changes: 37 additions & 29 deletions packages/next/client/route-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function withFuture<T>(
export interface RouteLoader {
whenEntrypoint(route: string): Promise<RouteEntrypoint>
onEntrypoint(route: string, execute: () => unknown): void
loadRoute(route: string): Promise<RouteLoaderEntry>
loadRoute(route: string, prefetch?: boolean): Promise<RouteLoaderEntry>
prefetch(route: string): Promise<void>
}

Expand Down Expand Up @@ -305,33 +305,41 @@ function createRouteLoader(assetPrefix: string): RouteLoader {
if (old && 'resolve' in old) old.resolve(input)
})
},
loadRoute(route: string) {
return withFuture<RouteLoaderEntry>(route, routes, async () => {
try {
const { scripts, css } = await getFilesForRoute(assetPrefix, route)
const [, styles] = await Promise.all([
entrypoints.has(route)
? []
: Promise.all(scripts.map(maybeExecuteScript)),
Promise.all(css.map(fetchStyleSheet)),
] as const)

const entrypoint: RouteEntrypoint = await resolvePromiseWithTimeout(
this.whenEntrypoint(route),
MS_MAX_IDLE_DELAY,
markAssetError(
new Error(`Route did not complete loading: ${route}`)
)
)

const res: RouteLoaderEntry = Object.assign<
{ styles: RouteStyleSheet[] },
RouteEntrypoint
>({ styles }, entrypoint)
return 'error' in entrypoint ? entrypoint : res
} catch (err) {
return { error: err }
}
loadRoute(route: string, prefetch?: boolean) {
return withFuture<RouteLoaderEntry>(route, routes, () => {
return resolvePromiseWithTimeout(
getFilesForRoute(assetPrefix, route)
.then(({ scripts, css }) => {
return Promise.all([
entrypoints.has(route)
? []
: Promise.all(scripts.map(maybeExecuteScript)),
Promise.all(css.map(fetchStyleSheet)),
] as const)
})
.then((res) => {
return this.whenEntrypoint(route).then((entrypoint) => ({
entrypoint,
styles: res[1],
}))
}),
MS_MAX_IDLE_DELAY,
markAssetError(new Error(`Route did not complete loading: ${route}`))
)
.then(({ entrypoint, styles }) => {
const res: RouteLoaderEntry = Object.assign<
{ styles: RouteStyleSheet[] },
RouteEntrypoint
>({ styles: styles! }, entrypoint)
return 'error' in entrypoint ? entrypoint : res
})
.catch((err) => {
if (prefetch) {
// we don't want to cache errors during prefetch
throw err
}
return { error: err }
})
})
},
prefetch(route: string): Promise<void> {
Expand All @@ -351,7 +359,7 @@ function createRouteLoader(assetPrefix: string): RouteLoader {
)
)
.then(() => {
requestIdleCallback(() => this.loadRoute(route))
requestIdleCallback(() => this.loadRoute(route, true).catch(() => {}))
})
.catch(
// swallow prefetch errors
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 @@ -99,7 +99,7 @@ describe('Build Output', () => {
expect(indexSize.endsWith('B')).toBe(true)

// should be no bigger than 64.8 kb
expect(parseFloat(indexFirstLoad)).toBeCloseTo(65.4, 1)
expect(parseFloat(indexFirstLoad)).toBeCloseTo(65.3, 1)
expect(indexFirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(err404Size)).toBeCloseTo(3.69, 1)
Expand All @@ -108,7 +108,7 @@ describe('Build Output', () => {
expect(parseFloat(err404FirstLoad)).toBeCloseTo(68.8, 0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll)).toBeCloseTo(65.1, 1)
expect(parseFloat(sharedByAll)).toBeCloseTo(65, 1)
expect(sharedByAll.endsWith('kB')).toBe(true)

if (_appSize.endsWith('kB')) {
Expand Down
63 changes: 62 additions & 1 deletion test/integration/css-client-nav/test/index.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-env jest */

import http from 'http'
import httpProxy from 'http-proxy'
import cheerio from 'cheerio'
import { remove } from 'fs-extra'
import {
Expand All @@ -18,6 +20,8 @@ jest.setTimeout(1000 * 60 * 1)
const fixturesDir = join(__dirname, '../../css-fixtures')
const appDir = join(fixturesDir, 'multi-module')

let proxyServer
let stallCss
let appPort
let app

Expand Down Expand Up @@ -152,12 +156,69 @@ describe('CSS Module client-side navigation', () => {
beforeAll(async () => {
await remove(join(appDir, '.next'))
await nextBuild(appDir)
const port = await findPort()
app = await nextStart(appDir, port)
appPort = await findPort()
app = await nextStart(appDir, appPort)

const proxy = httpProxy.createProxyServer({
target: `http://localhost:${port}`,
})

proxyServer = http.createServer(async (req, res) => {
if (stallCss && req.url.endsWith('.css')) {
console.log('stalling request for', req.url)
await new Promise((resolve) => setTimeout(resolve, 5 * 1000))
}
proxy.web(req, res)
})

proxy.on('error', (err) => {
console.warn('Failed to proxy', err)
})

await new Promise((resolve) => {
proxyServer.listen(appPort, () => resolve())
})
})
afterAll(async () => {
proxyServer.close()
await killApp(app)
})

it('should time out and hard navigate for stalled CSS request', async () => {
let browser
stallCss = true

try {
browser = await webdriver(appPort, '/red')
browser.eval('window.beforeNav = "hello"')

const redColor = await browser.eval(
`window.getComputedStyle(document.querySelector('#verify-red')).color`
)
expect(redColor).toMatchInlineSnapshot(`"rgb(255, 0, 0)"`)
expect(await browser.eval('window.beforeNav')).toBe('hello')

await browser.elementByCss('#link-blue').click()

await browser.waitForElementByCss('#verify-blue')

const blueColor = await browser.eval(
`window.getComputedStyle(document.querySelector('#verify-blue')).color`
)
expect(blueColor).toMatchInlineSnapshot(`"rgb(0, 0, 255)"`)

// the timeout should have been reached and we did a hard
// navigation
expect(await browser.eval('window.beforeNav')).toBe(null)
} finally {
stallCss = false
if (browser) {
await browser.close()
}
}
})

runTests()
})

Expand Down

0 comments on commit efa58ef

Please sign in to comment.