Skip to content

Commit

Permalink
Clarify app<->pages file conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
timneutkens committed Nov 3, 2022
1 parent 4af5011 commit 554a06a
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 37 deletions.
58 changes: 32 additions & 26 deletions packages/next/build/index.ts
Expand Up @@ -576,36 +576,42 @@ export default async function build(
})
)

const pageKeys = {
pages: Object.keys(mappedPages),
app: mappedAppPages
? Object.keys(mappedAppPages).map(
(key) => normalizeAppPath(key) || '/'
)
: undefined,
}

if (pageKeys.app) {
const conflictingAppPagePaths = []

for (const appPath of pageKeys.app) {
if (pageKeys.pages.includes(appPath)) {
conflictingAppPagePaths.push(appPath)
const pagesPageKeys = Object.keys(mappedPages)

const conflictingAppPagePaths: [pagePath: string, appPath: string][] = []
const appPageKeys: string[] = []
if (mappedAppPages) {
for (const appKey in mappedAppPages) {
const normalizedAppPageKey = normalizeAppPath(appKey) || '/'
const pagePath = mappedPages[normalizedAppPageKey]
if (pagePath) {
const appPath = mappedAppPages[appKey]
conflictingAppPagePaths.push([
pagePath.replace(/^private-next-pages/, 'pages'),
appPath.replace(/^private-next-app-dir/, 'app'),
])
}

appPageKeys.push(normalizedAppPageKey)
}
const numConflicting = conflictingAppPagePaths.length
}

if (numConflicting > 0) {
Log.error(
`Conflicting app and page file${
numConflicting === 1 ? ' was' : 's were'
} found, please remove the conflicting files to continue:`
)
for (const p of conflictingAppPagePaths) {
Log.error(` "pages${p}" - "app${p}"`)
}
process.exit(1)
const pageKeys = {
pages: pagesPageKeys,
app: appPageKeys.length > 0 ? appPageKeys : undefined,
}

const numConflicting = conflictingAppPagePaths.length
if (mappedAppPages && numConflicting > 0) {
Log.error(
`Conflicting app and page file${
numConflicting === 1 ? ' was' : 's were'
} found, please remove the conflicting files to continue:`
)
for (const [pagePath, appPath] of conflictingAppPagePaths) {
Log.error(` "${pagePath}" - "${appPath}"`)
}
process.exit(1)
}

const conflictingPublicFiles: string[] = []
Expand Down
18 changes: 13 additions & 5 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -323,7 +323,9 @@ export default class DevServer extends Server {
const appPaths: Record<string, string[]> = {}
const edgeRoutesSet = new Set<string>()
const pageNameSet = new Set<string>()
const conflictingAppPagePaths: string[] = []
const conflictingAppPagePaths = new Set<string>()
const appPageFilePaths = new Map<string, string>()
const pagesPageFilePaths = new Map<string, string>()

let envChange = false
let tsconfigChange = false
Expand Down Expand Up @@ -422,8 +424,13 @@ export default class DevServer extends Server {
pageName = pageName.replace(/\/index$/, '') || '/'
}

;(isAppPath ? appPageFilePaths : pagesPageFilePaths).set(
pageName,
fileName
)

if (pageNameSet.has(pageName)) {
conflictingAppPagePaths.push(pageName)
conflictingAppPagePaths.add(pageName)
} else {
pageNameSet.add(pageName)
}
Expand Down Expand Up @@ -451,17 +458,18 @@ export default class DevServer extends Server {
})
}

const numConflicting = conflictingAppPagePaths.length
const numConflicting = conflictingAppPagePaths.size
if (numConflicting > 0) {
Log.error(
`Conflicting app and page file${
numConflicting === 1 ? ' was' : 's were'
} found, please remove the conflicting files to continue:`
)
for (const p of conflictingAppPagePaths) {
Log.error(` "pages${p}" - "app${p}"`)
const appPath = relative(this.dir, appPageFilePaths.get(p)!)
const pagesPath = relative(this.dir, pagesPageFilePaths.get(p)!)
Log.error(` "${pagesPath}" - "${appPath}"`)
}
//process.exit(1)
}

if (!this.usingTypeScript && enabledTypeScript) {
Expand Down
3 changes: 3 additions & 0 deletions test/integration/conflicting-app-page-error/app/page.js
@@ -0,0 +1,3 @@
export default function Page(props) {
return <p>index page</p>
}
@@ -0,0 +1,3 @@
export default function Page() {
return <h1>Hello World!</h1>
}
24 changes: 18 additions & 6 deletions test/integration/conflicting-app-page-error/test/index.test.js
Expand Up @@ -21,14 +21,26 @@ function runTests({ dev }) {
it('should print error for conflicting app/page', async () => {
expect(output).toMatch(/Conflicting app and page files were found/)

for (const conflict of ['/hello', '/another']) {
expect(output).toContain(conflict)
for (const [pagePath, appPath] of [
['pages/index.js', 'app/page.js'],
['pages/hello.js', 'app/hello/page.js'],
['pages/another.js', 'app/another/page.js'],
]) {
expect(output).toContain(`"${pagePath}" - "${appPath}"`)
}
expect(output).not.toContain('/index')
expect(output).not.toContain('/non-conflict-pages')
expect(output).not.toContain('/non-conflict')
})

if (dev) {
it('should show error overlay for /', async () => {
const browser = await webdriver(appPort, '/')
expect(await hasRedbox(browser, true)).toBe(true)
expect(await getRedboxHeader(browser)).toContain(
'Conflicting app and page file found: "app/page.js" and "pages/index.js". Please remove one to continue.'
)
})

it('should show error overlay for /hello', async () => {
const browser = await webdriver(appPort, '/hello')
expect(await hasRedbox(browser, true)).toBe(true)
Expand All @@ -45,11 +57,11 @@ function runTests({ dev }) {
)
})

it('should not show error overlay for /', async () => {
const browser = await webdriver(appPort, '/')
it('should not show error overlay for /non-conflict-pages', async () => {
const browser = await webdriver(appPort, '/non-conflict-pages')
expect(await hasRedbox(browser, false)).toBe(false)
expect(await getRedboxHeader(browser)).toBe(null)
expect(await browser.elementByCss('p').text()).toBe('index page')
expect(await browser.elementByCss('h1').text()).toBe('Hello World')
})

it('should not show error overlay for /non-conflict', async () => {
Expand Down

0 comments on commit 554a06a

Please sign in to comment.