From 554a06a9cdcca734a9ee19f29d8590b2dbd49351 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 3 Nov 2022 16:21:46 +0100 Subject: [PATCH 1/4] Clarify app<->pages file conflict --- packages/next/build/index.ts | 58 ++++++++++--------- packages/next/server/dev/next-dev-server.ts | 18 ++++-- .../conflicting-app-page-error/app/page.js | 3 + .../pages/non-conflict-pages.js | 3 + .../test/index.test.js | 24 ++++++-- 5 files changed, 69 insertions(+), 37 deletions(-) create mode 100644 test/integration/conflicting-app-page-error/app/page.js create mode 100644 test/integration/conflicting-app-page-error/pages/non-conflict-pages.js diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index c9d6241b8fac..4e9e869d8b2f 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -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[] = [] diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index 8fa0e5c9712f..b41aef33a383 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -323,7 +323,9 @@ export default class DevServer extends Server { const appPaths: Record = {} const edgeRoutesSet = new Set() const pageNameSet = new Set() - const conflictingAppPagePaths: string[] = [] + const conflictingAppPagePaths = new Set() + const appPageFilePaths = new Map() + const pagesPageFilePaths = new Map() let envChange = false let tsconfigChange = false @@ -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) } @@ -451,7 +458,7 @@ export default class DevServer extends Server { }) } - const numConflicting = conflictingAppPagePaths.length + const numConflicting = conflictingAppPagePaths.size if (numConflicting > 0) { Log.error( `Conflicting app and page file${ @@ -459,9 +466,10 @@ export default class DevServer extends Server { } 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) { diff --git a/test/integration/conflicting-app-page-error/app/page.js b/test/integration/conflicting-app-page-error/app/page.js new file mode 100644 index 000000000000..821fdd14d097 --- /dev/null +++ b/test/integration/conflicting-app-page-error/app/page.js @@ -0,0 +1,3 @@ +export default function Page(props) { + return

index page

+} diff --git a/test/integration/conflicting-app-page-error/pages/non-conflict-pages.js b/test/integration/conflicting-app-page-error/pages/non-conflict-pages.js new file mode 100644 index 000000000000..86f3a93651b3 --- /dev/null +++ b/test/integration/conflicting-app-page-error/pages/non-conflict-pages.js @@ -0,0 +1,3 @@ +export default function Page() { + return

Hello World!

+} diff --git a/test/integration/conflicting-app-page-error/test/index.test.js b/test/integration/conflicting-app-page-error/test/index.test.js index 3a12a3a397d6..1d634c68e8fd 100644 --- a/test/integration/conflicting-app-page-error/test/index.test.js +++ b/test/integration/conflicting-app-page-error/test/index.test.js @@ -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) @@ -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 () => { From ed6d3efc04e3535032c334cddcb19d70cd6386a2 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 3 Nov 2022 18:02:30 +0100 Subject: [PATCH 2/4] Add ! --- test/integration/conflicting-app-page-error/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/conflicting-app-page-error/test/index.test.js b/test/integration/conflicting-app-page-error/test/index.test.js index 1d634c68e8fd..c2b74218e85d 100644 --- a/test/integration/conflicting-app-page-error/test/index.test.js +++ b/test/integration/conflicting-app-page-error/test/index.test.js @@ -61,7 +61,7 @@ function runTests({ dev }) { 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('h1').text()).toBe('Hello World') + expect(await browser.elementByCss('h1').text()).toBe('Hello World!') }) it('should not show error overlay for /non-conflict', async () => { From de593228b0e66bb9d2eed9651ceb369518acc478 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 3 Nov 2022 19:18:05 +0100 Subject: [PATCH 3/4] Fix lint --- packages/next/build/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 4e9e869d8b2f..46802a753c1a 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -601,11 +601,11 @@ export default async function build( app: appPageKeys.length > 0 ? appPageKeys : undefined, } - const numConflicting = conflictingAppPagePaths.length - if (mappedAppPages && numConflicting > 0) { + const numConflictingAppPaths = conflictingAppPagePaths.length + if (mappedAppPages && numConflictingAppPaths > 0) { Log.error( `Conflicting app and page file${ - numConflicting === 1 ? ' was' : 's were' + numConflictingAppPaths === 1 ? ' was' : 's were' } found, please remove the conflicting files to continue:` ) for (const [pagePath, appPath] of conflictingAppPagePaths) { From 133f36a63ae3e498a8fb69942f5b454f9fdc8f6c Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 3 Nov 2022 11:51:13 -0700 Subject: [PATCH 4/4] gate conflicting error --- packages/next/server/dev/next-dev-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index b41aef33a383..0a1f86351132 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -429,7 +429,7 @@ export default class DevServer extends Server { fileName ) - if (pageNameSet.has(pageName)) { + if (this.appDir && pageNameSet.has(pageName)) { conflictingAppPagePaths.add(pageName) } else { pageNameSet.add(pageName)