add ESM support (take 2)

[davidjgoss]

Support ESM usage with minimal additional flags/configuration. Re-adds the changes originally done in #1589, which were reverted due to the issues described in #1646 (which is now addressed). Fixes #1304.

To allow importing cucumber functions from ESM support code, provide a "wrapper" mjs entry point for ESM projects as described in https://nodejs.org/api/packages.html#packages_approach_1_use_an_es_module_wrapper.

Usage:

cucumber-js --esm

The --esm flag causes us to use import() instead of require() for loading support code - the former works for both ESM and commonjs, where the latter only works for commonjs.

I think we need to consider usage of this in conjunction with on-the-fly transpilers as out of scope for now. For example ts-node is at a very experimental phase with ESM per TypeStrong/ts-node#1007.

Test project: https://github.com/davidjgoss/cucumber-esm-example

TODO:

• Handle cucumber config file in an ESM project (fixes Cannot find cli config file if it has extension .cjs #1771)
• Test require-module compatibility with TypeScript, Babel and CoffeeScript

[davidjgoss]

Added a scenario to prove the issue, where user or third-party code (steps, formatters etc) does e.g.:

const TestCaseHookDefinition = require('@cucumber/cucumber/lib/models/test_case_hook_definition')

And we get an error like:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './lib/models/test_case_hook_definition' is not defined by "exports" in /Users/davidgoss/Projects/cucumber-js/package.json imported from /Users/davidgoss/Projects/cucumber-js/tmp/direct_imports.feature_49/features/step_definitions/cucumber_steps.js
           at throwExportsNotFound (internal/modules/esm/resolve.js:285:9)
           at packageExportsResolve (internal/modules/esm/resolve.js:508:3)
           at trySelf (internal/modules/cjs/loader.js:400:34)
           at Function.Module._resolveFilename (internal/modules/cjs/loader.js:793:24)
           at Function.Module._load (internal/modules/cjs/loader.js:667:27)
           at Module.require (internal/modules/cjs/loader.js:887:19)
           at require (internal/modules/cjs/helpers.js:74:18)
           at Object.<anonymous> (/Users/davidgoss/Projects/cucumber-js/tmp/direct_imports.feature_49/features/step_definitions/cucumber_steps.js:2:32)
           at Module._compile (internal/modules/cjs/loader.js:999:30)
           at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
           at Module.load (internal/modules/cjs/loader.js:863:32)
           at Function.Module._load (internal/modules/cjs/loader.js:708:14)
           at Module.require (internal/modules/cjs/loader.js:887:19)
           at require (internal/modules/cjs/helpers.js:74:18)
           at World.<anonymous> (/Users/davidgoss/Projects/cucumber-js/features/support/hooks.ts:119:11)

This happens whether or not you're using ESM, and is because of the new block added to our package.json:

"exports": {
  "import": "./lib/wrapper.mjs",
  "require": "./lib/index.js"
},

We need this in order for ESM to work (and for CommonJS to still work), but it does have the effect of enforcing that requires can only be done at the entry point, meaning "deep" ones to individual files will fail.

Node has the concept of subpath patterns for this kind of thing, but this can't be mix-and-matched with the usage of exports for ESM vs CommonJS entry points like we have, so that's off the table.

---

My current thinking: the public API for cucumber-js is what we expose on the main entry point. Once you require other things via their file path, you're into the weeds; package authors understand this. So in principle running with this as it is wouldn't be a breaking change...but it would be disruptive to end users and it's not their fault.

We could instead:

1. Add things to our main entry point to address the needs of package authors (see list in Changes in 7.2.0 break external reporters #1646)
2. Submit PRs to those projects to update their imports
3. Release this change again as semver minor (and be responsive to a possible long tail of small issues)

WDYT @charlierudolph @aurelien-reeves ?

[jan-molak]

@davidjgoss to avoid "polluting" the main entry point with functions and classes only package authors need to know about you might want to consider exporting them under some object like internal.

So a regular Cucumber user would only see one additional thing exported by the module, for example:

import { Given, When, Then, internal } from '@cucumber/cucumber'

const definition = new internal.TestCaseHookDefinition(...)

This way you could keep attaching "internal" functions useful for package maintainers without making it disruptive.

Could this work?

[davidjgoss]

@jan-molak I think that's a good point yeah. It's still part of the API but at least grouped where it's less commonly used. I think there is some prior art here with formatterHelpers

[TheLudd]

Have you looked into subpath exports? https://nodejs.org/api/packages.html#packages_subpath_exports
Not sure if it is possible to combine with the import/require statements here https://github.com/cucumber/cucumber-js/pull/1649/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R167 but I think that is the way to expose other exported paths than the main one going forward.

[coveralls]

Coverage increased (+0.02%) to 98.266% when pulling 8037766 on esm-take-2 into 6e958f1 on main.

[davidjgoss]

Thanks for the pointer @TheLudd - a combination of subpath exports and export patterns gets it done:

{
  "exports": {
    ".": {
      "import": "./lib/wrapper.mjs",
      "require": "./lib/index.js"
    },
    "./lib/*": {
      "require": "./lib/*.js"
    }
  },
}

[davidjgoss]

@charlierudolph @aurelien-reeves with the fix for deep imports in commonjs this is good to go now.

[aurelien-reeves]

This is really great, well done! 👍

We may have to add an entry in the changelog for bringing back esm support

And, the esm.md is a good idea 👍
Could we add some code examples in it?

[davidjgoss]

And, the esm.md is a good idea 👍 Could we add some code examples in it

Good point, I've added example code now.

[aurelien-reeves]

The failures with the Windows builds seems legit :(

I'll take a look as soon as I have access to a Windows computer

[davidjgoss]

@aurelien-reeves yeah I think adding allowJs might have done this. I'll probably just back that out and we'll live with doing an ugly require for now

[aurelien-reeves]

🎉

[nemanja-tosic]

What is the ETA for this feature? I see it's part of 8.0.0 but I can't find any specific dates.

[petermorlion]

@nemanja-tosic I don't think there is a specific date. There is one more issue that needs to be resolved as part of the 8.0.0 milestone. Disclaimer: I'm not a contributor to cucumber-js.

[davidjgoss]

@nemanja-tosic @petermorlion we're aiming to get a release candidate out in the next couple of days, please subscribe to releases to hear about it straight away.

[Ploppz]

Did this feature really go through in version 8? Or what am I doing wrong here?

$ ./node_modules/.bin/cucumber-js -v
8.5.1
$ ./node_modules/.bin/cucumber-js --esm
error: unknown option '--esm'

[aurelien-reeves]

You don't need the --esm flag anymore

https://github.com/cucumber/cucumber-js/blob/main/docs/esm.md

[Ploppz]

Hmm.. but if I add this package.json file to my support files and steps files directories:

{
  "name": "test1",
  "version": "0.0.1",
  "type": "module"
}

Then I get this error:

Error [ERR_REQUIRE_ESM]: require() of ES Module /home/node/tests/features/step_definitions/overview_steps.js from /home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/api/support.js not supported.
Instead change the require of overview_steps.js in /home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/api/support.js to a dynamic import() which is available in all CommonJS modules.
    at /home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/api/support.js:14:32
    at Array.map (<anonymous>)
    at getSupportCodeLibrary (/home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/api/support.js:14:18)
    at runCucumber (/home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/api/run_cucumber.js:33:53)
    at async Cli.run (/home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/cli/index.js:45:29)
    at async Object.run [as default] (/home/node/node_modules/.pnpm/@cucumber+cucumber@8.5.1/node_modules/@cucumber/cucumber/lib/cli/run.js:34:18)

[davidjgoss]

The default behaviour and the require option both use the legacy CommonJS modules API to load your files. If your files are native ES modules, you'll need to use the import option instead in the same way, and they'll be loaded with the new ES modules API. See ES Modules for more on using Cucumber in an ESM project.

[Ploppz]

Is that quoted from some source? Sorry if I misunderstand but I do already use import when importing anything cucumber-related; does that quote suggest anything else?
My error seems to indicate that cucumber tries to require my js files instead of importing them, if I understand correctly.

[aurelien-reeves]

I guess the quotation marks from David is a typo.
However the documentation here should help: https://github.com/cucumber/cucumber-js/blob/main/docs/esm.md

Did you read it? Is there something with that documentation that you do not understand (legit question! We may need to improve it!)?

You should not use a dedicated package.json file inside your support code directory.

What are you trying to achieve exactly? Your project is using ESM or not?

If it does, you do not need a package.json beside your support code.

The require and import things are used when you explicitly require or import your support code using configuration file or command-line arguments.

Would you have a repo where we could take a look?

[Ploppz]

I was initially confused about you'll need to use the import configuration option to specify your files, rather than the require option especially since the below example does not even use said import configuration option, but looking at the documentation for the config cleared that up. I tried the config:

const common = {
  import: ['features/support/**/*.js']
}

export default {
  ...common,
}

export const ci = {
  ...common,
}

but I get

export default {
^^^^^^

SyntaxError: Unexpected token 'export'

anyway I guess I shouldn't need such a config file?
My project is generally ESM I guess yes, but I say that solely because it consistently uses import instead of require, I don't know in what else determines whether it's an ESM project.

Well, if I remove the package.json files, I get errors:

import { When, Then, Given } from '@cucumber/cucumber';
^^^^^^

SyntaxError: Cannot use import statement outside a module

So I left them in the attached zip file. it's only the contents of the tests/ folder in my project. I start tests with ./node_modules/.bin/cucumber-js tests. Do you need anything else?
tests.zip

Edit: It's a React project btw, created with CRA

[aurelien-reeves]

What is your version of @cucumber/cucumber?
Which version of node-js?

Do you have "type": "module" in your root package.json?

Here's the documentation for the configuration: https://github.com/cucumber/cucumber-js/blob/main/docs/configuration.md

The error related to the config file as shown below seems to show that your project is not a module. If you still want to use ESM with cucumber, try renaming your configuration file into cucumber.mjs

export default {
^^^^^^

SyntaxError: Unexpected token 'export'

Please not also that per default, your cucumber configuration file should be at the root level of your project, not in your "tests" folder.

what is your cucumber command and arguments to run your tests? Are you using a npm script?

[davidjgoss]

Yeah further to what @aurelien-reeves said, a CRA project's files may use import and export syntax but they aren't really ES modules, since they're compiled down to CommonJS and then bundled for the browser. As such you probably shouldn't add type: module to your package.json. Having your Cucumber config and support code files as .mjs is probably the way to go here - you're marking those files as ESM without trying to reclassify the whole project.

[Ploppz]

Thank you both so much! Alright I changed everything cucumber-related to .mjs.

What is your version of @cucumber/cucumber?
what is your cucumber command and arguments to run your tests? Are you using a npm script?
Which version of node-js?

// ...
    "@cucumber/cucumber": "8.5.1",
// ...
  "scripts": {
    "test": "./node_modules/.bin/cucumber-js tests",
// ...

  "engines": {
    "node": "17.x"
  }

Now I'm getting a new error that seems to stem from inside node, with no information about code location in my own code in any case:

SyntaxError: Unexpected token ')'
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:117:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:361:14)
    at async link (node:internal/modules/esm/module_job:70:21)

idk if there's still some mismatch with like, node version (since JS seems to be very finicky about all its moving parts)?

[Ploppz]

Sorry, the extra ')' was in my own code! I just thought it wouldn't be because it didn't point to any of my files

[devexpert7]

Is this fix available in latest release? Please share working code if available.
I get same error
npx cucumber-js
Error: Cucumber expected a CommonJS module at '/Users/dev/features/support/support.js' but found an ES module.
Either change the file to CommonJS syntax or use the --import directive instead of --require.

  Original error message: require() of ES Module /Users/dev/features/support/support.js from /Users/develop/node_modules/@cucumber/cucumber/lib/try_require.js not supported.

Instead change the require of support.js in /Users/dev/gitlab1/node_modules/@cucumber/cucumber/lib/try_require.js to a dynamic import() which is available in all CommonJS modules.
at tryRequire (/Users/dev/gitlab1/node_modules/@cucumber/cucumber/lib/try_require.js:15:19)

[unional]

Hi, @devexpert7,

Probably need more information about your project. Do you have type: module in your package json?

[devexpert7]

Hi, @devexpert7,

Probably need more information about your project. Do you have type: module in your package json?

Yes @unional , I have type: module in your package json.
I tried to fix this by adding configuration file cucumber.cjs >
const common = {
requireModule: [
...(
process.env.CI ? [] : ["chromedriver"]
)
],
import: [
"src/steps/*.ts",
"src/steps",
]
}

module.exports = common
....

getting below error now:

my-dev@1.0.0 test-e2e-scenario
npm run cucumber -- features --name sayHello

my-dev@1.0.0 cucumber
ts-node node-modules/@cucumber/cucumber/bin/cucumber.js -p default features --name sayHello

node:internal/modules/cjs/loader:1080
throw err;
^

Error: Cannot find module './cucumber.js'
Require stack:

• /Users/project1/node-modules/@cucumber/cucumber/bin/imaginaryUncacheableRequireResolveScript
  at Module._resolveFilename (node:internal/modules/cjs/loader:1077:15)
  at Function.resolve (node:internal/modules/cjs/helpers:127:19)
  at requireResolveNonCached (/Users/project1/node_modules/ts-node/dist/bin.js:549:16)
  at getProjectSearchDir (/Users/project1/node_modules/ts-node/dist/bin.js:519:40)
  at phase3 (/Users/project1/node_modules/ts-node/dist/bin.js:267:27)
  at bootstrap (/Users/project1/node_modules/ts-node/dist/bin.js:47:30)
  at main (/Users/project1/node_modules/ts-node/dist/bin.js:33:12)
  at Object. (/Users/project1/node_modules/ts-node/dist/bin.js:579:5)
  at Module._compile (node:internal/modules/cjs/loader:1256:14)
  at Module._extensions..js (node:internal/modules/cjs/loader:1310:10) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
  '/Users/project1/node-modules/@cucumber/cucumber/bin/imaginaryUncacheableRequireResolveScript'
  ]
  }