From af07611a632311eeeac3360bfef6190ef2ddb72f Mon Sep 17 00:00:00 2001 From: JJ Kasper <22380829+ijjk@users.noreply.github.com> Date: Fri, 14 Dec 2018 05:25:59 -0600 Subject: [PATCH] Implement websockets based on-demand-entries ping (#4508) Fixes #4495 Here's my approach for replacing the XHR on-demand-entries pinger #1364 #4495. I'm not sure if this is the way everyone wants to accomplish this since I saw mention of using a separate server and port for the dynamic entries websocket, but thought this would be a fairly clean solution since it doesn't need that. With this method the only change when using a custom server is you have to listen for the upgrade event and pass it to next.getRequestHandler(). Example: ``` const server = app.listen(port) const handleRequest = next.getRequestHandler() if(dev) { server.on('upgrade', handleRequest) } ``` --- .../next/client/on-demand-entries-client.js | 91 ++++++++++++------- packages/next/server/hot-reloader.js | 20 ++++ .../next/server/on-demand-entry-handler.js | 70 +++++++------- test/integration/ondemand/test/index.test.js | 34 ++++++- 4 files changed, 147 insertions(+), 68 deletions(-) diff --git a/packages/next/client/on-demand-entries-client.js b/packages/next/client/on-demand-entries-client.js index 0f3bd4d4..ada77640 100644 --- a/packages/next/client/on-demand-entries-client.js +++ b/packages/next/client/on-demand-entries-client.js @@ -1,32 +1,58 @@ -/* global location */ +/* global location, WebSocket */ import Router from 'next/router' import fetch from 'unfetch' -export default ({assetPrefix}) => { +const { hostname } = location +const retryTime = 5000 +let ws = null +let lastHref = null + +export default async ({ assetPrefix }) => { Router.ready(() => { Router.events.on('routeChangeComplete', ping) }) - async function ping () { - try { - const url = `${assetPrefix || ''}/_next/on-demand-entries-ping?page=${Router.pathname}` - const res = await fetch(url, { - credentials: 'same-origin' - }) - const payload = await res.json() - if (payload.invalid) { - // Payload can be invalid even if the page is not exists. - // So, we need to make sure it's exists before reloading. - const pageRes = await fetch(location.href, { - credentials: 'same-origin' - }) - if (pageRes.status === 200) { - location.reload() + const setup = async (reconnect) => { + if (ws && ws.readyState === ws.OPEN) { + return Promise.resolve() + } + + return new Promise(resolve => { + ws = new WebSocket(`ws://${hostname}:${process.env.NEXT_WS_PORT}`) + ws.onopen = () => resolve() + ws.onclose = () => { + setTimeout(async () => { + // check if next restarted and we have to reload to get new port + await fetch(`${assetPrefix}/_next/on-demand-entries-ping`) + .then(res => res.status === 200 && location.reload()) + .catch(() => {}) + await setup(true) + resolve() + }, retryTime) + } + ws.onmessage = async ({ data }) => { + const payload = JSON.parse(data) + if (payload.invalid && lastHref !== location.href) { + // Payload can be invalid even if the page does not exist. + // So, we need to make sure it exists before reloading. + const pageRes = await fetch(location.href, { + credentials: 'omit' + }) + if (pageRes.status === 200) { + location.reload() + } else { + lastHref = location.href + } } } - } catch (err) { - console.error(`Error with on-demand-entries-ping: ${err.message}`) + }) + } + await setup() + + async function ping () { + if (ws.readyState === ws.OPEN) { + ws.send(Router.pathname) } } @@ -37,24 +63,27 @@ export default ({assetPrefix}) => { // at this point. while (!document.hidden) { await ping() - await new Promise((resolve) => { + await new Promise(resolve => { pingerTimeout = setTimeout(resolve, 5000) }) } } - document.addEventListener('visibilitychange', () => { - if (!document.hidden) { - runPinger() - } else { - clearTimeout(pingerTimeout) - } - }, false) + document.addEventListener( + 'visibilitychange', + () => { + if (!document.hidden) { + runPinger() + } else { + clearTimeout(pingerTimeout) + } + }, + false + ) setTimeout(() => { - runPinger() - .catch((err) => { - console.error(err) - }) + runPinger().catch(err => { + console.error(err) + }) }, 10000) } diff --git a/packages/next/server/hot-reloader.js b/packages/next/server/hot-reloader.js index a4f07a25..72ccbddf 100644 --- a/packages/next/server/hot-reloader.js +++ b/packages/next/server/hot-reloader.js @@ -5,6 +5,7 @@ import errorOverlayMiddleware from './lib/error-overlay-middleware' import del from 'del' import onDemandEntryHandler, {normalizePage} from './on-demand-entry-handler' import webpack from 'webpack' +import WebSocket from 'ws' import getBaseWebpackConfig from '../build/webpack-config' import {IS_BUNDLED_PAGE_REGEX, ROUTE_NAME_REGEX, BLOCKED_PAGES, CLIENT_STATIC_FILES_PATH} from 'next-server/constants' import {route} from 'next-server/dist/server/router' @@ -162,13 +163,28 @@ export default class HotReloader { return del(join(this.dir, this.config.distDir), { force: true }) } + addWsPort (configs) { + configs[0].plugins.push(new webpack.DefinePlugin({ + 'process.env.NEXT_WS_PORT': this.wsPort + })) + } + async start () { await this.clean() + await new Promise(resolve => { + // create dynamic entries WebSocket + this.wss = new WebSocket.Server({ port: 0 }, () => { + this.wsPort = this.wss.address().port + resolve() + }) + }) + const configs = await Promise.all([ getBaseWebpackConfig(this.dir, { dev: true, isServer: false, config: this.config, buildId: this.buildId }), getBaseWebpackConfig(this.dir, { dev: true, isServer: true, config: this.config, buildId: this.buildId }) ]) + this.addWsPort(configs) const multiCompiler = webpack(configs) @@ -179,6 +195,7 @@ export default class HotReloader { } async stop (webpackDevMiddleware) { + this.wss.close() const middleware = webpackDevMiddleware || this.webpackDevMiddleware if (middleware) { return new Promise((resolve, reject) => { @@ -199,6 +216,7 @@ export default class HotReloader { getBaseWebpackConfig(this.dir, { dev: true, isServer: false, config: this.config, buildId: this.buildId }), getBaseWebpackConfig(this.dir, { dev: true, isServer: true, config: this.config, buildId: this.buildId }) ]) + this.addWsPort(configs) const compiler = webpack(configs) @@ -215,6 +233,7 @@ export default class HotReloader { this.webpackDevMiddleware = webpackDevMiddleware this.webpackHotMiddleware = webpackHotMiddleware this.onDemandEntries = onDemandEntries + this.wss.on('connection', this.onDemandEntries.wsConnection) this.middlewares = [ webpackDevMiddleware, webpackHotMiddleware, @@ -357,6 +376,7 @@ export default class HotReloader { dev: true, reload: this.reload.bind(this), pageExtensions: this.config.pageExtensions, + wsPort: this.wsPort, ...this.config.onDemandEntries }) diff --git a/packages/next/server/on-demand-entry-handler.js b/packages/next/server/on-demand-entry-handler.js index 98c25b19..d2d178dd 100644 --- a/packages/next/server/on-demand-entry-handler.js +++ b/packages/next/server/on-demand-entry-handler.js @@ -1,7 +1,6 @@ import DynamicEntryPlugin from 'webpack/lib/DynamicEntryPlugin' import { EventEmitter } from 'events' import { join } from 'path' -import { parse } from 'url' import fs from 'fs' import promisify from '../lib/promisify' import globModule from 'glob' @@ -34,7 +33,8 @@ export default function onDemandEntryHandler (devMiddleware, multiCompiler, { reload, pageExtensions, maxInactiveAge = 1000 * 60, - pagesBufferLength = 2 + pagesBufferLength = 2, + wsPort }) { const {compilers} = multiCompiler const invalidator = new Invalidator(devMiddleware, multiCompiler) @@ -218,6 +218,37 @@ export default function onDemandEntryHandler (devMiddleware, multiCompiler, { }) }, + wsConnection (ws) { + ws.onmessage = ({ data }) => { + const page = normalizePage(data) + const entryInfo = entries[page] + + // If there's no entry. + // Then it seems like an weird issue. + if (!entryInfo) { + const message = `Client pings, but there's no entry for page: ${page}` + console.error(message) + return sendJson(ws, { invalid: true }) + } + + sendJson(ws, { success: true }) + + // We don't need to maintain active state of anything other than BUILT entries + if (entryInfo.status !== BUILT) return + + // If there's an entryInfo + if (!lastAccessPages.includes(page)) { + lastAccessPages.unshift(page) + + // Maintain the buffer max length + if (lastAccessPages.length > pagesBufferLength) { + lastAccessPages.pop() + } + } + entryInfo.lastActiveTime = Date.now() + } + }, + middleware () { return (req, res, next) => { if (stopped) { @@ -239,32 +270,9 @@ export default function onDemandEntryHandler (devMiddleware, multiCompiler, { } else { if (!/^\/_next\/on-demand-entries-ping/.test(req.url)) return next() - const { query } = parse(req.url, true) - const page = normalizePage(query.page) - const entryInfo = entries[page] - - // If there's no entry. - // Then it seems like an weird issue. - if (!entryInfo) { - const message = `Client pings, but there's no entry for page: ${page}` - console.error(message) - sendJson(res, { invalid: true }) - return - } - - sendJson(res, { success: true }) - - // We don't need to maintain active state of anything other than BUILT entries - if (entryInfo.status !== BUILT) return - - // If there's an entryInfo - if (!lastAccessPages.includes(page)) { - lastAccessPages.unshift(page) - - // Maintain the buffer max length - if (lastAccessPages.length > pagesBufferLength) lastAccessPages.pop() - } - entryInfo.lastActiveTime = Date.now() + res.statusCode = 200 + res.setHeader('port', wsPort) + res.end('200') } } } @@ -310,10 +318,8 @@ export function normalizePage (page) { return unixPagePath.replace(/\/index$/, '') } -function sendJson (res, payload) { - res.setHeader('Content-Type', 'application/json') - res.status = 200 - res.end(JSON.stringify(payload)) +function sendJson (ws, data) { + ws.send(JSON.stringify(data)) } // Make sure only one invalidation happens at a time diff --git a/test/integration/ondemand/test/index.test.js b/test/integration/ondemand/test/index.test.js index 9b1c067a..bc48afe4 100644 --- a/test/integration/ondemand/test/index.test.js +++ b/test/integration/ondemand/test/index.test.js @@ -3,8 +3,10 @@ import { join, resolve } from 'path' import { existsSync } from 'fs' import webdriver from 'next-webdriver' +import WebSocket from 'ws' import { renderViaHTTP, + fetchViaHTTP, findPort, launchApp, killApp, @@ -15,6 +17,13 @@ import { const context = {} +const doPing = path => { + return new Promise(resolve => { + context.ws.onmessage = () => resolve() + context.ws.send(path) + }) +} + jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5 describe('On Demand Entries', () => { @@ -22,19 +31,34 @@ describe('On Demand Entries', () => { beforeAll(async () => { context.appPort = await findPort() context.server = await launchApp(join(__dirname, '../'), context.appPort) + await new Promise(resolve => { + fetchViaHTTP(context.appPort, '/_next/on-demand-entries-ping').then(res => { + const wsPort = res.headers.get('port') + context.ws = new WebSocket( + `ws://localhost:${wsPort}` + ) + context.ws.on('open', () => resolve()) + }) + }) + }) + afterAll(() => { + context.ws.close() + killApp(context.server) }) - afterAll(() => killApp(context.server)) it('should compile pages for SSR', async () => { // The buffer of built page uses the on-demand-entries-ping to know which pages should be // buffered. Therefore, we need to double each render call with a ping. const pageContent = await renderViaHTTP(context.appPort, '/') - await renderViaHTTP(context.appPort, '/_next/on-demand-entries-ping', {page: '/'}) + await doPing('/') expect(pageContent.includes('Index Page')).toBeTruthy() }) it('should compile pages for JSON page requests', async () => { - const pageContent = await renderViaHTTP(context.appPort, '/_next/static/development/pages/about.js') + const pageContent = await renderViaHTTP( + context.appPort, + '/_next/static/development/pages/about.js' + ) expect(pageContent.includes('About Page')).toBeTruthy() }) @@ -44,11 +68,11 @@ describe('On Demand Entries', () => { // Render two pages after the index, since the server keeps at least two pages await renderViaHTTP(context.appPort, '/about') - await renderViaHTTP(context.appPort, '/_next/on-demand-entries-ping', {page: '/about'}) + await doPing('/about') const aboutPagePath = resolve(__dirname, '../.next/static/development/pages/about.js') await renderViaHTTP(context.appPort, '/third') - await renderViaHTTP(context.appPort, '/_next/on-demand-entries-ping', {page: '/third'}) + await doPing('/third') const thirdPagePath = resolve(__dirname, '../.next/static/development/pages/third.js') // Wait maximum of jasmine.DEFAULT_TIMEOUT_INTERVAL checking