diff --git a/packages/zip-it-and-ship-it/src/feature_flags.ts b/packages/zip-it-and-ship-it/src/feature_flags.ts index 18393dbabb..4e882997ea 100644 --- a/packages/zip-it-and-ship-it/src/feature_flags.ts +++ b/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, diff --git a/packages/zip-it-and-ship-it/src/runtimes/node/bundlers/nft/es_modules.ts b/packages/zip-it-and-ship-it/src/runtimes/node/bundlers/nft/es_modules.ts index dcbc409370..c3114055c0 100644 --- a/packages/zip-it-and-ship-it/src/runtimes/node/bundlers/nft/es_modules.ts +++ b/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, } diff --git a/packages/zip-it-and-ship-it/src/utils/error.ts b/packages/zip-it-and-ship-it/src/utils/error.ts index 81b8565596..0fd8e69f04 100644 --- a/packages/zip-it-and-ship-it/src/utils/error.ts +++ b/packages/zip-it-and-ship-it/src/utils/error.ts @@ -14,7 +14,14 @@ 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) @@ -22,16 +29,11 @@ export class FunctionBundlingUserError extends Error { 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 } diff --git a/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/function.js b/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/function.js new file mode 100644 index 0000000000..919bffdd83 --- /dev/null +++ b/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 }) diff --git a/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/helpers/greeting.js b/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/helpers/greeting.js new file mode 100644 index 0000000000..7cf8346a90 --- /dev/null +++ b/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/helpers/greeting.js @@ -0,0 +1 @@ +module.exports.greeting = 'Hello world' diff --git a/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/package.json b/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/package.json new file mode 100644 index 0000000000..3dbc1ca591 --- /dev/null +++ b/packages/zip-it-and-ship-it/tests/fixtures/node-cjs-in-esm-scope/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/packages/zip-it-and-ship-it/tests/main.test.ts b/packages/zip-it-and-ship-it/tests/main.test.ts index 3baa04668d..0f8024750a 100644 --- a/packages/zip-it-and-ship-it/tests/main.test.ts +++ b/packages/zip-it-and-ship-it/tests/main.test.ts @@ -19,6 +19,7 @@ import { ESBUILD_LOG_LIMIT } from '../src/runtimes/node/bundlers/esbuild/bundler 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' @@ -511,6 +512,71 @@ describe('zip-it-and-ship-it', () => { }, ) + 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 }) })