From 325968d9fceb925d4972c242aea446f5a1cb2f8d Mon Sep 17 00:00:00 2001
From: Michal Piechowiak <misiek.piechowiak@gmail.com>
Date: Mon, 22 Jul 2024 17:28:38 +0200
Subject: [PATCH] fix: apply type: module only to runtime modules (#2549)

* test: add integration and e2e test for requiring CJS .js module in serverless runtime

* fix: apply type: module only to runtime modules
---
 src/build/content/server.ts                   |  4 ++--
 src/build/functions/server.ts                 | 21 ++++++++++---------
 src/build/plugin-context.ts                   |  4 ++++
 tests/e2e/simple-app.test.ts                  | 11 ++++++++++
 .../cjs-file-with-js-extension/bundled.cjs    |  9 ++++++++
 .../api/cjs-file-with-js-extension/route.js   | 11 ++++++++++
 .../simple/cjs-file-with-js-extension.js      |  9 ++++++++
 tests/integration/simple-app.test.ts          | 14 +++++++++++++
 8 files changed, 71 insertions(+), 12 deletions(-)
 create mode 100644 tests/fixtures/simple/app/api/cjs-file-with-js-extension/bundled.cjs
 create mode 100644 tests/fixtures/simple/app/api/cjs-file-with-js-extension/route.js
 create mode 100644 tests/fixtures/simple/cjs-file-with-js-extension.js

diff --git a/src/build/content/server.ts b/src/build/content/server.ts
index 39e11ea8c6..ab7f4fc3cf 100644
--- a/src/build/content/server.ts
+++ b/src/build/content/server.ts
@@ -260,9 +260,9 @@ export const copyNextDependencies = async (ctx: PluginContext): Promise<void> =>
   await tracer.withActiveSpan('copyNextDependencies', async () => {
     const entries = await readdir(ctx.standaloneDir)
     const promises: Promise<void>[] = entries.map(async (entry) => {
-      // copy all except the package.json and distDir (.next) folder as this is handled in a separate function
+      // copy all except the distDir (.next) folder as this is handled in a separate function
       // this will include the node_modules folder as well
-      if (entry === 'package.json' || entry === ctx.nextDistDir) {
+      if (entry === ctx.nextDistDir) {
         return
       }
       const src = join(ctx.standaloneDir, entry)
diff --git a/src/build/functions/server.ts b/src/build/functions/server.ts
index dbdba6553f..bd38a82162 100644
--- a/src/build/functions/server.ts
+++ b/src/build/functions/server.ts
@@ -53,11 +53,20 @@ const copyHandlerDependencies = async (ctx: PluginContext) => {
       )
     }
 
+    // We need to create a package.json file with type: module to make sure that the runtime modules
+    // are handled correctly as ESM modules
+    promises.push(
+      writeFile(
+        join(ctx.serverHandlerRuntimeModulesDir, 'package.json'),
+        JSON.stringify({ type: 'module' }),
+      ),
+    )
+
     const fileList = await glob('dist/**/*', { cwd: ctx.pluginDir })
 
     for (const filePath of fileList) {
       promises.push(
-        cp(join(ctx.pluginDir, filePath), join(ctx.serverHandlerDir, '.netlify', filePath), {
+        cp(join(ctx.pluginDir, filePath), join(ctx.serverHandlerRuntimeModulesDir, filePath), {
           recursive: true,
           force: true,
         }),
@@ -85,13 +94,6 @@ const writeHandlerManifest = async (ctx: PluginContext) => {
   )
 }
 
-const writePackageMetadata = async (ctx: PluginContext) => {
-  await writeFile(
-    join(ctx.serverHandlerRootDir, 'package.json'),
-    JSON.stringify({ type: 'module' }),
-  )
-}
-
 const applyTemplateVariables = (template: string, variables: Record<string, string>) => {
   return Object.entries(variables).reduce((acc, [key, value]) => {
     return acc.replaceAll(key, value)
@@ -136,13 +138,12 @@ export const clearStaleServerHandlers = async (ctx: PluginContext) => {
  */
 export const createServerHandler = async (ctx: PluginContext) => {
   await tracer.withActiveSpan('createServerHandler', async () => {
-    await mkdir(join(ctx.serverHandlerDir, '.netlify'), { recursive: true })
+    await mkdir(join(ctx.serverHandlerRuntimeModulesDir), { recursive: true })
 
     await copyNextServerCode(ctx)
     await copyNextDependencies(ctx)
     await copyHandlerDependencies(ctx)
     await writeHandlerManifest(ctx)
-    await writePackageMetadata(ctx)
     await writeHandlerFile(ctx)
 
     await verifyHandlerDirStructure(ctx)
diff --git a/src/build/plugin-context.ts b/src/build/plugin-context.ts
index a28148db96..9b0ecc199c 100644
--- a/src/build/plugin-context.ts
+++ b/src/build/plugin-context.ts
@@ -182,6 +182,10 @@ export class PluginContext {
     return join(this.serverHandlerRootDir, this.distDirParent)
   }
 
+  get serverHandlerRuntimeModulesDir(): string {
+    return join(this.serverHandlerDir, '.netlify')
+  }
+
   get nextServerHandler(): string {
     if (this.relativeAppDir.length !== 0) {
       return join(this.lambdaWorkingDirectory, '.netlify/dist/run/handlers/server.js')
diff --git a/tests/e2e/simple-app.test.ts b/tests/e2e/simple-app.test.ts
index 87a90dcca9..a9f440c331 100644
--- a/tests/e2e/simple-app.test.ts
+++ b/tests/e2e/simple-app.test.ts
@@ -252,3 +252,14 @@ test('Compressed rewrites are readable', async ({ simple }) => {
   expect(resp.headers.get('content-encoding')).toEqual('br')
   expect(await resp.text()).toContain('<title>Example Domain</title>')
 })
+
+test('can require CJS module that is not bundled', async ({ simple }) => {
+  const resp = await fetch(`${simple.url}/api/cjs-file-with-js-extension`)
+
+  expect(resp.status).toBe(200)
+
+  const parsedBody = await resp.json()
+
+  expect(parsedBody.notBundledCJSModule.isBundled).toEqual(false)
+  expect(parsedBody.bundledCJSModule.isBundled).toEqual(true)
+})
diff --git a/tests/fixtures/simple/app/api/cjs-file-with-js-extension/bundled.cjs b/tests/fixtures/simple/app/api/cjs-file-with-js-extension/bundled.cjs
new file mode 100644
index 0000000000..3862da417b
--- /dev/null
+++ b/tests/fixtures/simple/app/api/cjs-file-with-js-extension/bundled.cjs
@@ -0,0 +1,9 @@
+const { parse: pathParse } = require('node:path')
+
+const fileBase = pathParse(__filename).base
+
+module.exports = {
+  fileBase,
+  // if fileBase is not the same as this module name, it was bundled
+  isBundled: fileBase !== 'bundled.cjs',
+}
diff --git a/tests/fixtures/simple/app/api/cjs-file-with-js-extension/route.js b/tests/fixtures/simple/app/api/cjs-file-with-js-extension/route.js
new file mode 100644
index 0000000000..c4be1999e8
--- /dev/null
+++ b/tests/fixtures/simple/app/api/cjs-file-with-js-extension/route.js
@@ -0,0 +1,11 @@
+import { NextResponse } from 'next/server'
+import { resolve } from 'node:path'
+
+export async function GET() {
+  return NextResponse.json({
+    notBundledCJSModule: __non_webpack_require__(resolve('./cjs-file-with-js-extension.js')),
+    bundledCJSModule: require('./bundled.cjs'),
+  })
+}
+
+export const dynamic = 'force-dynamic'
diff --git a/tests/fixtures/simple/cjs-file-with-js-extension.js b/tests/fixtures/simple/cjs-file-with-js-extension.js
new file mode 100644
index 0000000000..197e9507c4
--- /dev/null
+++ b/tests/fixtures/simple/cjs-file-with-js-extension.js
@@ -0,0 +1,9 @@
+const { parse: pathParse } = require('node:path')
+
+const fileBase = pathParse(__filename).base
+
+module.exports = {
+  fileBase,
+  // if fileBase is not the same as this module name, it was bundled
+  isBundled: fileBase !== 'cjs-file-with-js-extension.js',
+}
diff --git a/tests/integration/simple-app.test.ts b/tests/integration/simple-app.test.ts
index 423908e3ef..58ccf8be6d 100644
--- a/tests/integration/simple-app.test.ts
+++ b/tests/integration/simple-app.test.ts
@@ -245,6 +245,20 @@ test.skipIf(process.env.NEXT_VERSION !== 'canary')<FixtureTestContext>(
   },
 )
 
+test<FixtureTestContext>('can require CJS module that is not bundled', async (ctx) => {
+  await createFixture('simple', ctx)
+  await runPlugin(ctx)
+
+  const response = await invokeFunction(ctx, { url: '/api/cjs-file-with-js-extension' })
+
+  expect(response.statusCode).toBe(200)
+
+  const parsedBody = JSON.parse(response.body)
+
+  expect(parsedBody.notBundledCJSModule.isBundled).toEqual(false)
+  expect(parsedBody.bundledCJSModule.isBundled).toEqual(true)
+})
+
 describe('next patching', async () => {
   const { cp: originalCp, appendFile } = (await vi.importActual(
     'node:fs/promises',