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

Never cache assets and HTML in the dev mode. #2045

Merged
merged 2 commits into from
May 25, 2017
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
8 changes: 4 additions & 4 deletions server/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export class Head extends Component {

getChunkPreloadLink (filename) {
const { __NEXT_DATA__ } = this.context._documentProps
let { buildStats, assetPrefix } = __NEXT_DATA__
const hash = buildStats ? buildStats[filename].hash : '-'
let { buildStats, assetPrefix, buildId } = __NEXT_DATA__
const hash = buildStats ? buildStats[filename].hash : buildId

return (
<link
Expand Down Expand Up @@ -103,8 +103,8 @@ export class NextScript extends Component {

getChunkScript (filename, additionalProps = {}) {
const { __NEXT_DATA__ } = this.context._documentProps
let { buildStats, assetPrefix } = __NEXT_DATA__
const hash = buildStats ? buildStats[filename].hash : '-'
let { buildStats, assetPrefix, buildId } = __NEXT_DATA__
const hash = buildStats ? buildStats[filename].hash : buildId

return (
<script
Expand Down
5 changes: 3 additions & 2 deletions server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export default class Server {
res.setHeader('X-Powered-By', `Next.js ${pkg.version}`)
}
const html = await this.renderToHTML(req, res, pathname, query)
return sendHTML(req, res, html, req.method)
return sendHTML(req, res, html, req.method, this.renderOpts)
}

async renderToHTML (req, res, pathname, query) {
Expand Down Expand Up @@ -253,7 +253,7 @@ export default class Server {

async renderError (err, req, res, pathname, query) {
const html = await this.renderErrorToHTML(err, req, res, pathname, query)
return sendHTML(req, res, html, req.method)
return sendHTML(req, res, html, req.method, this.renderOpts)
}

async renderErrorToHTML (err, req, res, pathname, query) {
Expand Down Expand Up @@ -335,6 +335,7 @@ export default class Server {

handleBuildHash (filename, hash, res) {
if (this.dev) return

if (hash !== this.buildStats[filename].hash) {
throw new Error(`Invalid Build File Hash(${hash}) for chunk: ${filename}`)
}
Expand Down
19 changes: 15 additions & 4 deletions server/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import ErrorDebug from '../lib/error-debug'

export async function render (req, res, pathname, query, opts) {
const html = await renderToHTML(req, res, pathname, opts)
sendHTML(req, res, html, req.method)
sendHTML(req, res, html, req.method, opts)
}

export function renderToHTML (req, res, pathname, query, opts) {
Expand All @@ -24,7 +24,7 @@ export function renderToHTML (req, res, pathname, query, opts) {

export async function renderError (err, req, res, pathname, query, opts) {
const html = await renderErrorToHTML(err, req, res, query, opts)
sendHTML(req, res, html, req.method)
sendHTML(req, res, html, req.method, opts)
}

export function renderErrorToHTML (err, req, res, pathname, query, opts = {}) {
Expand Down Expand Up @@ -87,6 +87,11 @@ async function doRender (req, res, pathname, query, {
}

const docProps = await loadGetInitialProps(Document, { ...ctx, renderPage })
// While developing, we should not cache any assets.
// So, we use a different buildId for each page load.
// With that we can ensure, we have unique URL for assets per every page load.
// So, it'll prevent issues like this: https://git.io/vHLtb
const devBuildId = Date.now()

if (res.finished) return

Expand All @@ -96,7 +101,7 @@ async function doRender (req, res, pathname, query, {
props,
pathname,
query,
buildId,
buildId: dev ? devBuildId : buildId,
buildStats,
assetPrefix,
err: (err) ? serializeError(dev, err) : null
Expand Down Expand Up @@ -156,7 +161,7 @@ export async function renderScriptError (req, res, page, error, customFields, op
`)
}

export function sendHTML (req, res, html, method) {
export function sendHTML (req, res, html, method, { dev }) {
if (res.finished) return
const etag = generateETag(html)

Expand All @@ -166,6 +171,12 @@ export function sendHTML (req, res, html, method) {
return
}

if (dev) {
// In dev, we should not cache pages for any reason.
// That's why we do this.
res.setHeader('Cache-Control', 'no-store, must-revalidate')
}

res.setHeader('ETag', etag)
res.setHeader('Content-Type', 'text/html')
res.setHeader('Content-Length', Buffer.byteLength(html))
Expand Down
10 changes: 0 additions & 10 deletions test/integration/basic/test/misc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* global describe, test, expect */
import fetch from 'node-fetch'

export default function (context) {
describe('Misc', () => {
Expand All @@ -13,14 +12,5 @@ export default function (context) {
const html = await context.app.renderToHTML({}, res, '/finish-response', {})
expect(html).toBeFalsy()
})

test('allow etag header support', async () => {
const url = `http://localhost:${context.appPort}/stateless`
const etag = (await fetch(url)).headers.get('ETag')

const headers = { 'If-None-Match': etag }
const res2 = await fetch(url, { headers })
expect(res2.status).toBe(304)
})
})
}
10 changes: 10 additions & 0 deletions test/integration/production/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
renderViaHTTP
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import fetch from 'node-fetch'

const appDir = join(__dirname, '../')
let appPort
Expand All @@ -35,6 +36,15 @@ describe('Production Usage', () => {
const html = await renderViaHTTP(appPort, '/')
expect(html).toMatch(/Hello World/)
})

it('should allow etag header support', async () => {
const url = `http://localhost:${appPort}/`
const etag = (await fetch(url)).headers.get('ETag')

const headers = { 'If-None-Match': etag }
const res2 = await fetch(url, { headers })
expect(res2.status).toBe(304)
})
})

describe('With navigation', () => {
Expand Down