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 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
13 changes: 11 additions & 2 deletions packages/runtime/src/helpers/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ export const updateRequiredServerFiles = async (publish: string, modifiedConfig:
await writeJSON(configFile, modifiedConfig)
}

export const resolveModuleRoot = (moduleName) => {
export const resolveModuleRoot = (moduleName, paths = [process.cwd()]) => {
try {
return dirname(relative(process.cwd(), require.resolve(`${moduleName}/package.json`, { paths: [process.cwd()] })))
return dirname(relative(process.cwd(), require.resolve(`${moduleName}/package.json`, { paths })))
} catch {
return null
}
Expand Down Expand Up @@ -162,6 +162,15 @@ export const configureHandlerFunctions = async ({
`!${nextRoot}/dist/compiled/webpack/bundle4.js`,
`!${nextRoot}/dist/compiled/webpack/bundle5.js`,
)

// 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.
const styledJsxRoot = resolveModuleRoot('styled-jsx', [join(process.cwd(), nextRoot)])
if (styledJsxRoot) {
const styledJsxStyleModulePath = join(styledJsxRoot, 'style.js')
netlifyConfig.functions[functionName].included_files.push(styledJsxStyleModulePath)
}
}

excludedModules.forEach((moduleName) => {
Expand Down
9 changes: 4 additions & 5 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 @@ -462,8 +462,7 @@ export const writeEdgeFunctions = async ({
function: functionName,
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 +472,7 @@ export const writeEdgeFunctions = async ({
function: functionName,
name: edgeFunctionDefinition.name,
pattern: dataRoute,
cache: usesAppDir ? 'manual' : undefined,
cache: 'manual',
generator,
})
}
Expand Down
4 changes: 3 additions & 1 deletion packages/runtime/src/helpers/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { getResolverForPages, getResolverForSourceFiles } from '../templates/get
import { ApiConfig, extractConfigFromFile, isEdgeConfig } from './analysis'
import { getRequiredServerFiles } from './config'
import { getDependenciesOfFile, getServerFile, getSourceFileForPage } from './files'
import { writeFunctionConfiguration } from './functionsMetaData'
import { writeFunctionConfiguration, useRequireHooks } from './functionsMetaData'
import { pack } from './pack'
import { ApiRouteType } from './types'
import { getFunctionNameForPage } from './utils'
Expand Down Expand Up @@ -132,11 +132,13 @@ export const generateFunctions = async (
}

const writeHandler = async (functionName: string, functionTitle: string, isODB: boolean) => {
const useHooks = await useRequireHooks()
const handlerSource = getHandler({
isODB,
publishDir,
appDir: relative(functionDir, appDir),
nextServerModuleRelativeLocation,
useHooks,
})
await ensureDir(join(functionsDir, functionName))

Expand Down
7 changes: 5 additions & 2 deletions packages/runtime/src/helpers/functionsMetaData.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync, readJSON, writeFile } from 'fs-extra'
import { join } from 'pathe'
import { satisfies } from 'semver'

import { NEXT_PLUGIN, NEXT_PLUGIN_NAME } from '../constants'

Expand All @@ -17,8 +18,8 @@ const getNextRuntimeVersion = async (packageJsonPath: string, useNodeModulesPath

const PLUGIN_PACKAGE_PATH = '.netlify/plugins/package.json'

const nextPluginVersion = async () => {
const moduleRoot = resolveModuleRoot(NEXT_PLUGIN)
const nextPluginVersion = async (module?: string) => {
const moduleRoot = resolveModuleRoot(module || NEXT_PLUGIN)
const nodeModulesPath = moduleRoot ? join(moduleRoot, 'package.json') : null

return (
Expand All @@ -31,6 +32,8 @@ const nextPluginVersion = async () => {

export const getPluginVersion = async () => `${NEXT_PLUGIN_NAME}@${await nextPluginVersion()}`

export const useRequireHooks = async () => satisfies(await nextPluginVersion('next'), '13.3.3 - 13.4.9')

// The information needed to create a function configuration file
export interface FunctionInfo {
// The name of the function, e.g. `___netlify-handler`
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
26 changes: 20 additions & 6 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 @@ -39,11 +40,20 @@ type MakeHandlerParams = {
NextServer: NextServerType
staticManifest: Array<[string, string]>
mode: 'ssr' | 'odb'
useHooks: boolean
}

// We return a function and then call `toString()` on it to serialise it as the launcher function
// eslint-disable-next-line max-lines-per-function
const makeHandler = ({ conf, app, pageRoot, NextServer, staticManifest = [], mode = 'ssr' }: MakeHandlerParams) => {
const makeHandler = ({
conf,
app,
pageRoot,
NextServer,
staticManifest = [],
mode = 'ssr',
useHooks,
}: MakeHandlerParams) => {
// Change working directory into the site root, unless using Nx, which moves the
// dist directory and handles this itself
const dir = path.resolve(__dirname, app)
Expand All @@ -57,10 +67,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 is set and Next version before running requireHooks
if (appDir && useHooks) overrideRequireHooks(conf.experimental)
const NetlifyNextServer: NetlifyNextServerType = getNetlifyNextServer(NextServer)
applyRequireHooks()
if (appDir && useHooks) applyRequireHooks()

const ONE_YEAR_IN_SECONDS = 31536000

Expand Down Expand Up @@ -205,6 +218,7 @@ export const getHandler = ({
publishDir = '../../../.next',
appDir = '../../..',
nextServerModuleRelativeLocation,
useHooks,
}): string =>
// This is a string, but if you have the right editor plugin it should format as js (e.g. bierner.comment-tagged-templates in VS Code)
javascript/* javascript */ `
Expand All @@ -218,7 +232,7 @@ export const getHandler = ({
const { promises } = require("fs");
// We copy the file here rather than requiring from the node module
const { Bridge } = require("./bridge");
const { augmentFsModule, getMaxAge, getMultiValueHeaders, getPrefetchResponse, normalizePath } = require('./handlerUtils')
const { augmentFsModule, getMaxAge, getMultiValueHeaders, getPrefetchResponse, normalizePath, nextVersionNum } = require('./handlerUtils')
const { overrideRequireHooks, applyRequireHooks } = require("./requireHooks")
const { getNetlifyNextServer } = require("./server")
const NextServer = require(${JSON.stringify(nextServerModuleRelativeLocation)}).default
Expand All @@ -232,7 +246,7 @@ export const getHandler = ({
const pageRoot = path.resolve(path.join(__dirname, "${publishDir}", "server"));
exports.handler = ${
isODB
? `builder((${makeHandler.toString()})({ conf: config, app: "${appDir}", pageRoot, NextServer, staticManifest, mode: 'odb' }));`
: `(${makeHandler.toString()})({ conf: config, app: "${appDir}", pageRoot, NextServer, staticManifest, mode: 'ssr' });`
? `builder((${makeHandler.toString()})({ conf: config, app: "${appDir}", pageRoot, NextServer, staticManifest, mode: 'odb', useHooks: ${useHooks}}));`
: `(${makeHandler.toString()})({ conf: config, app: "${appDir}", pageRoot, NextServer, staticManifest, mode: 'ssr', useHooks: ${useHooks}});`
}
`
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
9 changes: 8 additions & 1 deletion packages/runtime/src/templates/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,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 +21,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 Down Expand Up @@ -53,7 +58,9 @@ const getNetlifyNextServer = (NextServer: NextServerType) => {
const { url, headers } = req

// conditionally use the prebundled React module
this.netlifyPrebundleReact(url, this.nextConfig, parsedUrl)
// PrebundledReact should only apply when appDir is set it falls between the specified Next versions
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
5 changes: 3 additions & 2 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',
'node_modules/styled-jsx/style.js',
'!node_modules/sharp/**/*',
]
// Relative paths in Windows are different
Expand Down Expand Up @@ -558,10 +559,10 @@ describe('onBuild()', () => {
expect(existsSync(odbHandlerFile)).toBeTruthy()

expect(readFileSync(handlerFile, 'utf8')).toMatch(
`({ conf: config, app: "../../..", pageRoot, NextServer, staticManifest, mode: 'ssr' })`,
`({ conf: config, app: "../../..", pageRoot, NextServer, staticManifest, mode: 'ssr', useHooks: false})`,
)
expect(readFileSync(odbHandlerFile, 'utf8')).toMatch(
`({ conf: config, app: "../../..", pageRoot, NextServer, staticManifest, mode: 'odb' })`,
`({ conf: config, app: "../../..", pageRoot, NextServer, staticManifest, mode: 'odb', useHooks: false})`,
)
expect(readFileSync(handlerFile, 'utf8')).toMatch(`require("../../../.next/required-server-files.json")`)
expect(readFileSync(odbHandlerFile, 'utf8')).toMatch(`require("../../../.next/required-server-files.json")`)
Expand Down
2 changes: 1 addition & 1 deletion test/templates/server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ describe('the netlify next server', () => {
await requestHandler(new NodeNextRequest(mockReq), new NodeNextResponse(mockRes))

// eslint-disable-next-line no-underscore-dangle
expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBe('')
expect(process.env.__NEXT_PRIVATE_PREBUNDLED_REACT).toBeFalsy()
})

it('resolves the prebundled react version for app routes', async () => {
Expand Down
7 changes: 5 additions & 2 deletions test/test-utils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import path, { dirname } from 'path'

import cpy from 'cpy'
import { writeJSON, existsSync, ensureDir, readJson, copy } from 'fs-extra'
import { writeJSON, writeFile, existsSync, ensureDir, readJson, copy } from 'fs-extra'
import { dir as getTmpDir } from 'tmp-promise'

const FIXTURES_DIR = `${__dirname}/fixtures`
Expand All @@ -26,7 +26,7 @@ const rewriteAppDir = async function (dir = '.next') {

// Move .next from sample project to current directory
export const moveNextDist = async function (dir = '.next', copyMods = false) {
await (copyMods ? copyModules(['next', 'sharp']) : stubModules(['next', 'sharp']))
await (copyMods ? copyModules(['next', 'sharp', 'styled-jsx']) : stubModules(['next', 'sharp', 'styled-jsx']))
await ensureDir(dirname(dir))
await copy(path.join(SAMPLE_PROJECT_DIR, '.next'), path.join(process.cwd(), dir))

Expand All @@ -53,6 +53,9 @@ export const stubModules = async function (modules) {
const dir = path.join(process.cwd(), 'node_modules', mod)
await ensureDir(dir)
await writeJSON(path.join(dir, 'package.json'), { name: mod })
if (mod === `styled-jsx`) {
await writeFile('style.js', '')
}
}
}

Expand Down