feat: output cjs extension for bundles and entry-files (#1190)

* feat: output cjs & mjs extension if possible

with FF enabled
* esbuild outputs the bundle with cjs extension
* entry-file will be generated with cjs extension or mjs extension

* Update src/runtimes/node/utils/entry_file.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* chore: restructure code a little

* chore: better func name

* chore: refactor

* Apply suggestions from code review

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

Author: danez

.eslintrc.cjs

@@ -7,6 +7,7 @@ module.exports = {
   },
   rules: {
     'import/extensions': ['error', 'ignorePackages'],
+    'max-lines': 'off',
     'n/no-missing-import': 'off',
     'no-magic-numbers': 'off',
     'max-lines-per-function': 'off',

src/feature_flags.ts

@@ -20,6 +20,9 @@ export const defaultFlags: Record<string, boolean> = {
 
   // Load configuration from per-function JSON files.
   project_deploy_configuration_api_use_per_function_configuration_files: false,
+
+  // Output CJS file extension
+  zisi_output_cjs_extension: false,
 }
 
 export type FeatureFlag = keyof typeof defaultFlags

src/runtimes/node/bundlers/esbuild/bundler.ts

@@ -8,7 +8,7 @@ import { FeatureFlags } from '../../../../feature_flags.js'
 import { FunctionBundlingUserError } from '../../../../utils/error.js'
 import { getPathWithExtension, safeUnlink } from '../../../../utils/fs.js'
 import { RuntimeType } from '../../../runtime.js'
-import { getFileExtensionForFormat, ModuleFileExtension, ModuleFormat } from '../../utils/module_format.js'
+import { getFileExtensionForFormat, ModuleFormat } from '../../utils/module_format.js'
 import { NodeBundlerType } from '../types.js'
 
 import { getBundlerTarget, getModuleFormat } from './bundler_target.js'
@@ -102,11 +102,7 @@ export const bundleJsFile = async function ({
   )
 
   // The extension of the output file.
-  const extension = getFileExtensionForFormat(moduleFormat, featureFlags)
-
-  // When outputting an ESM file, configure esbuild to produce a `.mjs` file.
-  const outExtension =
-    moduleFormat === ModuleFormat.ESM ? { [ModuleFileExtension.JS]: ModuleFileExtension.MJS } : undefined
+  const outputExtension = getFileExtensionForFormat(moduleFormat, featureFlags)
 
   // We add this banner so that calls to require() still work in ESM modules, especially when importing node built-ins
   // We have to do this until this is fixed in esbuild: https://github.com/evanw/esbuild/pull/2067
@@ -130,8 +126,8 @@ export const bundleJsFile = async function ({
       logLimit: ESBUILD_LOG_LIMIT,
       metafile: true,
       nodePaths: additionalModulePaths,
-      outExtension,
       outdir: targetDirectory,
+      outExtension: { '.js': outputExtension },
       platform: 'node',
       plugins,
       resolveExtensions: RESOLVE_EXTENSIONS,
@@ -141,9 +137,9 @@ export const bundleJsFile = async function ({
     })
     const bundlePaths = getBundlePaths({
       destFolder: targetDirectory,
-      extension,
       outputs: metafile.outputs,
       srcFile,
+      outputExtension,
     })
     const inputs = Object.keys(metafile.inputs).map((path) => resolve(path))
     const cleanTempFiles = getCleanupFunction([...bundlePaths.keys()])
@@ -153,11 +149,11 @@ export const bundleJsFile = async function ({
       additionalPaths,
       bundlePaths,
       cleanTempFiles,
-      extension,
       inputs,
       moduleFormat,
       nativeNodeModules,
       nodeModulesWithDynamicImports: [...nodeModulesWithDynamicImports],
+      outputExtension,
       warnings,
     }
   } catch (error) {
@@ -175,14 +171,14 @@ export const bundleJsFile = async function ({
 // with the `aliases` format used upstream.
 const getBundlePaths = ({
   destFolder,
-  extension: outputExtension,
+  outputExtension,
   outputs,
   srcFile,
 }: {
   destFolder: string
-  extension: string
   outputs: Metafile['outputs']
   srcFile: string
+  outputExtension: string
 }) => {
   const bundleFilename = basename(srcFile, extname(srcFile)) + outputExtension
   const mainFileDirectory = dirname(srcFile)

src/runtimes/node/bundlers/esbuild/index.ts

@@ -68,11 +68,11 @@ const bundle: BundleFunction = async ({
     additionalPaths,
     bundlePaths,
     cleanTempFiles,
-    extension: outputExtension,
     inputs,
     moduleFormat,
     nativeNodeModules = {},
     nodeModulesWithDynamicImports,
+    outputExtension,
     warnings,
   } = await bundleJsFile({
     additionalModulePaths: pluginsModulesPath ? [pluginsModulesPath] : [],

src/runtimes/node/utils/entry_file.ts

@@ -1,23 +1,88 @@
-import { ModuleFormat } from './module_format.js'
+import { basename, extname, resolve } from 'path'
+
+import type { FeatureFlags } from '../../../feature_flags.js'
+
+import { getFileExtensionForFormat, ModuleFileExtension, ModuleFormat } from './module_format.js'
 import { normalizeFilePath } from './normalize_path.js'
 
+export interface EntryFile {
+  contents: string
+  filename: string
+}
+
+const getEntryFileContents = (mainPath: string, moduleFormat: string) => {
+  const importPath = `.${mainPath.startsWith('/') ? mainPath : `/${mainPath}`}`
+
+  if (moduleFormat === ModuleFormat.COMMONJS) {
+    return `module.exports = require('${importPath}')`
+  }
+
+  return `export { handler } from '${importPath}'`
+}
+
+// They are in the order that AWS Lambda will try to find the entry point
+const POSSIBLE_LAMBDA_ENTRY_EXTENSIONS = [ModuleFileExtension.JS, ModuleFileExtension.MJS, ModuleFileExtension.CJS]
+
+// checks if the file is considered a entry-file in AWS Lambda
+export const isNamedLikeEntryFile = (
+  file: string,
+  {
+    basePath,
+    filename,
+  }: {
+    basePath: string
+    filename: string
+  },
+) =>
+  POSSIBLE_LAMBDA_ENTRY_EXTENSIONS.some((extension) => {
+    const entryFilename = getEntryFileName({ extension, filename })
+    const entryFilePath = resolve(basePath, entryFilename)
+
+    return entryFilePath === file
+  })
+
+// Check if any src file (except the mainFile) is considered an entry file for AWS Lambda
+export const conflictsWithEntryFile = (
+  srcFiles: string[],
+  {
+    basePath,
+    mainFile,
+    filename,
+  }: {
+    basePath: string
+    filename: string
+    mainFile: string
+  },
+) => srcFiles.some((srcFile) => isNamedLikeEntryFile(srcFile, { basePath, filename }) && srcFile !== mainFile)
+
+// Returns the name for the AWS Lambda entry file
+// We do set the handler in AWS Lambda to `<func-name>.handler` and because of
+// this it considers `<func-name>.(c|m)?js` as possible entry-points
+const getEntryFileName = ({ extension, filename }: { extension: ModuleFileExtension; filename: string }) =>
+  `${basename(filename, extname(filename))}${extension}`
+
 export const getEntryFile = ({
   commonPrefix,
+  featureFlags,
+  filename,
   mainFile,
   moduleFormat,
   userNamespace,
 }: {
   commonPrefix: string
+  featureFlags: FeatureFlags
+  filename: string
   mainFile: string
   moduleFormat: ModuleFormat
   userNamespace: string
-}) => {
+}): EntryFile => {
   const mainPath = normalizeFilePath({ commonPrefix, path: mainFile, userNamespace })
-  const importPath = `.${mainPath.startsWith('/') ? mainPath : `/${mainPath}`}`
+  const extension = getFileExtensionForFormat(moduleFormat, featureFlags)
+  const entryFilename = getEntryFileName({ extension, filename })
+  const contents = getEntryFileContents(mainPath, moduleFormat)
 
-  if (moduleFormat === ModuleFormat.COMMONJS) {
-    return `module.exports = require('${importPath}')`
+  return {
+    contents,
+    filename: entryFilename,
   }
-
-  return `export { handler } from '${importPath}'`
 }

src/runtimes/node/utils/module_format.ts

@@ -19,5 +19,9 @@ export const getFileExtensionForFormat = (
     return ModuleFileExtension.MJS
   }
 
+  if (featureFlags.zisi_output_cjs_extension && moduleFormat === ModuleFormat.COMMONJS) {
+    return ModuleFileExtension.CJS
+  }
+
   return ModuleFileExtension.JS
 }

src/runtimes/node/utils/zip.ts

@@ -1,7 +1,7 @@
 import { Buffer } from 'buffer'
 import { Stats, promises as fs } from 'fs'
 import os from 'os'
-import { basename, join, resolve } from 'path'
+import { basename, join } from 'path'
 
 import { copyFile } from 'cp-file'
 import { deleteAsync as deleteFiles } from 'del'
@@ -12,8 +12,8 @@ import type { FeatureFlags } from '../../../feature_flags.js'
 import type { RuntimeCache } from '../../../utils/cache.js'
 import { cachedLstat, mkdirAndWriteFile } from '../../../utils/fs.js'
 
-import { getEntryFile } from './entry_file.js'
-import { getFileExtensionForFormat, ModuleFormat } from './module_format.js'
+import { conflictsWithEntryFile, EntryFile, getEntryFile, isNamedLikeEntryFile } from './entry_file.js'
+import { ModuleFormat } from './module_format.js'
 import { normalizeFilePath } from './normalize_path.js'
 
 // Taken from https://www.npmjs.com/package/cpy.
@@ -51,23 +51,22 @@ const createDirectory = async function ({
   rewrites = new Map(),
   srcFiles,
 }: ZipNodeParameters) {
-  const entryFile = getEntryFile({
+  const { contents: entryContents, filename: entryFilename } = getEntryFile({
     commonPrefix: basePath,
+    featureFlags,
+    filename,
     mainFile,
     moduleFormat,
     userNamespace: DEFAULT_USER_SUBDIRECTORY,
   })
-  const entryFileExtension = getFileExtensionForFormat(moduleFormat, featureFlags)
-  const entryFilename = basename(filename, extension) + entryFileExtension
   const functionFolder = join(destFolder, basename(filename, extension))
-  const entryFilePath = resolve(functionFolder, entryFilename)
 
   // Deleting the functions directory in case it exists before creating it.
   await deleteFiles(functionFolder, { force: true })
   await fs.mkdir(functionFolder, { recursive: true })
 
   // Writing entry file.
-  await fs.writeFile(entryFilePath, entryFile)
+  await fs.writeFile(join(functionFolder, entryFilename), entryContents)
 
   // Copying source files.
   await pMap(
@@ -108,27 +107,35 @@ const createZipArchive = async function ({
 }: ZipNodeParameters) {
   const destPath = join(destFolder, `${basename(filename, extension)}.zip`)
   const { archive, output } = startZip(destPath)
-  const entryFileExtension = getFileExtensionForFormat(moduleFormat, featureFlags)
-  const entryFilename = basename(filename, extension) + entryFileExtension
-  const entryFilePath = resolve(basePath, entryFilename)
 
   // We don't need an entry file if it would end up with the same path as the
   // function's main file.
-  const needsEntryFile = entryFilePath !== mainFile
+  const needsEntryFile = !isNamedLikeEntryFile(mainFile, { basePath, filename })
 
   // There is a naming conflict with the entry file if one of the supporting
   // files (i.e. not the main file) has the path that the entry file needs to
   // take.
-  const hasEntryFileConflict = srcFiles.some((srcFile) => srcFile === entryFilePath && srcFile !== mainFile)
+  const hasEntryFileConflict = conflictsWithEntryFile(srcFiles, {
+    basePath,
+    filename,
+    mainFile,
+  })
 
   // If there is a naming conflict, we move all user files (everything other
   // than the entry file) to its own sub-directory.
   const userNamespace = hasEntryFileConflict ? DEFAULT_USER_SUBDIRECTORY : ''
 
   if (needsEntryFile) {
-    const entryFile = getEntryFile({ commonPrefix: basePath, mainFile, moduleFormat, userNamespace })
+    const entryFile = getEntryFile({
+      commonPrefix: basePath,
+      filename,
+      mainFile,
+      moduleFormat,
+      userNamespace,
+      featureFlags,
+    })
 
-    addEntryFileToZip(archive, entryFile, basename(entryFilePath))
+    addEntryFileToZip(archive, entryFile)
   }
 
   const srcFilesInfos = await Promise.all(srcFiles.map((file) => addStat(cache, file)))
@@ -163,7 +170,7 @@ export const zipNodeJs = function ({
   return createDirectory(options)
 }
 
-const addEntryFileToZip = function (archive: ZipArchive, contents: string, filename: string) {
+const addEntryFileToZip = function (archive: ZipArchive, { contents, filename }: EntryFile) {
   const contentBuffer = Buffer.from(contents)
 
   addZipContent(archive, contentBuffer, filename)

tests/fixtures/node-js-extension/func1.js

@@ -0,0 +1 @@
+module.exports.handler = () => true

tests/fixtures/node-js-extension/func2/func2.js

@@ -0,0 +1 @@
+module.exports.handler = () => true

tests/fixtures/node-js-extension/func3/index.js

@@ -0,0 +1 @@
+module.exports.handler = () => true

tests/main.test.ts

@@ -1000,7 +1000,7 @@ describe('zip-it-and-ship-it', () => {
 
   testMany(
     'Handles a JavaScript function ({name}.cjs, {name}/{name}.cjs, {name}/index.cjs)',
-    ['bundler_esbuild', 'bundler_default'],
+    ['bundler_esbuild'],
     async (options) => {
       const { files, tmpDir } = await zipFixture('node-cjs-extension', {
         length: 3,
@@ -1023,6 +1023,32 @@ describe('zip-it-and-ship-it', () => {
     },
   )
 
+  // TODO can be merged with the above once the FF is on `zisi_output_cjs_extension`
+  testMany(
+    'Handles a JavaScript function ({name}.cjs, {name}/{name}.cjs, {name}/index.cjs)',
+    ['bundler_default'],
+    async (options) => {
+      const { files, tmpDir } = await zipFixture('node-cjs-extension', {
+        length: 3,
+        opts: options,
+      })
+
+      await unzipFiles(files)
+
+      expect(files).toHaveLength(3)
+      files.forEach((file) => {
+        expect(file.bundler).toBe(options.getCurrentBundlerName() ?? 'zisi')
+      })
+
+      const { handler: handler1 } = await importFunctionFile(`${tmpDir}/func1.cjs`)
+      expect(handler1()).toBe(true)
+      const { handler: handler2 } = await importFunctionFile(`${tmpDir}/func2.cjs`)
+      expect(handler2()).toBe(true)
+      const { handler: handler3 } = await importFunctionFile(`${tmpDir}/func3.js`)
+      expect(handler3()).toBe(true)
+    },
+  )
+
   testMany(
     'Loads a tsconfig.json placed in the same directory as the function',
     ['bundler_default', 'bundler_esbuild', 'bundler_esbuild_zisi', 'bundler_default_nft', 'todo:bundler_nft'],
@@ -2445,4 +2471,81 @@ describe('zip-it-and-ship-it', () => {
     expect(result.stderr).not.toContain('Dynamic require of "path" is not supported')
     expect(result).not.toBeInstanceOf(Error)
   })
+
+  testMany(
+    'Emits entry file with .cjs extension when `zisi_output_cjs_extension` flag is on',
+    ['bundler_default', 'bundler_esbuild', 'bundler_nft'],
+    async (options) => {
+      const fixtureName = 'node-esm'
+      const opts = merge(options, {
+        basePath: join(FIXTURES_DIR, fixtureName),
+        featureFlags: { zisi_output_cjs_extension: true },
+      })
+      const { files, tmpDir } = await zipFixture(fixtureName, {
+        length: 2,
+        opts,
+      })
+
+      await unzipFiles(files)
+
+      const bundledFile2 = await importFunctionFile(join(tmpDir, 'func2.cjs'))
+      expect(bundledFile2.handler()).toBe(true)
+
+      if (options.getCurrentBundlerName() === 'esbuild') {
+        // nft does not create an entry here because the main file is already an entrypoint
+        const bundledFile1 = await importFunctionFile(join(tmpDir, 'func1.cjs'))
+        expect(bundledFile1.handler()).toBe(true)
+      }
+    },
+  )
+
+  testMany('Keeps .cjs extension', ['bundler_default', 'bundler_nft'], async (options) => {
+    const fixtureName = 'node-cjs-extension'
+    const opts = merge(options, {
+      basePath: join(FIXTURES_DIR, fixtureName),
+    })
+    const { files, tmpDir } = await zipFixture(fixtureName, {
+      length: 3,
+      opts,
+    })
+
+    await unzipFiles(files)
+
+    const bundledFile1 = await importFunctionFile(join(tmpDir, 'func1.cjs'))
+    const bundledFile2 = await importFunctionFile(join(tmpDir, 'func2.cjs'))
+    const bundledFile3 = await importFunctionFile(join(tmpDir, 'index.cjs'))
+
+    expect(bundledFile1.handler()).toBe(true)
+    expect(bundledFile2.handler()).toBe(true)
+    expect(bundledFile3.handler()).toBe(true)
+  })
+
+  testMany(
+    'Does not create .cjs entry file if entry with .js extension is already present',
+    ['bundler_default', 'bundler_nft'],
+    async (options) => {
+      const fixtureName = 'node-js-extension'
+      const opts = merge(options, {
+        basePath: join(FIXTURES_DIR, fixtureName),
+        featureFlags: { zisi_output_cjs_extension: true },
+      })
+      const { files, tmpDir } = await zipFixture(fixtureName, {
+        length: 3,
+        opts,
+      })
+
+      await unzipFiles(files)
+
+      const bundledFile1 = await importFunctionFile(join(tmpDir, 'func1.js'))
+      const bundledFile2 = await importFunctionFile(join(tmpDir, 'func2.js'))
+
+      expect(bundledFile1.handler()).toBe(true)
+      expect(bundledFile2.handler()).toBe(true)
+
+      expect(`${tmpDir}/func1.cjs`).not.toPathExist()
+      expect(`${tmpDir}/func1.mjs`).not.toPathExist()
+      expect(`${tmpDir}/func2.cjs`).not.toPathExist()
+      expect(`${tmpDir}/func2.mjs`).not.toPathExist()
+    },
+  )
 })