fix: improves support for __NEXT_PRIVATE_PREBUNDLED_REACT (#2236)

* fix: improves support for __NEXT_PRIVATE_PREBUNDLED_REACT

* chore: checks for server actions to determine experimental or next version of React

* chore: auto-fixed lint/prettier issues

* chore: fixed comment

* chore: fixed comment

* fix: tn/ moving Nicks try/catch to requirehooks

* fix: spelling

* fix: parsedUrl check for i18n

* fix: updated match for basepath/locale

* chore: added test for basePath w/ grouping

---------

Co-authored-by: taty2010 <[email protected]>
Co-authored-by: Tatyana <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Author: 3 people

packages/runtime/src/templates/getHandler.ts

@@ -108,8 +108,7 @@ const makeHandler = ({ conf, app, pageRoot, NextServer, staticManifest = [], mod
     const server = new Server(async (req, res) => {
       try {
         await requestHandler(req, res)
-      } catch (error) {
-        console.error(error)
+      } catch {
         throw new Error('Error handling request. See function logs for details.')
       }
     })

packages/runtime/src/templates/handlerUtils.ts

@@ -7,7 +7,9 @@ import { promisify } from 'util'
 
 import { HandlerEvent, HandlerResponse } from '@netlify/functions'
 import { http, https } from 'follow-redirects'
-import type NextNodeServer from 'next/dist/server/next-server'
+import NextNodeServer from 'next/dist/server/next-server'
+
+import type { StaticRoute } from '../helpers/types'
 
 export type NextServerType = typeof NextNodeServer
 
@@ -271,3 +273,28 @@ export const localizeDataRoute = (dataRoute: string, localizedRoute: string): st
     .replace(new RegExp(`/_next/data/(.+?)/(${locale}/)?`), `/_next/data/$1/${locale}/`)
     .replace(/\/index\.json$/, '.json')
 }
+
+export const getMatchedRoute = (
+  paths: string,
+  routesManifest: Array<StaticRoute>,
+  parsedUrl: string,
+  basePath: string,
+  trailingSlash: boolean,
+  // eslint-disable-next-line max-params
+): StaticRoute =>
+  routesManifest?.find((route) => {
+    // Some internationalized routes are automatically removing the locale prefix making the path not match the route
+    // we can use the parsedURL, which has the locale included and will match
+    const base = '/'
+    return new RegExp(route.regex).test(
+      new URL(
+        // If using basepath config, we have to use the original path to match the route
+        // This seems to only be an issue on the index page when using group routes
+        parsedUrl ||
+          (basePath && paths === (trailingSlash && !basePath?.endsWith('/') ? `${basePath}/` : basePath)
+            ? base
+            : paths),
+        'http://n',
+      ).pathname,
+    )
+  })

packages/runtime/src/templates/requireHooks.ts

@@ -112,7 +112,6 @@ export const applyRequireHooks = () => {
   ) {
     const reactMode = process.env.__NEXT_PRIVATE_PREBUNDLED_REACT || 'default'
     const resolvedRequest = hooks.get(reactMode)?.get(request) ?? request
-
     return originalResolveFilename.call(mod, resolvedRequest, parent, isMain, options)
 
     // We use `bind` here to avoid referencing outside variables to create potential memory leaks.

packages/runtime/src/templates/server.ts

@@ -1,6 +1,7 @@
 // eslint-disable-next-line n/no-deprecated-api -- this is what Next.js uses as well
 import { parse } from 'url'
 
+import { NextConfig } from 'next'
 import type { PrerenderManifest } from 'next/dist/build'
 import type { BaseNextResponse } from 'next/dist/server/base-http'
 import type { NodeRequestHandler, Options } from 'next/dist/server/next-server'
@@ -12,6 +13,7 @@ import {
   localizeRoute,
   localizeDataRoute,
   unlocalizeRoute,
+  getMatchedRoute,
 } from './handlerUtils'
 
 interface NetlifyConfig {
@@ -24,6 +26,10 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
     private netlifyConfig: NetlifyConfig
     private netlifyPrerenderManifest: PrerenderManifest
 
+    public getAppRouterReactVersion(): string {
+      return this.nextConfig.experimental?.serverActions ? 'experimental' : 'next'
+    }
+
     public constructor(options: Options, netlifyConfig: NetlifyConfig) {
       super(options)
       this.netlifyConfig = netlifyConfig
@@ -47,7 +53,7 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
         const { url, headers } = req
 
         // conditionally use the prebundled React module
-        this.netlifyPrebundleReact(url)
+        this.netlifyPrebundleReact(url, this.nextConfig, parsedUrl)
 
         // intercept on-demand revalidation requests and handle with the Netlify API
         if (headers['x-prerender-revalidate'] && this.netlifyConfig.revalidateToken) {
@@ -76,24 +82,20 @@ 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) {
+    private async netlifyPrebundleReact(path: string, { basePath, trailingSlash }: NextConfig, parsedUrl) {
       const routesManifest = this.getRoutesManifest?.()
       const appPathsRoutes = this.getAppPathRoutes?.()
-
       const routes = routesManifest && [...routesManifest.staticRoutes, ...routesManifest.dynamicRoutes]
-      const matchedRoute = routes?.find((route) => new RegExp(route.regex).test(new URL(path, 'http://n').pathname))
+      const matchedRoute = await getMatchedRoute(path, routes, parsedUrl, basePath, trailingSlash)
       const isAppRoute = appPathsRoutes && matchedRoute ? appPathsRoutes[matchedRoute.page] : false
 
       if (isAppRoute) {
         // app routes should use prebundled React
         // eslint-disable-next-line no-underscore-dangle
-        process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = this.nextConfig.experimental?.serverActions
-          ? 'experimental'
-          : 'next'
+        process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = this.getAppRouterReactVersion()
+
         return
       }
-
-      // pages routes should use use node_modules React
       // eslint-disable-next-line no-underscore-dangle
       process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = ''
     }

test/templates/server.spec.ts

@@ -68,6 +68,7 @@ jest.mock(
 jest.mock(
   'routes-manifest.json',
   () => ({
+    basePath: '',
     dynamicRoutes: [
       {
         page: '/posts/[title]',
@@ -88,6 +89,12 @@ jest.mock(
       },
     ],
     staticRoutes: [
+      {
+        namedRegex: '^/(?:/)?$',
+        page: '/',
+        regex: '^/(?:/)?$',
+        routeKeys: {},
+      },
       {
         page: '/non-i18n/with-revalidate',
         regex: '^/non-i18n/with-revalidate(?:/)?$',
@@ -101,11 +108,23 @@ jest.mock(
         namedRegex: '^/i18n/with-revalidate(?:/)?$',
       },
     ],
+    redirects: [
+      {
+        basePath: false,
+        destination: '/docs/',
+        internal: true,
+        locale: false,
+        regex: '^/docs$',
+        source: '/docs',
+        statusCode: 308,
+      },
+    ],
   }),
   { virtual: true },
 )
 
 const appPathsManifest = {
+  '/(group)/page': 'app/(group)/page.js',
   '/blog/(test)/[author]/[slug]/page': 'app/blog/[author]/[slug]/page.js',
 }
 
@@ -312,4 +331,19 @@ describe('the netlify next server', () => {
     // eslint-disable-next-line no-underscore-dangle
     expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('experimental')
   })
+
+  it('assigns correct prebundled react with basePath config using appdir', async () => {
+    const netlifyNextServer = new NetlifyNextServer({ conf: { experimental: { appDir: true }, basePath: '/docs' } }, {})
+    const requestHandler = netlifyNextServer.getRequestHandler()
+
+    const { req: mockReq, res: mockRes } = createRequestResponseMocks({
+      url: '/docs',
+    })
+
+    // @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')
+  })
 })