Fix require cache conflict between app and pages (#46736)

Related NEXT-472

When you have `app/page.js` and `pages/page.js`, the `pagePath` are the
same, which is `/page`. This will result require cache conflicts. When
you visit the `app/` page first, then the `pages/` page, the 2nd request
will still get the app dir page module, which result in server error.

Solution: use different cache key of pagePathCache for pages/ and app/ 

<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:
-->

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

packages/next/src/build/index.ts

@@ -2400,7 +2400,7 @@ export default async function build(
 
           // remove server bundles that were exported
           for (const page of staticPages) {
-            const serverBundle = getPagePath(page, distDir)
+            const serverBundle = getPagePath(page, distDir, undefined, false)
             await promises.unlink(serverBundle)
           }
 
@@ -2514,7 +2514,12 @@ export default async function build(
               .traceAsyncFn(async () => {
                 file = `${file}.${ext}`
                 const orig = path.join(exportOptions.outdir, file)
-                const pagePath = getPagePath(originPage, distDir)
+                const pagePath = getPagePath(
+                  originPage,
+                  distDir,
+                  undefined,
+                  false
+                )
 
                 const relativeDest = path
                   .relative(

packages/next/src/export/index.ts

@@ -719,7 +719,7 @@ export default async function exportApp(
           }
           route = normalizePagePath(route)
 
-          const pagePath = getPagePath(pageName, distDir)
+          const pagePath = getPagePath(pageName, distDir, undefined, false)
           const distPagesDir = join(
             pagePath,
             // strip leading / and then recurse number of nested dirs

packages/next/src/server/require.ts

@@ -29,10 +29,10 @@ const pagePathCache =
 export function getMaybePagePath(
   page: string,
   distDir: string,
-  locales?: string[],
-  appDirEnabled?: boolean
+  locales: string[] | undefined,
+  isAppPath: boolean
 ): string | null {
-  const cacheKey = `${page}:${distDir}:${locales}`
+  const cacheKey = `${page}:${distDir}:${locales}:${isAppPath}`
 
   if (pagePathCache.has(cacheKey)) {
     return pagePathCache.get(cacheKey) as string | null
@@ -41,7 +41,7 @@ export function getMaybePagePath(
   const serverBuildPath = join(distDir, SERVER_DIRECTORY)
   let appPathsManifest: undefined | PagesManifest
 
-  if (appDirEnabled) {
+  if (isAppPath) {
     appPathsManifest = require(join(serverBuildPath, APP_PATHS_MANIFEST))
   }
   const pagesManifest = require(join(
@@ -94,10 +94,10 @@ export function getMaybePagePath(
 export function getPagePath(
   page: string,
   distDir: string,
-  locales?: string[],
-  appDirEnabled?: boolean
+  locales: string[] | undefined,
+  isAppPath: boolean
 ): string {
-  const pagePath = getMaybePagePath(page, distDir, locales, appDirEnabled)
+  const pagePath = getMaybePagePath(page, distDir, locales, isAppPath)
 
   if (!pagePath) {
     throw new PageNotFoundError(page)
@@ -109,9 +109,9 @@ export function getPagePath(
 export function requirePage(
   page: string,
   distDir: string,
-  appDirEnabled?: boolean
+  isAppPath: boolean
 ): any {
-  const pagePath = getPagePath(page, distDir, undefined, appDirEnabled)
+  const pagePath = getPagePath(page, distDir, undefined, isAppPath)
   if (pagePath.endsWith('.html')) {
     return promises.readFile(pagePath, 'utf8').catch((err) => {
       throw new MissingStaticPage(page, err.message)

test/e2e/app-dir/similar-pages-paths/app/layout.js

@@ -0,0 +1,8 @@
+export default function Root({ children }) {
+  return (
+    <html>
+      <head></head>
+      <body>{children}</body>
+    </html>
+  )
+}

test/e2e/app-dir/similar-pages-paths/app/page.js

@@ -0,0 +1 @@
+export default () => '(app/page.js)'

test/e2e/app-dir/similar-pages-paths/next.config.js

@@ -0,0 +1,5 @@
+module.exports = {
+  experimental: {
+    appDir: true,
+  },
+}

test/e2e/app-dir/similar-pages-paths/pages/page.js

@@ -0,0 +1 @@
+export default () => '(pages/page.js)'

test/e2e/app-dir/similar-pages-paths/similar-pages-paths.test.ts

@@ -0,0 +1,25 @@
+import { createNextDescribe } from 'e2e-utils'
+
+createNextDescribe(
+  'app-dir similar pages paths',
+  {
+    files: __dirname,
+    skipDeployment: true,
+  },
+  ({ next, isNextDev }) => {
+    // TODO: enable development test
+    if (isNextDev) {
+      it('should skip dev test', () => {})
+      return
+    }
+    it('should redirect route when requesting it directly', async () => {
+      const res1 = await next.fetch('/')
+      expect(res1.status).toBe(200)
+      expect(await res1.text()).toContain('(app/page.js)')
+
+      const res2 = await next.fetch('/page')
+      expect(res2.status).toBe(200)
+      expect(await res2.text()).toContain('(pages/page.js)')
+    })
+  }
+)

test/unit/isolated/require-page.test.ts

@@ -52,18 +52,20 @@ describe('normalizePagePath', () => {
 
 describe('getPagePath', () => {
   it('Should not append /index to the / page', () => {
-    expect(() => getPagePath('/', distDir)).toThrow(
+    expect(() => getPagePath('/', distDir, undefined, false)).toThrow(
       'Cannot find module for page: /'
     )
   })
 
   it('Should prepend / when a page does not have it', () => {
-    const pagePath = getPagePath('_error', distDir)
+    const pagePath = getPagePath('_error', distDir, undefined, false)
     expect(pagePath).toBe(join(pathToBundles, `${sep}_error.js`))
   })
 
   it('Should throw with paths containing ../', () => {
-    expect(() => getPagePath('/../../package.json', distDir)).toThrow()
+    expect(() =>
+      getPagePath('/../../package.json', distDir, undefined, false)
+    ).toThrow()
   })
 })