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

Clean up landed experimental flags #10593

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cbfac04
Clean up landed experimental flags
timneutkens Feb 19, 2020
9ee3710
Remove check for experimental flags from build too
timneutkens Feb 19, 2020
758fef9
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
940ecfa
Merge branch 'fix/remove-landed-experimental-options' of github.com:t…
timneutkens Feb 19, 2020
63baa82
Remove /_errors/404 in favor of /404
timneutkens Feb 19, 2020
2d7e0fc
Remove unneeded check for pathname
timneutkens Feb 19, 2020
c4149fd
Update test paths
timneutkens Feb 19, 2020
8bee4e4
Fix test
timneutkens Feb 19, 2020
403982c
Update test
timneutkens Feb 19, 2020
7949c20
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
ac869bd
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
f9bfc3d
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
65ed154
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
d94eae3
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
778f764
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
554771a
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
e508246
Remove test for disabled config
timneutkens Feb 19, 2020
ea38c54
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
2964ba9
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
58e6678
Merge branch 'canary' into fix/remove-landed-experimental-options
Timer Feb 19, 2020
fcbe670
Merge branch 'fix/remove-landed-experimental-options' of github.com:t…
timneutkens Feb 19, 2020
9dd7d30
Set pages404 always to true
timneutkens Feb 19, 2020
15216b4
Merge branch 'canary' into fix/remove-landed-experimental-options
ijjk Feb 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 10 additions & 10 deletions packages/next/build/index.ts
Expand Up @@ -221,10 +221,9 @@ export default async function build(dir: string, conf = null): Promise<void> {
const hasCustomErrorPage = mappedPages['/_error'].startsWith(
'private-next-pages'
)
const hasPages404 =
config.experimental.pages404 &&
mappedPages['/404'] &&
mappedPages['/404'].startsWith('private-next-pages')
const hasPages404 = Boolean(
mappedPages['/404'] && mappedPages['/404'].startsWith('private-next-pages')
)

if (hasPublicDir) {
try {
Expand Down Expand Up @@ -285,7 +284,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
const routesManifestPath = path.join(distDir, ROUTES_MANIFEST)
const routesManifest: any = {
version: 1,
pages404: !!hasPages404,
pages404: true,
basePath: config.experimental.basePath,
redirects: redirects.map(r => buildCustomRoute(r, 'redirect')),
rewrites: rewrites.map(r => buildCustomRoute(r, 'rewrite')),
Expand Down Expand Up @@ -606,8 +605,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
// Since custom _app.js can wrap the 404 page we have to opt-out of static optimization if it has getInitialProps
// Only export the static 404 when there is no /_error present
const useStatic404 =
!customAppGetInitialProps &&
((!hasCustomErrorPage && config.experimental.static404) || hasPages404)
!customAppGetInitialProps && (!hasCustomErrorPage || hasPages404)

if (invalidPages.size > 0) {
throw new Error(
Expand Down Expand Up @@ -681,7 +679,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
})

if (useStatic404) {
defaultMap['/_errors/404'] = {
defaultMap['/404'] = {
page: hasPages404 ? '/404' : '/_error',
}
}
Expand All @@ -690,6 +688,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
},
exportTrailingSlash: false,
}

await exportApp(dir, exportOptions, exportConfig)

// remove server bundles that were exported
Expand Down Expand Up @@ -726,8 +725,9 @@ export default async function build(dir: string, conf = null): Promise<void> {
await fsMove(orig, dest)
}

if (useStatic404) {
await moveExportedPage('/_errors/404', '/_errors/404', false, 'html')
// Only move /404 to /404 when there is no custom 404 as in that case we don't know about the 404 page
if (!hasPages404 && useStatic404) {
await moveExportedPage('/404', '/404', false, 'html')
}

for (const page of combinedPages) {
Expand Down
2 changes: 0 additions & 2 deletions packages/next/next-server/server/config.ts
Expand Up @@ -52,8 +52,6 @@ const defaultConfig: { [key: string]: any } = {
reactMode: 'legacy',
workerThreads: false,
basePath: '',
static404: true,
pages404: true,
},
future: {
excludeDefaultMomentLocales: false,
Expand Down
22 changes: 4 additions & 18 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -114,7 +114,6 @@ export default class Server {
documentMiddlewareEnabled: boolean
hasCssMode: boolean
dev?: boolean
pages404?: boolean
}
private compression?: Middleware
private onErrorMiddleware?: ({ err }: { err: Error }) => Promise<void>
Expand Down Expand Up @@ -162,7 +161,6 @@ export default class Server {
staticMarkup,
buildId: this.buildId,
generateEtags,
pages404: this.nextConfig.experimental.pages404,
}

// Only the `publicRuntimeConfig` key is exposed to the client side
Expand Down Expand Up @@ -866,7 +864,7 @@ export default class Server {
opts: any
): Promise<string | null> {
// we need to ensure the status code if /404 is visited directly
if (this.nextConfig.experimental.pages404 && pathname === '/404') {
if (pathname === '/404') {
res.statusCode = 404
}

Expand Down Expand Up @@ -1171,22 +1169,13 @@ export default class Server {
) {
let result: null | FindComponentsResult = null

const { static404, pages404 } = this.nextConfig.experimental
const is404 = res.statusCode === 404
let using404Page = false

// use static 404 page if available and is 404 response
if (is404) {
if (static404) {
result = await this.findPageComponents('/_errors/404')
}

// use 404 if /_errors/404 isn't available which occurs
// during development and when _app has getInitialProps
if (!result && pages404) {
result = await this.findPageComponents('/404')
using404Page = result !== null
}
result = await this.findPageComponents('/404')
using404Page = result !== null
}

if (!result) {
Expand Down Expand Up @@ -1220,11 +1209,8 @@ export default class Server {
): Promise<void> {
const url: any = req.url
const { pathname, query } = parsedUrl ? parsedUrl : parseUrl(url, true)
if (!pathname) {
throw new Error('pathname is undefined')
}
res.statusCode = 404
return this.renderError(null, req, res, pathname, query)
return this.renderError(null, req, res, pathname!, query)
}

public async serveStatic(
Expand Down
4 changes: 1 addition & 3 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -145,7 +145,6 @@ type RenderOpts = LoadComponentsReturnType & {
documentMiddlewareEnabled?: boolean
isDataReq?: boolean
params?: ParsedUrlQuery
pages404?: boolean
previewProps: __ApiPreviewProps
}

Expand Down Expand Up @@ -278,7 +277,6 @@ export async function renderToHTML(
unstable_getServerProps,
isDataReq,
params,
pages404,
previewProps,
} = renderOpts

Expand Down Expand Up @@ -391,7 +389,7 @@ export async function renderToHTML(
renderOpts.nextExport = true
}

if (pages404 && pathname === '/404' && !isAutoExport) {
if (pathname === '/404' && !isAutoExport) {
throw new Error(PAGES_404_GET_INITIAL_PROPS_ERROR)
}
}
Expand Down
12 changes: 5 additions & 7 deletions packages/next/server/next-dev-server.ts
Expand Up @@ -463,13 +463,11 @@ export default class DevServer extends Server {
})
} catch (err) {
if (err.code === 'ENOENT') {
if (this.nextConfig.experimental.pages404) {
try {
await this.hotReloader!.ensurePage('/404')
} catch (err) {
if (err.code !== 'ENOENT') {
throw err
}
try {
await this.hotReloader!.ensurePage('/404')
} catch (err) {
if (err.code !== 'ENOENT') {
throw err
}
}

Expand Down
6 changes: 1 addition & 5 deletions test/integration/404-page/next.config.js
@@ -1,5 +1 @@
module.exports = {
experimental: {
pages404: true,
},
}
module.exports = {}
15 changes: 6 additions & 9 deletions test/integration/404-page/test/index.test.js
Expand Up @@ -49,7 +49,7 @@ const runTests = (mode = 'server') => {
})

if (mode !== 'dev') {
it('should output _errors/404.html during build', async () => {
it('should output 404.html during build', async () => {
expect(
await fs.exists(
join(
Expand All @@ -58,17 +58,17 @@ const runTests = (mode = 'server') => {
mode === 'serverless'
? 'serverless/pages'
: `server/static/${buildId}/pages`,
'_errors/404.html'
'404.html'
)
)
).toBe(true)
})

it('should add _errors/404 to pages-manifest correctly', async () => {
it('should add /404 to pages-manifest correctly', async () => {
const manifest = await fs.readJSON(
join(appDir, '.next', mode, 'pages-manifest.json')
)
expect('/_errors/404' in manifest).toBe(true)
expect('/404' in manifest).toBe(true)
})
}
}
Expand Down Expand Up @@ -104,10 +104,7 @@ describe('404 Page Support', () => {
nextConfig,
`
module.exports = {
target: 'serverless',
experimental: {
pages404: true
}
target: 'serverless'
}
`
)
Expand Down Expand Up @@ -212,7 +209,7 @@ describe('404 Page Support', () => {
it('should not output static 404 if _app has getInitialProps', async () => {
expect(
await fs.exists(
join(appDir, '.next/server/static', buildId, 'pages/_errors/404.html')
join(appDir, '.next/server/static', buildId, 'pages/404.html')
)
).toBe(false)
})
Expand Down
2 changes: 1 addition & 1 deletion test/integration/custom-routes/test/index.test.js
Expand Up @@ -344,7 +344,7 @@ const runTests = (isDev = false) => {

expect(manifest).toEqual({
version: 1,
pages404: false,
pages404: true,
basePath: '',
redirects: [
{
Expand Down
2 changes: 1 addition & 1 deletion test/integration/dynamic-routing/test/index.test.js
Expand Up @@ -487,7 +487,7 @@ function runTests(dev) {

expect(manifest).toEqual({
version: 1,
pages404: false,
pages404: true,
basePath: '',
headers: [],
rewrites: [],
Expand Down
21 changes: 3 additions & 18 deletions test/integration/static-404/test/index.test.js
Expand Up @@ -13,10 +13,7 @@ import {
jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2
const appDir = join(__dirname, '..')
const nextConfig = join(appDir, 'next.config.js')
const static404 = join(
appDir,
'.next/server/static/test-id/pages/_errors/404.html'
)
const static404 = join(appDir, '.next/server/static/test-id/pages/404.html')
const appPage = join(appDir, 'pages/_app.js')
const errorPage = join(appDir, 'pages/_error.js')
const buildId = `generateBuildId: () => 'test-id'`
Expand All @@ -31,17 +28,6 @@ describe('Static 404 page', () => {
})
beforeEach(() => fs.remove(join(appDir, '.next/server')))

describe('With config disabled', () => {
it('should not have exported static 404 page', async () => {
await fs.writeFile(
nextConfig,
`module.exports = { ${buildId}, experimental: { static404: false } }`
)
await nextBuild(appDir)
expect(await fs.exists(static404)).toBe(false)
})
})

describe('With config enabled', () => {
beforeEach(() =>
fs.writeFile(nextConfig, `module.exports = { ${buildId} }`)
Expand All @@ -62,8 +48,7 @@ describe('Static 404 page', () => {
nextConfig,
`
module.exports = {
target: 'experimental-serverless-trace',
experimental: { static404: true }
target: 'experimental-serverless-trace'
}
`
)
Expand All @@ -74,7 +59,7 @@ describe('Static 404 page', () => {
await killApp(app)
expect(html).toContain('This page could not be found')
expect(
await fs.exists(join(appDir, '.next/serverless/pages/_errors/404.html'))
await fs.exists(join(appDir, '.next/serverless/pages/404.html'))
).toBe(true)
})

Expand Down