Skip to content

Commit

Permalink
Remove buildId from server-side files (#14413)
Browse files Browse the repository at this point in the history
Gets rid of the custom function for naming files by removing buildId from the file paths.
  • Loading branch information
timneutkens authored Jun 22, 2020
1 parent a7af013 commit 6c2ce70
Show file tree
Hide file tree
Showing 30 changed files with 269 additions and 388 deletions.
6 changes: 3 additions & 3 deletions packages/next/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export function createEntrypoints(
const isApiRoute = page.match(API_ROUTE)

const clientBundlePath = join('static', 'pages', bundleFile)
const serverBundlePath = join('static', 'BUILD_ID', 'pages', bundleFile)
const serverBundlePath = join('pages', bundleFile)

const isLikeServerless = isTargetLikeServerless(target)

Expand All @@ -111,7 +111,7 @@ export function createEntrypoints(
absolutePagePath,
...defaultServerlessOptions,
}
server[join('pages', bundleFile)] = `next-serverless-loader?${stringify(
server[serverBundlePath] = `next-serverless-loader?${stringify(
serverlessLoaderOptions
)}!`
} else if (isApiRoute || target === 'server') {
Expand All @@ -122,7 +122,7 @@ export function createEntrypoints(
absolutePagePath,
...defaultServerlessOptions,
}
server[join('pages', bundleFile)] = `next-serverless-loader?${stringify(
server[serverBundlePath] = `next-serverless-loader?${stringify(
serverlessLoaderOptions
)}!`
}
Expand Down
20 changes: 2 additions & 18 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -679,24 +679,8 @@ export default async function getBaseWebpackConfig(
},
output: {
path: outputPath,
filename: isServer
? ({ chunk }: { chunk: { name: string } }) => {
// Use `[name]-[contenthash].js` in production
if (chunk.name.includes('BUILD_ID')) {
return (
escapePathVariables(chunk.name).replace(
'BUILD_ID',
isServer || dev ? buildId : '[contenthash]'
) + '.js'
)
}

return '[name].js'
}
: // Client compilation only
dev
? '[name].js'
: '[name]-[chunkhash].js',
// On the server we don't use the chunkhash
filename: dev || isServer ? '[name].js' : '[name]-[chunkhash].js',
libraryTarget: isServer ? 'commonjs2' : 'var',
hotUpdateChunkFilename: isWebpack5
? 'static/webpack/[id].[fullhash].hot-update.js'
Expand Down
23 changes: 15 additions & 8 deletions packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { loadEnvConfig } from '../lib/load-env-config'
import { PrerenderManifest } from '../build'
import type exportPage from './worker'
import { PagesManifest } from '../build/webpack/plugins/pages-manifest-plugin'
import { getPagePath } from '../next-server/server/require'

const exists = promisify(existsOrig)

Expand Down Expand Up @@ -157,14 +158,6 @@ export default async function exportApp(
prerenderManifest = require(join(distDir, PRERENDER_MANIFEST))
} catch (_) {}

const distPagesDir = join(
distDir,
isLikeServerless
? SERVERLESS_DIRECTORY
: join(SERVER_DIRECTORY, 'static', buildId),
'pages'
)

const excludedPrerenderRoutes = new Set<string>()
const pages = options.pages || Object.keys(pagesManifest)
const defaultPathMap: ExportPathMap = {}
Expand Down Expand Up @@ -425,7 +418,21 @@ export default async function exportApp(
if (!options.buildExport && prerenderManifest) {
await Promise.all(
Object.keys(prerenderManifest.routes).map(async (route) => {
const { srcRoute } = prerenderManifest!.routes[route]
const pageName = srcRoute || route
const pagePath = getPagePath(pageName, distDir, isLikeServerless)
const distPagesDir = join(
pagePath,
// strip leading / and then recurse number of nested dirs
// to place from base folder
pageName
.substr(1)
.split('/')
.map(() => '..')
.join('/')
)
route = normalizePagePath(route)

const orig = join(distPagesDir, route)
const htmlDest = join(
outDir,
Expand Down
12 changes: 5 additions & 7 deletions packages/next/next-server/server/get-route-from-entrypoint.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { denormalizePagePath } from './normalize-page-path'

// matches static/<buildid>/pages/:page*.js
const ROUTE_NAME_REGEX = /^static[/\\][^/\\]+[/\\]pages[/\\](.*)$/
// const SERVER_ROUTE_NAME_REGEX = /^static[/\\][^/\\]+[/\\]pages[/\\](.*)$/
// matches pages/:page*.js
const SERVERLESS_ROUTE_NAME_REGEX = /^pages[/\\](.*)$/
const SERVER_ROUTE_NAME_REGEX = /^pages[/\\](.*)$/
// matches static/pages/:page*.js
const BROWSER_ROUTE_NAME_REGEX = /^static[/\\]pages[/\\](.*)$/

Expand All @@ -19,12 +19,10 @@ function matchBundle(regex: RegExp, input: string): string | null {

export default function getRouteFromEntrypoint(
entryFile: string,
isServerlessLike: boolean = false
// TODO: Remove this parameter
_isServerlessLike: boolean = false
): string | null {
let pagePath = matchBundle(
isServerlessLike ? SERVERLESS_ROUTE_NAME_REGEX : ROUTE_NAME_REGEX,
entryFile
)
let pagePath = matchBundle(SERVER_ROUTE_NAME_REGEX, entryFile)

if (pagePath) {
return pagePath
Expand Down
4 changes: 1 addition & 3 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,7 @@ export default class Server {
distDir: this.distDir,
pagesDir: join(
this.distDir,
this._isLikeServerless
? SERVERLESS_DIRECTORY
: `${SERVER_DIRECTORY}/static/${this.buildId}`,
this._isLikeServerless ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY,
'pages'
),
flushToDisk: this.nextConfig.experimental.sprFlushToDisk,
Expand Down
2 changes: 1 addition & 1 deletion packages/next/server/hot-reloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ export default class HotReloader {
// We only watch `_document` for changes on the server compilation
// the rest of the files will be triggered by the client compilation
const documentChunk = compilation.chunks.find(
(c) => c.name === normalize(`static/BUILD_ID/pages/_document`)
(c) => c.name === normalize(`pages/_document`)
)
// If the document chunk can't be found we do nothing
if (!documentChunk) {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/server/on-demand-entry-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export default function onDemandEntryHandler(
pageUrl = pageUrl === '' ? '/' : pageUrl

const bundleFile = normalizePagePath(pageUrl)
const serverBundlePath = join('static', 'BUILD_ID', 'pages', bundleFile)
const serverBundlePath = join('pages', bundleFile)
const clientBundlePath = join('static', 'pages', bundleFile)
const absolutePagePath = pagePath.startsWith('next/dist/pages')
? require.resolve(pagePath)
Expand Down
1 change: 1 addition & 0 deletions test/integration/404-page-app/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {}
5 changes: 5 additions & 0 deletions test/integration/404-page-app/pages/404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const page = (props) => {
return <h1 id="404-title">{props.extraProp}</h1>
}

export default page
18 changes: 18 additions & 0 deletions test/integration/404-page-app/pages/_app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import App from 'next/app'

function MyApp({ Component, pageProps, extraProp }) {
return <Component {...pageProps} extraProp={extraProp} />
}

// Only uncomment this method if you have blocking data requirements for
// every single page in your application. This disables the ability to
// perform automatic static optimization, causing every page in your app to
// be server-side rendered.
MyApp.getInitialProps = async (appContext) => {
// calls page's `getInitialProps` and fills `appProps.pageProps`
const appProps = await App.getInitialProps(appContext)

return { ...appProps, extraProp: 'Hi There' }
}

export default MyApp
5 changes: 5 additions & 0 deletions test/integration/404-page-app/pages/err.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const page = () => 'custom 404 page'
page.getInitialProps = () => {
throw new Error('oops')
}
export default page
1 change: 1 addition & 0 deletions test/integration/404-page-app/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hello from index'
89 changes: 89 additions & 0 deletions test/integration/404-page-app/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/* eslint-env jest */

import fs from 'fs-extra'
import { join } from 'path'
import {
killApp,
findPort,
launchApp,
nextStart,
nextBuild,
fetchViaHTTP,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
import cheerio from 'cheerio'

jest.setTimeout(1000 * 60 * 2)

const appDir = join(__dirname, '../')
const gip404Err = /`pages\/404` can not have getInitialProps\/getServerSideProps/

let appPort
let app

describe('404 Page Support with _app', () => {
describe('production mode', () => {
afterAll(() => killApp(app))

it('should build successfully', async () => {
const { code, stderr, stdout } = await nextBuild(appDir, [], {
stderr: true,
stdout: true,
})

expect(code).toBe(0)
expect(stderr).not.toMatch(gip404Err)
expect(stdout).not.toMatch(gip404Err)

appPort = await findPort()
app = await nextStart(appDir, appPort)
})

it('should not output static 404 if _app has getInitialProps', async () => {
const browser = await webdriver(appPort, '/404')
const isAutoExported = await browser.eval('__NEXT_DATA__.autoExport')
expect(isAutoExported).toBe(null)
})

it('specify to use the 404 page still in the routes-manifest', async () => {
const manifest = await fs.readJSON(
join(appDir, '.next/routes-manifest.json')
)
expect(manifest.pages404).toBe(true)
})

it('should still use 404 page', async () => {
const res = await fetchViaHTTP(appPort, '/abc')
expect(res.status).toBe(404)
const $ = cheerio.load(await res.text())
expect($('#404-title').text()).toBe('Hi There')
})
})

describe('dev mode', () => {
let stderr = ''
let stdout = ''

beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort, {
onStderr(msg) {
stderr += msg
},
onStdout(msg) {
stdout += msg
},
})
})
afterAll(() => killApp(app))

it('should not show pages/404 GIP error if _app has GIP', async () => {
const res = await fetchViaHTTP(appPort, '/abc')
expect(res.status).toBe(404)
const $ = cheerio.load(await res.text())
expect($('#404-title').text()).toBe('Hi There')
expect(stderr).not.toMatch(gip404Err)
expect(stdout).not.toMatch(gip404Err)
})
})
})
21 changes: 3 additions & 18 deletions test/integration/404-page-custom-error/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
nextBuild,
renderViaHTTP,
fetchViaHTTP,
getPageFileFromPagesManifest,
} from 'next-test-utils'

jest.setTimeout(1000 * 60 * 2)
Expand All @@ -18,7 +19,6 @@ const appDir = join(__dirname, '../')
const nextConfig = join(appDir, 'next.config.js')

let appPort
let buildId
let app

const runTests = (mode) => {
Expand Down Expand Up @@ -47,21 +47,8 @@ const runTests = (mode) => {
})

it('should have output 404.html', async () => {
expect(
await fs
.access(
join(
appDir,
'.next',
...(mode === 'server'
? ['server', 'static', buildId, 'pages']
: ['serverless', 'pages']),
'404.html'
)
)
.then(() => true)
.catch(() => false)
).toBe(true)
const page = getPageFileFromPagesManifest(appDir, '/404')
expect(page.endsWith('.html')).toBe(true)
})
}
}
Expand All @@ -81,7 +68,6 @@ describe('Default 404 Page with custom _error', () => {
appPort = await findPort()

app = await nextStart(appDir, appPort)
buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
})

runTests('server')
Expand Down Expand Up @@ -109,7 +95,6 @@ describe('Default 404 Page with custom _error', () => {

appPort = await findPort()
app = await nextStart(appDir, appPort)
buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
})

runTests('serverless')
Expand Down
Loading

0 comments on commit 6c2ce70

Please sign in to comment.