fix: keep the CommonJS interop shim out of the ESM build (#346)

AGENTS.md

@@ -59,6 +59,23 @@ api.use((err, req, res, next) => {
 });
 ```
 
+## Build
+
+`src/` is **pure ESM** and is compiled twice by SWC — to `dist/cjs` and to `dist/esm`. The
+CommonJS interop that makes `require('lambda-api')` callable is appended to the CJS artifact
+_only_, by `scripts/cjs-interop.js`.
+
+Never put a `module.exports` / `typeof module` write in `src/`. It would also land in `dist/esm`,
+and bundlers that inline the ESM artifact into a generated CommonJS wrapper (esbuild
+`--format=cjs`, AWS CDK `NodejsFunction`, SST, Serverless) execute it against the _consumer's_
+`module`, wiping out their exports — on Lambda that reads as `Runtime.HandlerNotFound`
+(issue #346). `__tests__/module-compat.unit.js` fails the build if one reappears.
+
+The footer is derived per file, not enumerated: a module whose only export is `default` collapses
+to that value, a module with named exports is left as SWC emitted it, and a module with both fails
+the build until you add an explicit entry to `EXCEPTIONS` in `scripts/cjs-interop.js`. New files
+are covered automatically.
+
 ## Testing
 
 - Tests live in `__tests__/*.unit.js`
@@ -72,6 +89,7 @@ api.use((err, req, res, next) => {
 
 - Add external npm dependencies (zero-dependency policy is non-negotiable)
 - Introduce breaking changes to the public API
+- Write to `module.exports` or `exports` from `src/` — see Build below
 
 **Always do:**
 

__tests__/cjs-interop.unit.js

@@ -0,0 +1,88 @@
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+
+const {
+  footerFor,
+  COLLAPSE_TO_DEFAULT,
+  EXCEPTIONS,
+} = require('../scripts/cjs-interop.js');
+
+const SRC = path.join(__dirname, '..', 'src');
+
+// The build step decides each module's CommonJS shape from its ESM source. Getting that
+// classification wrong is silent in the worst direction — a module that mixes a default with
+// named exports would collapse to the default and DROP the rest — so every export form is
+// pinned here rather than only the ones src/ happens to use today.
+describe('cjs-interop footer classification:', function () {
+  describe('collapses to the default export', function () {
+    const cases = {
+      'export default': 'const t = () => 1;\nexport default t;\n',
+      'export default class': 'export default class T {}\n',
+      'export default function': 'export default function t() {}\n',
+      'export default object': 'export default { a: 1 };\n',
+    };
+
+    Object.keys(cases).forEach((name) => {
+      it(name, function () {
+        expect(footerFor('lib/probe.js', cases[name])).toBe(
+          COLLAPSE_TO_DEFAULT
+        );
+      });
+    });
+  });
+
+  describe('leaves SWC output alone', function () {
+    const cases = {
+      'export const': 'export const a = 1;\n',
+      'export async function': 'export async function a() {}\n',
+      'export list': 'const a = 1;\nexport { a };\n',
+      'export star': "export * from './x.js';\n",
+      'no exports at all': 'const a = 1;\n',
+    };
+
+    Object.keys(cases).forEach((name) => {
+      it(name, function () {
+        expect(footerFor('lib/probe.js', cases[name])).toBeNull();
+      });
+    });
+  });
+
+  describe('refuses ambiguous shapes rather than dropping exports', function () {
+    const cases = {
+      'default + const': 'export const a = 1;\nexport default a;\n',
+      'default + async function':
+        'export async function a() {}\nconst t = 1;\nexport default t;\n',
+      'default + star': "export * from './x.js';\nexport default 1;\n",
+      'default + named list':
+        'const a = 1;\nconst t = 2;\nexport { a };\nexport default t;\n',
+      'export { x as default }': 'const t = 1;\nexport { t as default };\n',
+    };
+
+    Object.keys(cases).forEach((name) => {
+      it(name, function () {
+        expect(() => footerFor('lib/probe.js', cases[name])).toThrow(
+          /ambiguous|silently drop/
+        );
+      });
+    });
+  });
+
+  describe('exceptions win over the rule', function () {
+    Object.keys(EXCEPTIONS).forEach((relative) => {
+      it(`${relative} uses its explicit footer`, function () {
+        // Passing source that the rule would classify differently proves the override applies.
+        expect(footerFor(relative, 'export const a = 1;\n')).toBe(
+          EXCEPTIONS[relative]
+        );
+      });
+    });
+
+    it('every exception names a file that still exists in src/', function () {
+      Object.keys(EXCEPTIONS).forEach((relative) => {
+        expect(fs.existsSync(path.join(SRC, relative))).toBe(true);
+      });
+    });
+  });
+});

__tests__/esm-compat.mjs

@@ -4,6 +4,7 @@ import createAPI from '../dist/esm/index.js';
 import * as utils from '../dist/esm/lib/utils.js';
 import prettyPrint from '../dist/esm/lib/prettyPrint.js';
 import { ApiError } from '../dist/esm/lib/errors.js';
+import * as s3 from '../dist/esm/lib/s3-service.js';
 
 const event = {
   httpMethod: 'GET',
@@ -43,6 +44,17 @@ async function main() {
   if (JSON.parse(result.body).ok !== true) {
     throw new Error('Expected successful ESM route response');
   }
+
+  for (const name of ['setConfig', 'getObject', 'getSignedUrl']) {
+    if (typeof s3[name] !== 'function') {
+      throw new Error(`Expected s3-service to export ${name} as a function`);
+    }
+  }
+
+  // The CJS artifact collapses to this object; under ESM it stays a plain named export.
+  if (typeof s3.service !== 'object' || typeof s3.service.getObject !== 'function') {
+    throw new Error('Expected s3-service to export the service object');
+  }
 }
 
 main().catch((error) => {

__tests__/module-compat.unit.js

@@ -1,8 +1,11 @@
 'use strict';
 
 const { execFileSync } = require('child_process');
+const fs = require('fs');
 const path = require('path');
 
+const DIST = path.join(__dirname, '..', 'dist');
+
 const event = {
   httpMethod: 'GET',
   path: '/compat',
@@ -18,6 +21,14 @@ const runRoute = async (api) => {
   return api.run(event, {});
 };
 
+// Recursively collect every compiled .js file under a dist directory.
+const jsFilesIn = (dir) =>
+  fs.readdirSync(dir, { withFileTypes: true }).reduce((acc, entry) => {
+    const full = path.join(dir, entry.name);
+    if (entry.isDirectory()) return acc.concat(jsFilesIn(full));
+    return entry.name.endsWith('.js') ? acc.concat(full) : acc;
+  }, []);
+
 describe('Module Compatibility Tests:', function () {
   describe('CommonJS build output', function () {
     it('loads the package factory from dist/cjs', function () {
@@ -52,13 +63,73 @@ describe('Module Compatibility Tests:', function () {
     });
   });
 
+  // The CommonJS interop shape below is what `scripts/cjs-interop.js` appends to dist/cjs. It is
+  // pinned here because the source is pure ESM: nothing in src/ produces this shape any more.
+  describe('CommonJS interop shape (dist/cjs only)', function () {
+    it('exports the default value itself for single-default modules', function () {
+      expect(typeof require('../dist/cjs/lib/request.js')).toBe('function');
+      expect(typeof require('../dist/cjs/lib/response.js')).toBe('function');
+      expect(typeof require('../dist/cjs/lib/prettyPrint.js')).toBe('function');
+      expect(typeof require('../dist/cjs/lib/statusCodes.js')[404]).toBe(
+        'string'
+      );
+      expect(typeof require('../dist/cjs/lib/mimemap.js').json).toBe('string');
+    });
+
+    it('does not graft a .default self-reference onto the lib modules', function () {
+      // `.default` is only correct on the package root. On mimemap in particular it would make
+      // a `default` file extension resolve to the whole map.
+      expect(require('../dist/cjs/lib/mimemap.js').default).toBeUndefined();
+      expect(require('../dist/cjs/lib/statusCodes.js').default).toBeUndefined();
+    });
+
+    it('exposes s3-service as a single mutable object that sinon can stub', function () {
+      // response.js reads S3 methods through this same object, and four unit suites do
+      // `sinon.stub(require('../lib/s3-service'), 'getSignedUrl')`. SWC's own `_export()` emits
+      // non-configurable getters, which would make stubbing throw.
+      const s3 = require('../dist/cjs/lib/s3-service.js');
+
+      expect(s3.__esModule).toBe(true);
+      expect('client' in s3).toBe(true);
+
+      ['getObject', 'getSignedUrl', 'setConfig'].forEach((method) => {
+        const descriptor = Object.getOwnPropertyDescriptor(s3, method);
+        expect(typeof s3[method]).toBe('function');
+        expect(descriptor.writable).toBe(true);
+        expect(descriptor.configurable).toBe(true);
+      });
+    });
+  });
+
   describe('ESM build output', function () {
     it('passes Node ESM compatibility checks', function () {
       execFileSync(process.execPath, ['__tests__/esm-compat.mjs'], {
         cwd: path.join(__dirname, '..'),
         stdio: 'pipe',
       });
     });
+
+    // Regression guard for issue #346. Bundlers that inline the ESM artifact into a generated
+    // CommonJS wrapper (esbuild --format=cjs, CDK NodejsFunction, SST, Serverless) leave exactly
+    // one `module` in scope: the CONSUMER's. Any write to it from here silently replaces the
+    // consumer's exports, which on Lambda surfaces as `Runtime.HandlerNotFound`.
+    it('never touches the CommonJS module system (issue #346)', function () {
+      const esm = path.join(DIST, 'esm');
+      const patterns = [
+        /\bmodule\s*\.\s*exports\b/,
+        /\btypeof\s+module\b/,
+        /(?:^|[^.\w$])exports\s*(?:\.|\[)/,
+      ];
+
+      const offenders = jsFilesIn(esm)
+        .filter((file) => {
+          const source = fs.readFileSync(file, 'utf8');
+          return patterns.some((pattern) => pattern.test(source));
+        })
+        .map((file) => path.relative(esm, file));
+
+      expect(offenders).toEqual([]);
+    });
   });
 
   describe('Package exports resolution', function () {

e2e/fixtures/esm-to-cjs-bundle/handler.mjs

@@ -0,0 +1,9 @@
+// Issue #346: an ESM handler bundled to a single CommonJS file — the shape produced by
+// AWS CDK `NodejsFunction`, SST and Serverless Framework. esbuild resolves lambda-api through
+// the `import` condition and inlines dist/esm into a generated CJS wrapper.
+import createAPI from 'lambda-api';
+
+const api = createAPI({ version: 'v1' });
+api.get('/', (req, res) => res.json({ hello: 'world', lang: 'esm-to-cjs' }));
+
+export const handler = async (event, context) => api.run(event, context);

e2e/fixtures/esm-to-cjs-bundle/invoke.cjs

@@ -0,0 +1,28 @@
+'use strict';
+// Loads the esbuild CJS bundle exactly the way the AWS Lambda Node runtime does — a plain
+// `require()` followed by a lookup of the named export — and reports what actually survived.
+const { readFileSync } = require('fs');
+
+const bundle = require('./bundle.cjs');
+const event = JSON.parse(readFileSync(process.argv[2], 'utf8'));
+
+const out = {
+  keys: Object.keys(bundle),
+  handlerType: typeof bundle.handler,
+  response: null,
+};
+
+Promise.resolve()
+  .then(() =>
+    out.handlerType === 'function'
+      ? bundle.handler(event, { getRemainingTimeInMillis: () => 3000 })
+      : null
+  )
+  .then((response) => {
+    out.response = response;
+    process.stdout.write(JSON.stringify(out));
+  })
+  .catch((e) => {
+    process.stderr.write(String((e && e.stack) || e));
+    process.exit(1);
+  });

e2e/fixtures/esm-to-cjs-bundle/package.json

@@ -0,0 +1 @@
+{ "name": "fixture-esm-to-cjs-bundle", "private": true, "type": "module" }

e2e/run-layer1.mjs

@@ -143,6 +143,34 @@ section('esbuild bundle (issue #295: no "Dynamic require")');
   });
 }
 
+// 4b. issue #346 — an ESM entry bundled to CJS must not lose the consumer's own exports.
+//     esbuild resolves lambda-api through the `import` condition here and inlines dist/esm into
+//     a generated CommonJS wrapper, so any `module.exports = ...` in the ESM artifact lands on
+//     the BUNDLE's exports. On Lambda that shows up as `Runtime.HandlerNotFound`.
+section('esbuild ESM entry -> cjs output (issue #346: consumer exports survive)');
+{
+  const dir = stageFixture(base, 'esm-to-cjs-bundle');
+  check('bundles, keeps `handler` export, and returns 200', () => {
+    try {
+      execFileSync(
+        esbuildBin,
+        ['handler.mjs', '--bundle', '--format=cjs', '--platform=node', '--external:@aws-sdk/*', '--outfile=bundle.cjs'],
+        { cwd: dir, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }
+      );
+    } catch (e) {
+      throw new Error(`esbuild failed: ${(e.stderr || '') + (e.stdout || '')}`);
+    }
+    const r = parseResponse(runNode(dir, 'invoke.cjs', ev1), 'esm-to-cjs-bundle');
+    assert(
+      r.handlerType === 'function',
+      `bundle exports ${JSON.stringify(r.keys)} — handler is ${r.handlerType} (issue #346 regression)`
+    );
+    assert(r.keys.includes('handler'), `expected 'handler' in exports, got ${JSON.stringify(r.keys)}`);
+    assert(r.response && r.response.statusCode === 200, `status ${r.response && r.response.statusCode}`);
+    assert(JSON.parse(r.response.body).hello === 'world', 'body.hello');
+  });
+}
+
 // 5. exports map subpath resolution (root, lib/*, lib/*.js, package.json)
 section('exports map subpath resolution');
 {

package.json

@@ -38,7 +38,7 @@
   },
   "scripts": {
     "clean": "rm -rf dist",
-    "build:cjs": "swc src -d dist/cjs --config-file .swcrc.cjs.json --strip-leading-paths",
+    "build:cjs": "swc src -d dist/cjs --config-file .swcrc.cjs.json --strip-leading-paths && node scripts/cjs-interop.js",
     "build:esm": "swc src -d dist/esm --config-file .swcrc.esm.json --copy-files --strip-leading-paths",
     "build:types": "tsc -p tsconfig.types.json && cp dist/types/lib/*.d.ts dist/cjs/lib/ && cp dist/types/lib/*.d.ts dist/esm/lib/ && rm -rf dist/types",
     "build": "npm run clean && npm run build:cjs && npm run build:esm && npm run build:types",