Skip to content

Commit

Permalink
Update check for fallback pages during export (#33323)
Browse files Browse the repository at this point in the history
This fixes our check for fallback pages during `next export` as we are currently erroring even when the erroneous pages are not included in the `exportPathMap`. This also adds additional tests to certify the expected behavior for the error. 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [x] Errors have helpful link attached, see `contributing.md`

Fixes: #29135
  • Loading branch information
ijjk authored Jan 17, 2022
1 parent aaa77dd commit 3894320
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 26 deletions.
12 changes: 5 additions & 7 deletions packages/next/export/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,12 @@ export default async function exportApp(
if (prerenderManifest && !options.buildExport) {
const fallbackEnabledPages = new Set()

for (const key of Object.keys(prerenderManifest.dynamicRoutes)) {
// only error if page is included in path map
if (!exportPathMap[key] && !excludedPrerenderRoutes.has(key)) {
continue
}
for (const path of Object.keys(exportPathMap)) {
const page = exportPathMap[path].page
const prerenderInfo = prerenderManifest.dynamicRoutes[page]

if (prerenderManifest.dynamicRoutes[key].fallback !== false) {
fallbackEnabledPages.add(key)
if (prerenderInfo && prerenderInfo.fallback !== false) {
fallbackEnabledPages.add(page)
}
}

Expand Down
7 changes: 7 additions & 0 deletions test/integration/export-fallback-true-error/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
exportPathMap() {
return {
'/first': { page: '/[slug]' },
}
},
}
3 changes: 3 additions & 0 deletions test/lib/next-modes/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ export class NextInstance {
}
}

public async export(): Promise<{ exitCode?: number; cliOutput?: string }> {
return {}
}
public async setup(): Promise<void> {}
public async start(): Promise<void> {}
public async stop(): Promise<void> {
Expand Down
77 changes: 58 additions & 19 deletions test/lib/next-modes/next-start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { spawn, SpawnOptions } from 'child_process'
export class NextStartInstance extends NextInstance {
private _buildId: string
private _cliOutput: string
private spawnOpts: SpawnOptions

public get buildId() {
return this._buildId
Expand All @@ -19,11 +20,26 @@ export class NextStartInstance extends NextInstance {
await super.createTestDir()
}

private handleStdio = (childProcess) => {
childProcess.stdout.on('data', (chunk) => {
const msg = chunk.toString()
process.stdout.write(chunk)
this._cliOutput += msg
this.emit('stdout', [msg])
})
childProcess.stderr.on('data', (chunk) => {
const msg = chunk.toString()
process.stderr.write(chunk)
this._cliOutput += msg
this.emit('stderr', [msg])
})
}

public async start() {
if (this.childProcess) {
throw new Error('next already started')
}
const spawnOpts: SpawnOptions = {
this.spawnOpts = {
cwd: this.testDir,
stdio: ['ignore', 'pipe', 'pipe'],
shell: false,
Expand All @@ -34,20 +50,6 @@ export class NextStartInstance extends NextInstance {
__NEXT_RAND_PORT: '1',
},
}
const handleStdio = () => {
this.childProcess.stdout.on('data', (chunk) => {
const msg = chunk.toString()
process.stdout.write(chunk)
this._cliOutput += msg
this.emit('stdout', [msg])
})
this.childProcess.stderr.on('data', (chunk) => {
const msg = chunk.toString()
process.stderr.write(chunk)
this._cliOutput += msg
this.emit('stderr', [msg])
})
}
let buildArgs = ['yarn', 'next', 'build']
let startArgs = ['yarn', 'next', 'start']

Expand All @@ -60,8 +62,12 @@ export class NextStartInstance extends NextInstance {

await new Promise<void>((resolve, reject) => {
console.log('running', buildArgs.join(' '))
this.childProcess = spawn(buildArgs[0], buildArgs.slice(1), spawnOpts)
handleStdio()
this.childProcess = spawn(
buildArgs[0],
buildArgs.slice(1),
this.spawnOpts
)
this.handleStdio(this.childProcess)
this.childProcess.on('exit', (code, signal) => {
if (code || signal)
reject(
Expand All @@ -85,8 +91,12 @@ export class NextStartInstance extends NextInstance {
console.log('running', startArgs.join(' '))

await new Promise<void>((resolve) => {
this.childProcess = spawn(startArgs[0], startArgs.slice(1), spawnOpts)
handleStdio()
this.childProcess = spawn(
startArgs[0],
startArgs.slice(1),
this.spawnOpts
)
this.handleStdio(this.childProcess)

this.childProcess.on('close', (code, signal) => {
if (this.isStopping) return
Expand All @@ -108,4 +118,33 @@ export class NextStartInstance extends NextInstance {
this.on('stdout', readyCb)
})
}

public async export() {
return new Promise((resolve) => {
const curOutput = this._cliOutput.length
const exportArgs = ['yarn', 'next', 'export']

if (this.childProcess) {
throw new Error(
`can not run export while server is running, use next.stop() first`
)
}
console.log('running', exportArgs.join(' '))

this.childProcess = spawn(
exportArgs[0],
exportArgs.slice(1),
this.spawnOpts
)
this.handleStdio(this.childProcess)

this.childProcess.on('exit', (code, signal) => {
this.childProcess = undefined
resolve({
exitCode: signal || code,
cliOutput: this.cliOutput.substr(curOutput),
})
})
})
}
}
89 changes: 89 additions & 0 deletions test/production/fallback-export-error/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { renderViaHTTP } from 'next-test-utils'
import { join } from 'path'

describe('fallback export error', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: {
pages: new FileRef(join(__dirname, 'pages')),
},
})
})
afterAll(() => next.destroy())

it('should have built', async () => {
const html = await renderViaHTTP(next.url, '/')
expect(html).toContain('index page')
})

it('should not error with default exportPathMap', async () => {
await next.stop()

const result = await next.export()
console.log(result.cliOutput)

expect(result.exitCode).toBe(0)
expect(result.cliOutput).not.toContain(
'Found pages with `fallback` enabled'
)
})

it('should not error with valid exportPathMap', async () => {
await next.stop()
await next.patchFile(
'next.config.js',
`
module.exports = {
exportPathMap() {
return {
'/': { page: '/' },
}
}
}
`
)

try {
const result = await next.export()
console.log(result.cliOutput)

expect(result.exitCode).toBe(0)
expect(result.cliOutput).not.toContain(
'Found pages with `fallback` enabled'
)
} finally {
await next.deleteFile('next.config.js')
}
})

it('should error with invalid exportPathMap', async () => {
await next.stop()
await next.patchFile(
'next.config.js',
`
module.exports = {
exportPathMap() {
return {
'/': { page: '/' },
'/second': { page: '/[...slug]' }
}
}
}
`
)

try {
const result = await next.export()
console.log(result.cliOutput)

expect(result.exitCode).toBe(1)
expect(result.cliOutput).toContain('Found pages with `fallback` enabled')
} finally {
await next.deleteFile('next.config.js')
}
})
})
18 changes: 18 additions & 0 deletions test/production/fallback-export-error/pages/[...slug].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export function getStaticProps() {
return {
props: {
now: Date.now(),
},
}
}

export function getStaticPaths() {
return {
paths: ['/first'],
fallback: 'blocking',
}
}

export default function Page() {
return <p>catch-all page</p>
}
3 changes: 3 additions & 0 deletions test/production/fallback-export-error/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>index page</p>
}

0 comments on commit 3894320

Please sign in to comment.