Skip to content

fix: improve support for new requireHooks update #2313

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 22 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 10 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
2 changes: 1 addition & 1 deletion demos/next-i18next/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"lint": "next lint"
},
"dependencies": {
"next": "^13.4.1",
"next": "^13.5.4",
"next-i18next": "^11.0.0",
"react": "^18.2.0",
"react-dom": "^18.2.0"
Expand Down
13 changes: 13 additions & 0 deletions packages/runtime/src/helpers/config.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import mod from 'module'

import type { NetlifyConfig } from '@netlify/build/types'
import destr from 'destr'
import { readJSON, writeJSON } from 'fs-extra'
Expand Down Expand Up @@ -164,6 +166,17 @@ export const configureHandlerFunctions = async ({
)
}

try {
// on Next 13.5+ there is no longer statically analyzable import to styled-jsx/style
// so lambda fails to bundle it. Next require hooks actually try to resolve it
// and fail if it is not bundled, so we forcefully add it to lambda.

// eslint-disable-next-line n/no-unsupported-features/node-builtins
const nextRequire = mod.createRequire(require.resolve(`next`))
const styledJsxPath = nextRequire.resolve(`styled-jsx/style`)
netlifyConfig.functions[functionName].included_files.push(styledJsxPath)
} catch {}

excludedModules.forEach((moduleName) => {
const moduleRoot = resolveModuleRoot(moduleName)
if (moduleRoot) {
Expand Down
8 changes: 4 additions & 4 deletions packages/runtime/src/helpers/edge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { getRequiredServerFiles, NextConfig } from './config'
import { getPluginVersion } from './functionsMetaData'
import { makeLocaleOptional, stripLookahead, transformCaptureGroups } from './matchers'
import { RoutesManifest } from './types'

// This is the format as of [email protected]
interface EdgeFunctionDefinitionV1 {
env: string[]
Expand All @@ -38,7 +39,7 @@ export interface MiddlewareMatcher {

// This is the format after [email protected]
interface EdgeFunctionDefinitionV2 {
env: string[]
env?: string[]
files: string[]
name: string
page: string
Expand Down Expand Up @@ -376,7 +377,6 @@ export const writeEdgeFunctions = async ({
const { publish } = netlifyConfig.build
const nextConfigFile = await getRequiredServerFiles(publish)
const nextConfig = nextConfigFile.config
const usesAppDir = nextConfig.experimental?.appDir

await copy(getEdgeTemplatePath('../vendor'), join(edgeFunctionRoot, 'vendor'))
await copy(getEdgeTemplatePath('../edge-shared'), join(edgeFunctionRoot, 'edge-shared'))
Expand Down Expand Up @@ -463,7 +463,7 @@ export const writeEdgeFunctions = async ({
name: edgeFunctionDefinition.name,
pattern,
// cache: "manual" is currently experimental, so we restrict it to sites that use experimental appDir
cache: usesAppDir ? 'manual' : undefined,
cache: 'manual',
generator,
})
// pages-dir page routes also have a data route. If there's a match, add an entry mapping that to the function too
Expand All @@ -473,7 +473,7 @@ export const writeEdgeFunctions = async ({
function: functionName,
name: edgeFunctionDefinition.name,
pattern: dataRoute,
cache: usesAppDir ? 'manual' : undefined,
cache: 'manual',
generator,
})
}
Expand Down
3 changes: 2 additions & 1 deletion packages/runtime/src/helpers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ export const getFunctionNameForPage = (page: string, background = false) =>
.replace(DYNAMIC_PARAMETER_REGEX, '_$1-PARAM')
.replace(RESERVED_FILENAME, '_')}-${background ? 'background' : 'handler'}`

type ExperimentalConfigWithLegacy = ExperimentalConfig & {
export type ExperimentalConfigWithLegacy = ExperimentalConfig & {
images?: Pick<ImageConfigComplete, 'remotePatterns'>
appDir?: boolean
}

export const toNetlifyRoute = (nextRoute: string): Array<string> => {
Expand Down
14 changes: 12 additions & 2 deletions packages/runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ import {
getSSRLambdas,
} from './helpers/functions'
import { generateRedirects, generateStaticRedirects } from './helpers/redirects'
import { shouldSkip, isNextAuthInstalled, getCustomImageResponseHeaders, getRemotePatterns } from './helpers/utils'
import {
shouldSkip,
isNextAuthInstalled,
getCustomImageResponseHeaders,
getRemotePatterns,
ExperimentalConfigWithLegacy,
} from './helpers/utils'
import {
verifyNetlifyBuildVersion,
checkNextSiteHasBuilt,
Expand Down Expand Up @@ -248,7 +254,11 @@ const plugin: NetlifyPlugin = {
await checkZipSize(join(FUNCTIONS_DIST, `${ODB_FUNCTION_NAME}.zip`))
const nextConfig = await getNextConfig({ publish, failBuild })

const { basePath, appDir, experimental } = nextConfig
const {
basePath,
appDir,
experimental,
}: { basePath: string; appDir?: string; experimental: ExperimentalConfigWithLegacy } = nextConfig

generateCustomHeaders(nextConfig, headers)

Expand Down
8 changes: 6 additions & 2 deletions packages/runtime/src/templates/getHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { Bridge as NodeBridge } from '@vercel/node-bridge/bridge'
import { outdent as javascript } from 'outdent'

import type { NextConfig } from '../helpers/config'
import { ExperimentalConfigWithLegacy } from '../helpers/utils'

import type { NextServerType } from './handlerUtils'
import type { NetlifyNextServerType } from './server'
Expand Down Expand Up @@ -57,10 +58,13 @@ const makeHandler = ({ conf, app, pageRoot, NextServer, staticManifest = [], mod
require.resolve('./pages.js')
} catch {}

const { appDir }: ExperimentalConfigWithLegacy = conf.experimental
// Next 13.4 conditionally uses different React versions and we need to make sure we use the same one
overrideRequireHooks(conf)
// With the release of 13.5 experimental.appDir is no longer used.
// we will need to check if appDir exists to run requireHooks for older versions
if (appDir) overrideRequireHooks(conf.experimental)
const NetlifyNextServer: NetlifyNextServerType = getNetlifyNextServer(NextServer)
applyRequireHooks()
if (appDir) applyRequireHooks()

const ONE_YEAR_IN_SECONDS = 31536000

Expand Down
12 changes: 6 additions & 6 deletions packages/runtime/src/templates/requireHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@

import mod from 'module'

import type { NextConfig } from '../helpers/config'
import type { ExperimentalConfigWithLegacy } from '../helpers/utils'

const resolveFilename = (mod as any)._resolveFilename
const requireHooks = new Map<string, Map<string, string>>()

export const overrideRequireHooks = (config: NextConfig) => {
setRequireHooks(config)
export const overrideRequireHooks = (experimental: ExperimentalConfigWithLegacy) => {
setRequireHooks(experimental)
resolveRequireHooks()
}

const setRequireHooks = (config: NextConfig) => {
const setRequireHooks = (experimental: ExperimentalConfigWithLegacy) => {
requireHooks.set(
'default',
new Map([
Expand All @@ -24,8 +24,8 @@ const setRequireHooks = (config: NextConfig) => {
]),
)

if (config.experimental.appDir) {
if (config.experimental.serverActions) {
if (experimental.appDir) {
if (experimental.serverActions) {
requireHooks.set(
'experimental',
new Map([
Expand Down
18 changes: 15 additions & 3 deletions packages/runtime/src/templates/server.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { join } from 'path'
// eslint-disable-next-line n/no-deprecated-api -- this is what Next.js uses as well
import { parse } from 'url'

Expand All @@ -6,6 +7,8 @@ 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'

import { ExperimentalConfigWithLegacy } from '../helpers/utils'

import {
netlifyApiFetch,
NextServerType,
Expand All @@ -19,6 +22,9 @@ import {
interface NetlifyConfig {
revalidateToken?: string
}
interface NextConfigWithAppDir extends NextConfig {
experimental: ExperimentalConfigWithLegacy
}

// eslint-disable-next-line max-lines-per-function
const getNetlifyNextServer = (NextServer: NextServerType) => {
Expand All @@ -30,11 +36,16 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
return this.nextConfig.experimental?.serverActions ? 'experimental' : 'next'
}

protected getManifest(manifest: string) {
// eslint-disable-next-line import/no-dynamic-require
return require(join(this.distDir, manifest))
}

public constructor(options: Options, netlifyConfig: NetlifyConfig) {
super(options)
this.netlifyConfig = netlifyConfig
// copy the prerender manifest so it doesn't get mutated by Next.js
const manifest = this.getPrerenderManifest()
const manifest = this.getPrerenderManifest() || this.getManifest('prerender-manifest.json')
this.netlifyPrerenderManifest = {
...manifest,
routes: { ...manifest.routes },
Expand All @@ -53,7 +64,8 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
const { url, headers } = req

// conditionally use the prebundled React module
this.netlifyPrebundleReact(url, this.nextConfig, parsedUrl)
const { experimental }: NextConfigWithAppDir = this.nextConfig
if (experimental?.appDir) 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 @@ -83,7 +95,7 @@ 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 async netlifyPrebundleReact(path: string, { basePath, trailingSlash }: NextConfig, parsedUrl) {
const routesManifest = this.getRoutesManifest?.()
const routesManifest = this.getRoutesManifest?.() || this.getManifest('routes-manifest.json')
const appPathsRoutes = this.getAppPathRoutes?.()
const routes = routesManifest && [...routesManifest.staticRoutes, ...routesManifest.dynamicRoutes]
const matchedRoute = await getMatchedRoute(path, routes, parsedUrl, basePath, trailingSlash)
Expand Down
1 change: 1 addition & 0 deletions test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ describe('onBuild()', () => {
`!node_modules/next/dist/next-server/server/lib/squoosh/**/*.wasm`,
'!node_modules/next/dist/compiled/webpack/bundle4.js',
'!node_modules/next/dist/compiled/webpack/bundle5.js',
'/home/runner/work/next-runtime/next-runtime/node_modules/styled-jsx/style.js',
'!node_modules/sharp/**/*',
]
// Relative paths in Windows are different
Expand Down