fix: use native path matching for prebundled react conditional (#2206)

* fix: use native path matching for app routes

* chore: re-enable custom server tests and add react tests

* chore: update tests to work on windows

Author: orinokai

packages/runtime/src/templates/server.ts

@@ -12,7 +12,6 @@ import {
   localizeRoute,
   localizeDataRoute,
   unlocalizeRoute,
-  joinPaths,
 } from './handlerUtils'
 
 interface NetlifyConfig {
@@ -78,12 +77,11 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
     // doing what they do in https://github.com/vercel/vercel/blob/1663db7ca34d3dd99b57994f801fb30b72fbd2f3/packages/next/src/server-build.ts#L576-L580
     private netlifyPrebundleReact(path: string) {
       const routesManifest = this.getRoutesManifest?.()
-      const appPathsManifest = this.getAppPathsManifest?.()
+      const appPathsRoutes = this.getAppPathRoutes?.()
 
       const routes = routesManifest && [...routesManifest.staticRoutes, ...routesManifest.dynamicRoutes]
-      const matchedRoute = routes?.find((route) => new RegExp(route.regex).test(path.split('?')[0]))
-      const isAppRoute =
-        appPathsManifest && matchedRoute ? appPathsManifest[joinPaths(matchedRoute.page, 'page')] : false
+      const matchedRoute = routes?.find((route) => new RegExp(route.regex).test(new URL(path, 'http://n').pathname))
+      const isAppRoute = appPathsRoutes && matchedRoute ? appPathsRoutes[matchedRoute.page] : false
 
       if (isAppRoute) {
         // app routes should use prebundled React

test/templates/server.spec.ts

@@ -65,6 +65,50 @@ jest.mock(
   { virtual: true },
 )
 
+jest.mock(
+  'routes-manifest.json',
+  () => ({
+    dynamicRoutes: [
+      {
+        page: '/posts/[title]',
+        regex: '^/posts/([^/]+?)(?:/)?$',
+        routeKeys: {
+          nxtPtitle: 'nxtPtitle',
+        },
+        namedRegex: '^/posts/(?<nxtPtitle>[^/]+?)(?:/)?$',
+      },
+      {
+        page: '/blog/[author]/[slug]',
+        regex: '^/blog/([^/]+?)/([^/]+?)(?:/)?$',
+        routeKeys: {
+          nxtPauthor: 'nxtPauthor',
+          nxtPslug: 'nxtPslug',
+        },
+        namedRegex: '^/blog/(?<nxtPauthor>[^/]+?)/(?<nxtPslug>[^/]+?)(?:/)?$',
+      },
+    ],
+    staticRoutes: [
+      {
+        page: '/non-i18n/with-revalidate',
+        regex: '^/non-i18n/with-revalidate(?:/)?$',
+        routeKeys: {},
+        namedRegex: '^/non-i18n/with-revalidate(?:/)?$',
+      },
+      {
+        page: '/i18n/with-revalidate',
+        regex: '^/i18n/with-revalidate(?:/)?$',
+        routeKeys: {},
+        namedRegex: '^/i18n/with-revalidate(?:/)?$',
+      },
+    ],
+  }),
+  { virtual: true },
+)
+
+const appPathsManifest = {
+  '/blog/(test)/[author]/[slug]/page': 'app/blog/[author]/[slug]/page.js',
+}
+
 let NetlifyNextServer: NetlifyNextServerType
 beforeAll(() => {
   const NextServer: NextServerType = require(getServerFile(__dirname, false)).default
@@ -76,12 +120,14 @@ beforeAll(() => {
     this.buildId = mockBuildId
     this.nextConfig = nextOptions.conf
     this.netlifyConfig = netlifyConfig
+    this.renderOpts = { previewProps: {} }
+    this.appPathsManifest = appPathsManifest
   }
   Object.setPrototypeOf(NetlifyNextServer, MockNetlifyNextServerConstructor)
 })
 
 describe('the netlify next server', () => {
-  it.skip('does not revalidate a request without an `x-prerender-revalidate` header', async () => {
+  it('does not revalidate a request without an `x-prerender-revalidate` header', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig })
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -92,7 +138,7 @@ describe('the netlify next server', () => {
     expect(mockedApiFetch).not.toHaveBeenCalled()
   })
 
-  it.skip('revalidates a static non-i18n route with an `x-prerender-revalidate` header', async () => {
+  it('revalidates a static non-i18n route with an `x-prerender-revalidate` header', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig })
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -112,7 +158,7 @@ describe('the netlify next server', () => {
     )
   })
 
-  it.skip('revalidates a static i18n route with an `x-prerender-revalidate` header', async () => {
+  it('revalidates a static i18n route with an `x-prerender-revalidate` header', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: { ...mocki18nConfig } }, { ...mockTokenConfig })
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -132,7 +178,7 @@ describe('the netlify next server', () => {
     )
   })
 
-  it.skip('revalidates a dynamic non-i18n route with an `x-prerender-revalidate` header', async () => {
+  it('revalidates a dynamic non-i18n route with an `x-prerender-revalidate` header', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: {} }, { ...mockTokenConfig })
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -152,7 +198,7 @@ describe('the netlify next server', () => {
     )
   })
 
-  it.skip('revalidates a dynamic i18n route with an `x-prerender-revalidate` header', async () => {
+  it('revalidates a dynamic i18n route with an `x-prerender-revalidate` header', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: { ...mocki18nConfig } }, { ...mockTokenConfig })
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -172,7 +218,7 @@ describe('the netlify next server', () => {
     )
   })
 
-  it.skip('throws an error when route is not found in the manifest', async () => {
+  it('throws an error when route is not found in the manifest', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: {} }, mockTokenConfig)
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -187,7 +233,7 @@ describe('the netlify next server', () => {
     )
   })
 
-  it.skip('throws an error when paths are not found by the API', async () => {
+  it('throws an error when paths are not found by the API', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: {} }, mockTokenConfig)
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -203,7 +249,7 @@ describe('the netlify next server', () => {
     )
   })
 
-  it.skip('throws an error when the revalidate API is unreachable', async () => {
+  it('throws an error when the revalidate API is unreachable', async () => {
     const netlifyNextServer = new NetlifyNextServer({ conf: {} }, mockTokenConfig)
     const requestHandler = netlifyNextServer.getRequestHandler()
 
@@ -218,4 +264,52 @@ describe('the netlify next server', () => {
       'Unable to connect',
     )
   })
+
+  it('resolves react as normal for pages routes', async () => {
+    const netlifyNextServer = new NetlifyNextServer({ conf: {} }, {})
+    const requestHandler = netlifyNextServer.getRequestHandler()
+
+    const { req: mockReq, res: mockRes } = createRequestResponseMocks({
+      url: '/posts/hello',
+    })
+
+    // @ts-expect-error - Types are incorrect for `MockedResponse`
+    await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes))
+
+    // eslint-disable-next-line no-underscore-dangle
+    expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('')
+  })
+
+  it('resolves the prebundled react version for app routes', async () => {
+    const netlifyNextServer = new NetlifyNextServer({ conf: { experimental: { appDir: true } } }, {})
+    const requestHandler = netlifyNextServer.getRequestHandler()
+
+    const { req: mockReq, res: mockRes } = createRequestResponseMocks({
+      url: '/blog/rob/hello',
+    })
+
+    // @ts-expect-error - Types are incorrect for `MockedResponse`
+    await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes))
+
+    // eslint-disable-next-line no-underscore-dangle
+    expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('next')
+  })
+
+  it('resolves the experimental prebundled react version for app routes with server actions', async () => {
+    const netlifyNextServer = new NetlifyNextServer(
+      { conf: { experimental: { appDir: true, serverActions: true } } },
+      {},
+    )
+    const requestHandler = netlifyNextServer.getRequestHandler()
+
+    const { req: mockReq, res: mockRes } = createRequestResponseMocks({
+      url: '/blog/rob/hello',
+    })
+
+    // @ts-expect-error - Types are incorrect for `MockedResponse`
+    await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes))
+
+    // eslint-disable-next-line no-underscore-dangle
+    expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('experimental')
+  })
 })