Add dual CJS/ESM build#326
Merged
Merged
Conversation
Collaborator
|
Hey @tomassabol, Can you please resolve the CI errors? 🙏 |
There was a problem hiding this comment.
Pull request overview
This PR introduces a dual-build distribution strategy for lambda-api, adding first-class ESM output while preserving CommonJS compatibility via conditional exports and SWC-built artifacts in dist/cjs and dist/esm.
Changes:
- Migrate source to
src/and compile todist/cjs+dist/esmusing SWC. - Add an
exportsmap to supportrequire()/import(including deep./lib/*subpaths). - Add module-compatibility tests validating CJS + ESM loading and basic runtime behavior.
Reviewed changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/package.json | Marks src scope as ESM (type: module) to drive ESM compilation behavior. |
| src/index.js | Converts entrypoint to ESM syntax and adds CJS fallback export logic. |
| src/lib/utils.js | Converts utilities to ESM exports/imports and removes CJS exports.* usage. |
| src/lib/statusCodes.js | Switches to ESM default export with CJS fallback assignment. |
| src/lib/s3-service.js | Adds new ESM S3 service implementation intended for both builds. |
| src/lib/response.js | Converts response module to ESM and updates S3 usage sites. |
| src/lib/request.js | Converts request module to ESM default export with CJS fallback. |
| src/lib/prettyPrint.js | Switches to ESM default export with CJS fallback assignment. |
| src/lib/mimemap.js | Switches to ESM default export with CJS fallback assignment. |
| src/lib/logger.js | Converts logger module exports to ESM named exports. |
| src/lib/errors.js | Converts error exports to ESM named exports. |
| src/lib/compression.js | Converts compression module to ESM named export(s). |
| package.json | Adds SWC build scripts, exports map, and publishes dist/ output. |
| lib/s3-service.js | Removes legacy root lib/ S3 service (source now under src/). |
| jest.config.js | Adds Jest mapping to test against built dist/cjs artifacts. |
| index.test-d.ts | Updates type tests to validate default factory typing alignment. |
| .swcrc.esm.json | Adds SWC config for ESM output build. |
| .swcrc.cjs.json | Adds SWC config for CJS output build. |
| .gitignore | Ignores generated dist/ output. |
| .eslintrc.json | Adjusts ESLint parsing defaults with ESM overrides for src/**/*.js and __tests__/**/*.mjs. |
| tests/module-compat.unit.js | Adds CJS/exports-resolution compatibility tests. |
| tests/esm-compat.mjs | Adds Node ESM runtime compatibility smoke test for the ESM build. |
Comments suppressed due to low confidence (6)
src/index.js:14
- Importing the S3 helper at module load time makes the optional AWS SDK peerDependencies effectively required. Since src/lib/s3-service.js imports @aws-sdk/* at top-level, this static import can throw on startup for users who do not install the optional peers (even if they never use S3 features). Consider loading/configuring S3 lazily from response methods instead.
src/index.js:53 - This eager S3 client configuration will also break if the optional AWS SDK peerDependencies are not installed (and if S3 is no longer statically imported). Since S3 is only needed for getLink/sendFile with s3:// paths, defer S3 module loading/config until those helpers are invoked.
src/lib/response.js:13 - Static-importing ./s3-service.js causes the module to attempt loading optional @aws-sdk/* peerDependencies during package initialization, which can fail for users that don't install them. To preserve the current "optional S3" behavior, load the S3 service lazily only when getLink()/sendFile(s3://...) are used.
src/lib/response.js:200 - After switching to lazy-loading the S3 service, getLink() should resolve the S3 module before invoking getSignedUrl so the AWS SDK peer dependencies are only required when this method is called.
src/lib/response.js:342 - After switching to lazy-loading the S3 service, resolve/configure the S3 module before calling getObject() so users without the optional AWS SDK peer dependencies can still use non-S3 sendFile() paths.
src/index.js:570 - The CommonJS entrypoint previously set
module.exports.default = module.exportsto support consumers that access the factory via.default(and to align with the TypeScript default export). With the newmodule.exports = createAPIassignment, that compatibility property is dropped and can be a breaking change for existing CJS consumers.
# Conflicts: # package-lock.json
The SWC build toolchain requires Node >= 16.14, so the Node 14 test job could no longer build. Restructure CI to build the CJS/ESM output once on Node 20 and share dist/ as an artifact, then run the Node 14-22 test matrix against that prebuilt output (npm run test:prebuilt). This preserves runtime testing on all supported Node versions without building on the ones the toolchain can't run on. - pull-request.yml / release.yml: add a build job (build + tsd + upload dist), make the test matrix download dist and run tests without rebuilding - build.yml: bump the coverage job from Node 14 to 20 (build tooling floor) - add the test:prebuilt script (jest unit against an existing dist/)
The dual CJS/ESM refactor turned lambda-api's lazy S3 loading into static
top-level imports: src/index.js and src/lib/response.js statically import
s3-service, which statically imported @aws-sdk/*. That made require('lambda-api')
/ import 'lambda-api' throw "Cannot find module '@aws-sdk/client-s3'" whenever
the OPTIONAL peer deps weren't installed — breaking every non-S3 consumer — and
constructed an S3Client at module-eval time on every import.
Fix (src/lib/s3-service.js only): load @aws-sdk and construct the S3 client
lazily via dynamic import() inside the service, on first real S3 use. Keeps the
named exports getObject/getSignedUrl/setConfig and the synchronous setConfig the
existing unit tests rely on (sinon stubs + constructor assertion). SWC compiles
import() to a lazy require() in the CJS build and keeps native import() in ESM.
Also add "./package.json" to the exports map.
Add an extensive e2e suite (e2e/, excluded from the tarball):
- Layer 1 (no Docker): installs the packed tarball into isolated CJS/ESM/deep-
import/esbuild-.mjs(jeremydaly#295)/TypeScript-node16/S3/without-aws-sdk fixtures and
asserts responses. 15/15.
- Layer 2 (LocalStack): deploys CJS, ESM, an esbuild-bundled .mjs, and an S3
handler to real nodejs20.x Lambda + real S3 + API Gateway, invokes them and
checks logs for the jeremydaly#295 'Dynamic require' error. 6/6.
- npm scripts test:e2e / test:e2e:localstack; manual e2e.yml workflow.
Existing jest unit + module-compat + tsd stay green (484 tests).
response.getLink() invokes s3-service.getSignedUrl in callback style and ignores its returned promise, relying on the callback always firing. The lazy-import refactor could reject getSignedUrl (SDK missing) before the callback ran, turning the ignored promise into an unhandled rejection (which crashes the process under Node's default) instead of the graceful 500 main produced. Wrap getSignedUrl in try/catch so it always resolves via the callback and never rejects — restoring the original contract. Add an e2e guard (s3-no-sdk fixture): an S3 route without @aws-sdk must return a handled 5xx and not crash. Layer 1 now 17/17.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 55 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- e2e/localstack/package-lock.json: Generated file
Comments suppressed due to low confidence (1)
src/index.js:570
- The CommonJS entrypoint previously exposed a
.defaultalias (module.exports.default = module.exports) to match the default export inindex.d.ts. Dropping that alias can be a breaking change for CJS consumers doingrequire('lambda-api').default(or for some TS default-import interop setups). Consider restoring the.defaultalias while keepingmodule.exportsas the factory function.
Two regressions found in an adversarial audit of the dual-package refactor:
1. require('lambda-api').default was undefined. main ended index.js with
'module.exports.default = module.exports' (explicitly 'to match index.d.ts'),
which the refactor dropped. This breaks TS consumers compiled to CommonJS with
esModuleInterop:false — they emit require('lambda-api').default(...) which
type-checks clean but throws at runtime. Restore the self-reference in the CJS
build guard and guard it with a module-compat test.
2. No prepare/prepack hook: dist is gitignored and was built only by
prepublishOnly (publish only). So 'npm pack' and 'npm install <git-url>'
shipped a package with no dist/ — broken. main shipped source directly, so
this is a regression for git installs. Add "prepare": "npm run build".
Because prepare now builds on 'npm install', switch CI 'npm ci' to
'npm ci --ignore-scripts' (the Node 14 test-matrix jobs can't run the SWC build;
they download the prebuilt dist artifact and the build job builds explicitly).
Existing suite stays green (485 tests incl. the new .default guard); tsd, e2e
Layer 1 (17/17) unaffected.
The exports map's ./lib/* subpaths had no `types` condition and there were no declaration files, so deep imports like `lambda-api/lib/utils` failed to resolve types under node16 (TS7016). Generate .d.ts from the JS source with tsc (build:types) into dist/cjs/lib and dist/esm/lib, and add per-condition `types` to the ./lib/* and ./lib/*.js exports entries. Verified: deep imports (named/default/namespace/.js subpath, import + require) resolve and type-check clean under moduleResolution node16 with esModuleInterop on and off; root import + existing 485 tests + tsd + e2e Layer 1 unaffected. Adds typescript as a devDependency for tsc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds dual CommonJS and ESM support.
Until now, the package has only shipped as CommonJS, which can make modern ESM bundling workflows awkward. In particular, users bundling Lambda handlers to
.mjswith tools like esbuild can hit dynamicrequire()issues, as described in #295.This change keeps the existing CommonJS API working while adding a proper ESM build and package export map.
What Changed
src/and builds published output intodist/cjsanddist/esmwith SWCrequire()andimportlambda-api/lib/utilsWhy
This should make
lambda-apifriendlier for modern Lambda projects using ESM,.mjsentrypoints, or bundlers like esbuild, while remaining backwards compatible for existing CommonJS users.