-
Notifications
You must be signed in to change notification settings - Fork 27.9k
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
Add Gzip support for JSON pages #571
Changes from 12 commits
bc2774a
b6daa28
6dd5db3
bff2435
901915d
6401e33
3af72f1
81ba173
298629f
b0b04a7
1e0e7a8
e24f261
7a04f38
79d566c
352fe77
20d01ba
343cb92
370189b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,29 @@ | ||
import fs from 'fs' | ||
import path from 'path' | ||
import zlib from 'zlib' | ||
import glob from 'glob-promise' | ||
|
||
export default async function gzipAssets (dir) { | ||
const nextDir = path.resolve(dir, '.next') | ||
|
||
await Promise.all([ | ||
gzip(path.resolve(nextDir, 'commons.js')), | ||
gzip(path.resolve(nextDir, 'main.js')) | ||
]) | ||
const coreAssets = [ | ||
path.join(nextDir, 'commons.js'), | ||
path.join(nextDir, 'main.js') | ||
] | ||
const pages = await glob('bundles/pages/**/*.js', { cwd: nextDir }) | ||
|
||
const allAssets = [ | ||
...coreAssets, | ||
...pages.map(page => path.join(nextDir, page)) | ||
] | ||
|
||
while (true) { | ||
// gzip only 10 assets in parallel at a time. | ||
const currentChunk = allAssets.splice(0, 10) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rauchg it didn't sounds nice to gzip all at once in parallel to me. |
||
if (currentChunk.length === 0) break | ||
|
||
await Promise.all(currentChunk.map(gzip)) | ||
} | ||
} | ||
|
||
export function gzip (filePath) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
export default class JsonPagesPlugin { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nkzawa This is my first webpack plugin. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks good to me 👍 |
||
apply (compiler) { | ||
compiler.plugin('after-compile', (compilation, callback) => { | ||
const pages = Object | ||
.keys(compilation.assets) | ||
.filter((filename) => /^bundles\/pages/.test(filename)) | ||
|
||
pages.forEach((pageName) => { | ||
const page = compilation.assets[pageName] | ||
if (page.jsoned) return | ||
|
||
const content = page.source() | ||
const newContent = JSON.stringify({ component: content }) | ||
page.source = () => newContent | ||
page.size = () => newContent.length | ||
page.jsoned = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we change the extension to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. That can be done. I'll try it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
}) | ||
|
||
callback() | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,9 @@ import WatchPagesPlugin from './plugins/watch-pages-plugin' | |
import WatchRemoveEventPlugin from './plugins/watch-remove-event-plugin' | ||
import DynamicEntryPlugin from './plugins/dynamic-entry-plugin' | ||
import DetachPlugin from './plugins/detach-plugin' | ||
import JsonPagesPlugin from './plugins/json-pages-plugin' | ||
import getConfig from '../config' | ||
import { pagePathToJson } from '../../lib/utils' | ||
|
||
const documentPage = join('pages', '_document.js') | ||
const defaultPages = [ | ||
|
@@ -32,7 +34,7 @@ export default async function createCompiler (dir, { dev = false, quiet = false | |
const defaultEntries = dev | ||
? [join(__dirname, '..', '..', 'client/webpack-hot-middleware-client')] : [] | ||
for (const p of pages) { | ||
entry[join('bundles', p)] = defaultEntries.concat([`./${p}?entry`]) | ||
entry[pagePathToJson(join('bundles', p))] = defaultEntries.concat([`./${p}?entry`]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think, ideally, extensions should be changed on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. That's not hard. I think that makes sense. I'll do it. |
||
} | ||
|
||
const nextPagesDir = join(__dirname, '..', '..', 'pages') | ||
|
@@ -42,7 +44,7 @@ export default async function createCompiler (dir, { dev = false, quiet = false | |
const entryName = join('bundles', 'pages', p) | ||
const path = join(nextPagesDir, p) | ||
if (!entry[entryName]) { | ||
entry[entryName] = defaultEntries.concat([path + '?entry']) | ||
entry[pagePathToJson(entryName)] = defaultEntries.concat([path + '?entry']) | ||
} | ||
interpolateNames.set(path, `dist/pages/${p}`) | ||
} | ||
|
@@ -69,7 +71,8 @@ export default async function createCompiler (dir, { dev = false, quiet = false | |
name: 'commons', | ||
filename: 'commons.js', | ||
minChunks: Math.max(2, minChunks) | ||
}) | ||
}), | ||
new JsonPagesPlugin() | ||
] | ||
|
||
if (dev) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import fs from 'mz/fs' | ||
import resolve from './resolve' | ||
|
||
/** | ||
* resolve a JSON page like `require.resolve`, | ||
* and read and cache the file content | ||
*/ | ||
|
||
async function readPage (path) { | ||
const f = await resolve(path) | ||
if (cache.hasOwnProperty(f)) { | ||
return cache[f] | ||
} | ||
|
||
const source = await fs.readFile(f, 'utf8') | ||
const { component } = JSON.parse(source) | ||
|
||
cache[f] = component | ||
return component | ||
} | ||
|
||
export default readPage | ||
export const cache = {} | ||
|
||
readPage.cache = cache |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
import { join } from 'path' | ||
import { createElement } from 'react' | ||
import { renderToString, renderToStaticMarkup } from 'react-dom/server' | ||
import fs from 'mz/fs' | ||
import send from 'send' | ||
import accepts from 'accepts' | ||
import requireModule from './require' | ||
import read from './read' | ||
import readPage from './read-page' | ||
import { Router } from '../lib/router' | ||
import Head, { defaultHead } from '../lib/head' | ||
import App from '../lib/app' | ||
|
@@ -48,8 +51,8 @@ async function doRender (req, res, pathname, query, { | |
errorComponent | ||
] = await Promise.all([ | ||
Component.getInitialProps ? Component.getInitialProps(ctx) : {}, | ||
read(join(dir, '.next', 'bundles', 'pages', page)), | ||
read(join(dir, '.next', 'bundles', 'pages', dev ? '_error-debug' : '_error')) | ||
readPage(join(dir, '.next', 'bundles', 'pages', page)), | ||
readPage(join(dir, '.next', 'bundles', 'pages', dev ? '_error-debug' : '_error')) | ||
]) | ||
|
||
// the response might be finshed on the getinitialprops call | ||
|
@@ -93,14 +96,15 @@ async function doRender (req, res, pathname, query, { | |
return '<!DOCTYPE html>' + renderToStaticMarkup(doc) | ||
} | ||
|
||
export async function renderJSON (res, page, { dir = process.cwd() } = {}) { | ||
const component = await read(join(dir, '.next', 'bundles', 'pages', page)) | ||
sendJSON(res, { component }) | ||
export async function renderJSON (req, res, page, { dir = process.cwd() } = {}) { | ||
const pagePath = join(dir, '.next', 'bundles', 'pages', `${page}.json`) | ||
return serveStaticWithGzip(req, res, pagePath) | ||
} | ||
|
||
export async function renderErrorJSON (err, res, { dir = process.cwd(), dev = false } = {}) { | ||
export async function renderErrorJSON (err, req, res, { dir = process.cwd(), dev = false } = {}) { | ||
const page = err && dev ? '/_error-debug' : '/_error' | ||
const component = await read(join(dir, '.next', 'bundles', 'pages', page)) | ||
const component = await readPage(join(dir, '.next', 'bundles', 'pages', page)) | ||
|
||
sendJSON(res, { | ||
component, | ||
err: err && dev ? errorToJSON(err) : null | ||
|
@@ -132,3 +136,36 @@ function errorToJSON (err) { | |
|
||
return json | ||
} | ||
|
||
export async function serveStaticWithGzip (req, res, path) { | ||
const encoding = accepts(req).encodings(['gzip']) | ||
if (encoding !== 'gzip') { | ||
return serveStatic(req, res, path) | ||
} | ||
|
||
const gzipPath = `${path}.gz` | ||
const exists = await fs.exists(gzipPath) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a deprecated API. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah! Seems like I need to read along the Node.js API again :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like I'll work on with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nkzawa Check now. |
||
if (!exists) { | ||
return serveStatic(req, res, path) | ||
} | ||
|
||
res.setHeader('Content-Encoding', 'gzip') | ||
return serveStatic(req, res, gzipPath) | ||
} | ||
|
||
export function serveStatic (req, res, path) { | ||
return new Promise((resolve, reject) => { | ||
send(req, path) | ||
.on('error', (err) => { | ||
if (err.code === 'ENOENT') { | ||
res.statusCode = 404 | ||
res.end('Not Found') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this change is acceptable, but it'd be better if we can show the original 404. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the orginal 404, it sends a bunch of HTML too. It's okay for pages, but this is just a JSON endpoint. We don't need that. (Since user won't access these files directly) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not only for JSON endpoint, but it's also for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll work a separate PR for that. |
||
resolve() | ||
} else { | ||
reject(err) | ||
} | ||
}) | ||
.pipe(res) | ||
.on('finish', resolve) | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nkzawa
I had to do this because now these files routes have
.json
extension.Do you know why this has changed?
Is there any better way to deal with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change
server/hot-reloader.js
instead of this file. MaybetoRoute
method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.