Skip to content
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

Fix accessing router before ready for HMR ping #31588

Merged
merged 4 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/next/client/dev/amp-dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,5 @@ connectHMR({
path: '/_next/webpack-hmr',
})
displayContent()
initOnDemandEntries()

initOnDemandEntries(data.page)
18 changes: 14 additions & 4 deletions packages/next/client/dev/on-demand-entries-client.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
import Router from 'next/router'
import { addMessageListener, sendMessage } from './error-overlay/websocket'

export default async () => {
setInterval(() => {
sendMessage(JSON.stringify({ event: 'ping', page: Router.pathname }))
}, 2500)
export default async (page) => {
if (page) {
// in AMP the router isn't initialized on the client and
// client-transitions don't occur so ping initial page
setInterval(() => {
sendMessage(JSON.stringify({ event: 'ping', page }))
}, 2500)
} else {
Router.ready(() => {
setInterval(() => {
sendMessage(JSON.stringify({ event: 'ping', page: Router.pathname }))
}, 2500)
})
}

addMessageListener((event) => {
if (event.data.indexOf('{') === -1) return
Expand Down
16 changes: 0 additions & 16 deletions test/.eslintrc.json

This file was deleted.

29 changes: 29 additions & 0 deletions test/integration/dynamic-routing/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,35 @@ const appDir = join(__dirname, '../')
const buildIdPath = join(appDir, '.next/BUILD_ID')

function runTests(dev) {
if (dev) {
it('should not have error after pinging WebSocket', async () => {
const browser = await webdriver(appPort, '/')
await browser.eval(`(function() {
window.uncaughtErrs = []
window.addEventListener('uncaughtexception', function (err) {
window.uncaught.push(err)
})
})()`)
const curFrames = [...(await browser.websocketFrames())]
await check(async () => {
const frames = await browser.websocketFrames()
const newFrames = frames.slice(curFrames.length)
// console.error({newFrames, curFrames, frames});

return newFrames.some((frame) => {
try {
const data = JSON.parse(frame.payload)
return data.event === 'pong'
} catch (_) {}
return false
})
? 'success'
: JSON.stringify(newFrames)
}, 'success')
expect(await browser.eval('window.uncaughtErrs.length')).toBe(0)
})
}

it('should support long URLs for dynamic routes', async () => {
const res = await fetchViaHTTP(
appPort,
Expand Down
3 changes: 3 additions & 0 deletions test/lib/browsers/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ export class BrowserInterface {
async log(): Promise<any[]> {
return []
}
async websocketFrames(): Promise<any[]> {
return []
}
async url(): Promise<string> {
return ''
}
Expand Down
21 changes: 16 additions & 5 deletions test/lib/browsers/playwright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ let page: Page
let browser: Browser
let context: BrowserContext
let pageLogs: Array<{ source: string; message: string }> = []
let websocketFrames: Array<{ payload: string | Buffer }> = []

const tracePlaywright = process.env.TRACE_PLAYWRIGHT

Expand Down Expand Up @@ -71,6 +72,7 @@ class Playwright extends BrowserInterface {
}
page = await context.newPage()
pageLogs = []
websocketFrames = []

page.on('console', (msg) => {
console.log('browser log:', msg)
Expand All @@ -83,25 +85,30 @@ class Playwright extends BrowserInterface {
console.error('page error', error)
})

if (tracePlaywright) {
page.on('websocket', (ws) => {
page.on('websocket', (ws) => {
if (tracePlaywright) {
page
.evaluate(`console.log('connected to ws at ${ws.url()}')`)
.catch(() => {})

ws.on('close', () =>
page
.evaluate(`console.log('closed websocket ${ws.url()}')`)
.catch(() => {})
)
ws.on('framereceived', (frame) => {
}
ws.on('framereceived', (frame) => {
websocketFrames.push({ payload: frame.payload })

if (tracePlaywright) {
if (!frame.payload.includes('pong')) {
page
.evaluate(`console.log('received ws message ${frame.payload}')`)
.catch(() => {})
}
})
}
})
}
})

if (tracePlaywright) {
await context.tracing.start({
Expand Down Expand Up @@ -310,6 +317,10 @@ class Playwright extends BrowserInterface {
return this.chain(() => pageLogs) as any
}

async websocketFrames() {
return this.chain(() => websocketFrames) as any
}

async url() {
return this.chain(() => page.evaluate('window.location.href')) as any
}
Expand Down