Support CommonJS (again)

[eyalroth]

Please bring back support for CommonJS.

We have a lot of serverless projects (dozens) in NodeJS. They are all written in CommonJS.

There is very little chance that we will migrate to ESM, since it will require a lot of work. Most notably, we rely heavily on Jest, which doesn't support ESM properly.

If Middy won't support CommonJS, we may have to migrate away from it at some point.

[willfarrell]

We have no plans to re-induce cjs support. However, the nodejs team may have a solution for you in version 22 with the new --experimental-require-module flag. From my understanding this would address the issue you're experience. If you do end up experimenting with it, please share your finding for others.

[willfarrell]

It just landed in Node 20.17 https://nodejs.org/en/blog/release/v20.17.0

[eyalroth]

@willfarrell It doesn't seem to help.

Running:

node --experimental-require-module node_modules/.bin/jest

And getting the same error:

Cannot find module '@middy/core' from '../../src/foo.ts'

And foo.ts:

import middy from '@middy/core';

NodeJS: 20.17.0 Jest: 29.7.0 Middy (core): 5.4.6

jest.config.js:

module.exports = (testsType) => ({
  preset: "ts-jest",
  testEnvironment: "node",
  modulePathIgnorePatterns: ['dist'],
  testMatch: ['<rootDir>/**/*.test.ts'],
  resetMocks: true,
  transform: {
    '^.+test/.*\\.ts$': ['ts-jest', {
      tsconfig: "<rootDir>/tsconfig.json",
    }]
  },
});

tsconfig.json:

{
  "compilerOptions": {
    "target": "ES2019",
    "module": "commonjs",
    "lib": ["ES2019", "es2020"],
    "allowJs": true,
    "rootDir": "./",
    "strict": true,
    "noImplicitAny": false,
    "esModuleInterop": true,
    "resolveJsonModule": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "incremental": true
  },
  "include": ["src/**/*", "test/**/*"],
  "exclude": ["node_modules/**/*"]
}

[willfarrell]

Thanks for testing. Based on the node release notes this should have allowed middy to be imported using require based on how it's currently configured. I ran a quick test to confirm:

const { default: middy } = require('@middy/core')

const baseHandler = async (event, context, { signal }) => {
  console.log('success')
}
middy(baseHandler)()

node --experimental-require-module index.cjs

Which, as expected, passed. This leads me to believe that there is an issue somewhere in your build process, likely typescript. I would recommend opening an issue for typescript to support transpiling to vanilla CommonJS so --experimental-require-module can be leveraged.

Edit: We might need to push a small config change (was modifying too many files at once). Looks like the following is needed in the package.json:

{"exports":{".":{
      "require":{
        "default": "./index.js"
      }
 }}}

[eyalroth]

@willfarrell This is happening when running jest, which is known not to support ESM properly.

Using require instead of import changes nothing.

Adding the exports to the package.json of @middy/core then raises this error:

    /path/to/repo/node_modules/@middy/core/index.js:2
    import { Readable } from 'node:stream'
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      12 |
      13 | // import middy from '@middy/core';
    > 14 | const { default: middy } = require('@middy/core');

Given how fundamental both Typescript and Jest are for nodejs codebases, I would highly encourage making middy compatible with them; otherwise, I fear devs would abandon middy.

We are aware of the exclusive ESM feature of running top-level await, and how it can help with cold starts; however, this is only relevant to provisioned concurrency, which is a very costly solution, especially with non-consistent traffic.

[willfarrell]

Thanks for the additional testing. I know Jest has been working on ESM support, curious how close they are.

however, this is only relevant to provisioned concurrency

In my experience this is not true. The platform I maintain has a high cold start hit rate (>80%), migrating to ESM had a notable improvement for us at the time. We don't use provisioned concurrency because, as you mentioned, it is a very costly solution. We do take advantage of the warmup middleware on a few select endpoints, worth checking out if you don't already use it.

[eyalroth]

@willfarrell It's strange that you saw any change without provisioned concurrency.

Without provisioned concurrency, the top-level code of the lambda will start running only on the first request to the container, so it doesn't really matter if it runs in the top-level or inside the handler itself.

Maybe there's a general improvement of imports/requires, but I haven't seen any post about it from AWS.