feat(zisi): add zisi_error_cjs_in_esm_scope flag for erroring on misconfigured module type

packages/zip-it-and-ship-it/src/feature_flags.ts

@@ -24,6 +24,10 @@ export const defaultFlags = {
   // If multiple glob stars are in includedFiles, fail the build instead of warning.
   zisi_esbuild_fail_double_glob: false,
 
+  // Fail the build when a CommonJS function file sits inside a `"type": "module"`
+  // package scope, instead of producing a bundle that fails at runtime.
+  zisi_error_cjs_in_esm_scope: false,
+
   // Adds the `___netlify-telemetry.mjs` file to the function bundle.
   zisi_add_instrumentation_loader: true,
 

packages/zip-it-and-ship-it/src/runtimes/node/bundlers/nft/es_modules.ts

@@ -5,10 +5,13 @@ import { NodeFileTraceReasons } from '@vercel/nft'
 import type { FunctionConfig } from '../../../../config.js'
 import { FeatureFlags } from '../../../../feature_flags.js'
 import type { RuntimeCache } from '../../../../utils/cache.js'
+import { FunctionBundlingUserError } from '../../../../utils/error.js'
 import { cachedReadFile } from '../../../../utils/fs.js'
+import { RUNTIME } from '../../../runtime.js'
 import { ModuleFormat, MODULE_FILE_EXTENSION, MODULE_FORMAT } from '../../utils/module_format.js'
 import { getNodeSupportMatrix } from '../../utils/node_version.js'
 import { getPackageJsonIfAvailable, PackageJson } from '../../utils/package_json.js'
+import { NODE_BUNDLER } from '../types.js'
 
 import { transpileESMToCJS } from './transpile.js'
 
@@ -81,6 +84,21 @@ export const processESM = async ({
   }
 
   if (!isEntrypointESM({ basePath, esmPaths, mainFile })) {
+    // The entrypoint is a `.js` file with no ESM syntax. If it lives inside a
+    // `"type": "module"` package scope, the `package.json` is traced into the
+    // bundle verbatim and Node.js will load the file as an ES module at
+    // runtime, where its CommonJS globals are not defined.
+    if (featureFlags.zisi_error_cjs_in_esm_scope && extension === MODULE_FILE_EXTENSION.JS) {
+      const scopePackageJson = await getPackageJsonIfAvailable(dirname(mainFile))
+
+      if (scopePackageJson.type === 'module') {
+        throw new FunctionBundlingUserError(
+          `The function file '${mainFile}' is a CommonJS module, but the closest 'package.json' declares '"type": "module"', so Node.js will fail to load it at runtime. Either use ESM syntax ('import'/'export'), rename the file extension to '.cjs', or remove '"type": "module"' from the 'package.json'.`,
+          { functionName: name, runtime: RUNTIME.JAVASCRIPT, bundler: NODE_BUNDLER.NFT },
+        )
+      }
+    }
+
     return {
       moduleFormat: MODULE_FORMAT.COMMONJS,
     }

packages/zip-it-and-ship-it/src/utils/error.ts

@@ -14,24 +14,26 @@ interface CustomErrorInfo {
 
 type UserError = Error & { customErrorInfo: CustomErrorInfo }
 
+const createFunctionsBundlingErrorInfo = (location: CustomErrorLocation): CustomErrorInfo => ({
+  type: 'functionsBundling',
+  location,
+})
+
 export class FunctionBundlingUserError extends Error {
+  customErrorInfo: CustomErrorInfo
+
   constructor(message: string, customErrorInfo: CustomErrorLocation) {
     super(message)
 
     Object.setPrototypeOf(this, new.target.prototype)
     this.name = 'FunctionBundlingUserError'
     Error.captureStackTrace(this, FunctionBundlingUserError)
 
-    FunctionBundlingUserError.addCustomErrorInfo(this, customErrorInfo)
+    this.customErrorInfo = createFunctionsBundlingErrorInfo(customErrorInfo)
   }
 
   static addCustomErrorInfo(error: Error, customErrorInfo: CustomErrorLocation): UserError {
-    const info: CustomErrorInfo = {
-      type: 'functionsBundling',
-      location: customErrorInfo,
-    }
-
-    ;(error as UserError).customErrorInfo = info
+    ;(error as UserError).customErrorInfo = createFunctionsBundlingErrorInfo(customErrorInfo)
 
     return error as UserError
   }

packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/function.js

@@ -0,0 +1 @@
+module.exports.handler = async () => ({ statusCode: 200, body: require('./helpers/greeting.js').greeting })

packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/helpers/greeting.js

@@ -0,0 +1 @@
+module.exports.greeting = 'Hello world'

packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/package.json

@@ -0,0 +1,3 @@
+{
+  "type": "module"
+}

packages/zip-it-and-ship-it/tests/main.test.ts

@@ -19,6 +19,7 @@
 import { NODE_BUNDLER } from '../src/runtimes/node/bundlers/types.js'
 import { detectEsModule } from '../src/runtimes/node/utils/detect_es_module.js'
 import { MODULE_FORMAT } from '../src/runtimes/node/utils/module_format.js'
+import type { FunctionBundlingUserError } from '../src/utils/error.js'
 import { shellUtils } from '../src/utils/shell.js'
 import type { ZipFunctionsOptions } from '../src/zip.js'
 import { zipFunctions } from '../src/zip.js'
@@ -430,7 +431,7 @@
       const { files } = await zipFixture(fixtureName, {
         opts,
       })
 
       const unzippedFunctions = await unzipFiles(files)
 
       const func = await importFunctionFile(join(unzippedFunctions[0].unzipPath, 'function.js'))
@@ -511,6 +512,71 @@
     },
   )
 
+  testMany(
+    'Produces a bundle that fails at runtime when bundling a CJS function inside a `"type": "module"` package scope (nft)',
+    ['bundler_nft'],
+    async (options) => {
+      const fixtureName = 'node-cjs-in-esm-scope'
+      const { files } = await zipFixture(fixtureName, { opts: options })
+      const unzippedFunctions = await unzipFiles(files)
+      const { unzipPath } = unzippedFunctions[0]
+
+      // NFT traces the repository root `package.json` into the bundle, unpatched.
+      const packageJson = JSON.parse(await readFile(join(unzipPath, 'package.json'), 'utf8'))
+      expect(packageJson.type).toBe('module')
+
+      await expect(importFunctionFile(join(unzipPath, 'function.js'))).rejects.toThrow(
+        'module is not defined in ES module scope',
+      )
+    },
+  )
+
+  testMany(
+    'Errors at bundle time when bundling a CJS function inside a `"type": "module"` package scope and the `zisi_error_cjs_in_esm_scope` flag is on (nft)',
+    ['bundler_nft'],
+    async (options) => {
+      const fixtureName = 'node-cjs-in-esm-scope'
+      const opts = merge(options, {
+        featureFlags: { zisi_error_cjs_in_esm_scope: true },
+      })
+
+      try {
+        await zipFixture(fixtureName, { opts })
+
+        expect.fail('Bundling should have thrown')
+      } catch (error) {
+        const { customErrorInfo, message } = error as FunctionBundlingUserError
+
+        expect(message).toMatch('is a CommonJS module, but the closest \'package.json\' declares \'"type": "module"\'')
+        expect(customErrorInfo.type).toBe('functionsBundling')
+        expect(customErrorInfo.location.bundler).toBe('nft')
+        expect(customErrorInfo.location.functionName).toBe('function')
+        expect(customErrorInfo.location.runtime).toBe('js')
+      }
+    },
+  )
+
+  testMany(
+    'Produces a working bundle when bundling a CJS function inside a `"type": "module"` package scope (zisi)',
+    ['bundler_default'],
+    async (options) => {
+      const fixtureName = 'node-cjs-in-esm-scope'
+      const { files } = await zipFixture(fixtureName, { opts: options })
+      const unzippedFunctions = await unzipFiles(files)
+      const { unzipPath } = unzippedFunctions[0]
+
+      // With no `package.json` in the bundle, Node falls back to loading the
+      // `.js` main file as CJS.
+      expect(await pathExists(join(unzipPath, 'package.json'))).toBe(false)
+
+      const func = await importFunctionFile(join(unzipPath, 'function.js'))
+      const { body, statusCode } = await func.handler()
+
+      expect(statusCode).toBe(200)
+      expect(body).toBe('Hello world')
+    },
+  )
+
   testMany('Can require local files deeply', [...allBundleConfigs], async (options) => {
     await zipNode('local-deep-require', { opts: options })
   })