fix: improve support for new requireHooks update (#2313)

* fix: make sure appDir exists before running requireHooks * fix: run prebundledReact if appDir exists in conf * fix: use ExperimentalConfigWithLegacy for appDir * fix: set cache to manual and env as optional * fix: prettier * fix: update appDir check * fix: styled-jsx solution for 13.5 * test: add styled-jsx to included files test * test: added next latest to i18n for testing * fix: remove i18n for wp * fix: use requireHooks based on next -v and appDir * fix: remove fs-extra * fix: add semver to handler * fix: try to import semver * fix: testing something new * fix: reset * fix: add back useHooks * fix: test updates + remove preRender update * test: add useHooks * refactor: use existing resolveModuleRoot function instead of module.createRequire * fix: remove 13.5 --------- Co-authored-by: Michal Piechowiak <[email protected]> Co-authored-by: Rob Stanford <[email protected]>
opennextjs · Oct 12, 2023 · e354b73 · e354b73
1 parent b3be3f8
commit e354b73
Show file tree

Hide file tree

Showing 12 changed files with 80 additions and 31 deletions.
diff --git a/packages/runtime/src/helpers/config.ts b/packages/runtime/src/helpers/config.ts
@@ -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
   }
@@ -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) => {

diff --git a/packages/runtime/src/helpers/edge.ts b/packages/runtime/src/helpers/edge.ts
@@ -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[]
@@ -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
@@ -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'))
@@ -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
@@ -473,7 +472,7 @@ export const writeEdgeFunctions = async ({
           function: functionName,
           name: edgeFunctionDefinition.name,
           pattern: dataRoute,
-          cache: usesAppDir ? 'manual' : undefined,
+          cache: 'manual',
           generator,
         })
       }

diff --git a/packages/runtime/src/helpers/functions.ts b/packages/runtime/src/helpers/functions.ts
@@ -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'
@@ -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))
 

diff --git a/packages/runtime/src/helpers/functionsMetaData.ts b/packages/runtime/src/helpers/functionsMetaData.ts
@@ -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'
 
@@ -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 (
@@ -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`

diff --git a/packages/runtime/src/helpers/utils.ts b/packages/runtime/src/helpers/utils.ts
@@ -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> => {

diff --git a/packages/runtime/src/index.ts b/packages/runtime/src/index.ts
@@ -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,
@@ -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)
 

diff --git a/packages/runtime/src/templates/getHandler.ts b/packages/runtime/src/templates/getHandler.ts
@@ -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'
@@ -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)
@@ -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
 
@@ -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 */ `
@@ -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
@@ -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}});`
   }
 `
diff --git a/packages/runtime/src/templates/requireHooks.ts b/packages/runtime/src/templates/requireHooks.ts
@@ -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([
@@ -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([

diff --git a/packages/runtime/src/templates/server.ts b/packages/runtime/src/templates/server.ts
@@ -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,
@@ -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) => {
@@ -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) {

diff --git a/test/index.spec.ts b/test/index.spec.ts
@@ -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
@@ -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")`)

diff --git a/test/templates/server.spec.ts b/test/templates/server.spec.ts
@@ -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 () => {

diff --git a/test/test-utils.ts b/test/test-utils.ts
@@ -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`
@@ -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))
 
@@ -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', '')
+    }
   }
 }