Skip to content

fix: improves support for __NEXT_PRIVATE_PREBUNDLED_REACT #2236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/runtime/src/templates/getHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
}
})
Expand Down
29 changes: 28 additions & 1 deletion packages/runtime/src/templates/handlerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
)
})
1 change: 0 additions & 1 deletion packages/runtime/src/templates/requireHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
20 changes: 11 additions & 9 deletions packages/runtime/src/templates/server.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -12,6 +13,7 @@ import {
localizeRoute,
localizeDataRoute,
unlocalizeRoute,
getMatchedRoute,
} from './handlerUtils'

interface NetlifyConfig {
Expand All @@ -24,6 +26,10 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
private netlifyConfig: NetlifyConfig
private netlifyPrerenderManifest: PrerenderManifest

public getAppRouterReactVersion(): string {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this public method so we can get the React version to set process.env.__NEXT_PRIVATE_PREBUNDLED_REACT if the request fails with [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './ server.edge' is not defined

return this.nextConfig.experimental?.serverActions ? 'experimental' : 'next'
}

public constructor(options: Options, netlifyConfig: NetlifyConfig) {
super(options)
this.netlifyConfig = netlifyConfig
Expand All @@ -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) {
Expand Down Expand Up @@ -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 = ''
}
Expand Down
34 changes: 34 additions & 0 deletions test/templates/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ jest.mock(
jest.mock(
'routes-manifest.json',
() => ({
basePath: '',
dynamicRoutes: [
{
page: '/posts/[title]',
Expand All @@ -88,6 +89,12 @@ jest.mock(
},
],
staticRoutes: [
{
namedRegex: '^/(?:/)?$',
page: '/',
regex: '^/(?:/)?$',
routeKeys: {},
},
{
page: '/non-i18n/with-revalidate',
regex: '^/non-i18n/with-revalidate(?:/)?$',
Expand All @@ -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',
}

Expand Down Expand Up @@ -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')
})
})