Print error when next dev has conflicting app & page (#41656)

This PR prints an error when the `app` and `pages` directory contain the
same route.

## build error
<img width="904" alt="image"
src="https://user-images.githubusercontent.com/229881/197424839-67dac580-1e1d-4c31-b769-112f2f38b06e.png">

## dev error
<img width="957" alt="image"
src="https://user-images.githubusercontent.com/229881/197426784-9b5041d9-b6d0-48d5-8ce5-b759bd9e4438.png">

Co-authored-by: JJ Kasper <jj@jjsweb.site>

packages/next/build/index.ts

@@ -586,7 +586,7 @@ export default async function build(
 
         for (const appPath of pageKeys.app) {
           if (pageKeys.pages.includes(appPath)) {
-            conflictingAppPagePaths.push(`pages${appPath} - app${appPath}`)
+            conflictingAppPagePaths.push(appPath)
           }
         }
         const numConflicting = conflictingAppPagePaths.length
@@ -595,10 +595,11 @@ export default async function build(
           Log.error(
             `Conflicting app and page file${
               numConflicting === 1 ? ' was' : 's were'
-            } found, please remove the conflicting files to continue. \n${conflictingAppPagePaths.join(
-              '\n'
-            )}\n`
+            } found, please remove the conflicting files to continue:`
           )
+          for (const p of conflictingAppPagePaths) {
+            Log.error(`  "pages${p}" - "app${p}"`)
+          }
           process.exit(1)
         }
       }

packages/next/server/dev/next-dev-server.ts

@@ -322,6 +322,8 @@ export default class DevServer extends Server {
         const knownFiles = wp.getTimeInfoEntries()
         const appPaths: Record<string, string[]> = {}
         const edgeRoutesSet = new Set<string>()
+        const pageNameSet = new Set<string>()
+        const conflictingAppPagePaths: string[] = []
 
         let envChange = false
         let tsconfigChange = false
@@ -419,6 +421,12 @@ export default class DevServer extends Server {
             pageName = pageName.replace(/\/index$/, '') || '/'
           }
 
+          if (pageNameSet.has(pageName)) {
+            conflictingAppPagePaths.push(pageName)
+          } else {
+            pageNameSet.add(pageName)
+          }
+
           /**
            * If there is a middleware that is not declared in the root we will
            * warn without adding it so it doesn't make its way into the system.
@@ -442,6 +450,19 @@ export default class DevServer extends Server {
           })
         }
 
+        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)
+        }
+
         if (!this.usingTypeScript && enabledTypeScript) {
           // we tolerate the error here as this is best effort
           // and the manual install command will be shown
@@ -751,27 +772,33 @@ export default class DevServer extends Server {
       ).then(Boolean)
     }
 
-    // check appDir first if enabled
+    let appFile: string | null = null
+    let pagesFile: string | null = null
+
     if (this.appDir) {
-      const pageFile = await findPageFile(
+      appFile = await findPageFile(
         this.appDir,
-        normalizedPath,
+        normalizedPath + '/page',
         this.nextConfig.pageExtensions,
         true
       )
-      if (pageFile) return true
     }
 
     if (this.pagesDir) {
-      const pageFile = await findPageFile(
+      pagesFile = await findPageFile(
         this.pagesDir,
         normalizedPath,
         this.nextConfig.pageExtensions,
         false
       )
-      return !!pageFile
     }
-    return false
+    if (appFile && pagesFile) {
+      throw new Error(
+        `Conflicting app and page file found: "app${appFile}" and "pages${pagesFile}". Please remove one to continue.`
+      )
+    }
+
+    return Boolean(appFile || pagesFile)
   }
 
   protected async _beforeCatchAllRender(

test/integration/conflicting-app-page-error/app/layout.js

@@ -0,0 +1,10 @@
+export default function RootLayout({ children }) {
+  return (
+    <html>
+      <head>
+        <title>Conflict Test</title>
+      </head>
+      <body>{children}</body>
+    </html>
+  )
+}

test/integration/conflicting-app-page-error/app/non-conflict/page.js

@@ -1,3 +1,3 @@
 export default function Page(props) {
-  return <p>another app</p>
+  return <p>non-conflict app</p>
 }

test/integration/conflicting-app-page-error/pages/index.js

@@ -0,0 +1,3 @@
+export default function Page(props) {
+  return <p>index page</p>
+}

test/integration/conflicting-app-page-error/test/index.test.js

@@ -1,24 +1,100 @@
 /* eslint-env jest */
 
 import path from 'path'
-import { nextBuild } from 'next-test-utils'
+import {
+  killApp,
+  findPort,
+  getRedboxHeader,
+  hasRedbox,
+  launchApp,
+  nextBuild,
+  waitFor,
+} from 'next-test-utils'
+import webdriver from 'next-webdriver'
 
+let app
+let appPort
 const appDir = path.join(__dirname, '..')
+let output = ''
 
-describe('conflict between app file and page file', () => {
-  it('errors during build', async () => {
-    const conflicts = ['/hello', '/another']
-    const results = await nextBuild(appDir, [], {
-      stdout: true,
-      stderr: true,
-      env: { NEXT_SKIP_APP_REACT_INSTALL: '1' },
-    })
-    const output = results.stdout + results.stderr
+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 conflicts) {
+    for (const conflict of ['/hello', '/another']) {
       expect(output).toContain(conflict)
     }
+    expect(output).not.toContain('/index')
     expect(output).not.toContain('/non-conflict')
   })
+
+  if (dev) {
+    it('should show error overlay for /hello', async () => {
+      const browser = await webdriver(appPort, '/hello')
+      expect(await hasRedbox(browser, true)).toBe(true)
+      expect(await getRedboxHeader(browser)).toContain(
+        'Conflicting app and page file found: "app/hello/page.js" and "pages/hello.js". Please remove one to continue.'
+      )
+    })
+
+    it('should show error overlay for /another', async () => {
+      const browser = await webdriver(appPort, '/another')
+      expect(await hasRedbox(browser, true)).toBe(true)
+      expect(await getRedboxHeader(browser)).toContain(
+        'Conflicting app and page file found: "app/another/page.js" and "pages/another.js". Please remove one to continue.'
+      )
+    })
+
+    it('should not show error overlay for /', async () => {
+      const browser = await webdriver(appPort, '/')
+      expect(await hasRedbox(browser, false)).toBe(false)
+      expect(await getRedboxHeader(browser)).toBe(null)
+      expect(await browser.elementByCss('p').text()).toBe('index page')
+    })
+
+    it('should not show error overlay for /non-conflict', async () => {
+      const browser = await webdriver(appPort, '/non-conflict')
+      expect(await hasRedbox(browser, false)).toBe(false)
+      expect(await getRedboxHeader(browser)).toBe(null)
+      expect(await browser.elementByCss('p').text()).toBe('non-conflict app')
+    })
+  }
+}
+
+describe('Conflict between app file and page file', () => {
+  describe('next dev', () => {
+    beforeAll(async () => {
+      output = ''
+      appPort = await findPort()
+      app = await launchApp(appDir, appPort, {
+        onStdout(msg) {
+          output += msg || ''
+        },
+        onStderr(msg) {
+          output += msg || ''
+        },
+      })
+      await waitFor(800)
+    })
+    afterAll(() => {
+      killApp(app)
+    })
+    runTests({ dev: true })
+  })
+
+  describe('next build', () => {
+    beforeAll(async () => {
+      output = ''
+      const { stdout, stderr } = await nextBuild(appDir, [], {
+        stdout: true,
+        stderr: true,
+        env: { NEXT_SKIP_APP_REACT_INSTALL: '1' },
+      })
+      output = stdout + stderr
+    })
+    afterAll(() => {
+      killApp(app)
+    })
+    runTests({ dev: false })
+  })
 })