esm: some deep requires don't work

[davidjgoss]

Describe the bug

Not exactly about ESM, but a symptom of the way we've made the package "hybrid".

Some require statements that work fine when everything's CommonJS don't work with new ESM-aware Node versions and our module config.

To Reproduce
Steps to reproduce the behavior:

1. Use @cucumber/[email protected] and @cucumber/pretty-formatter
2. Do a test run with --format @cucumber/pretty-formatter

Expected behavior

Normal test run with output from the formatter.

Actual behaviour

Error: Cannot find module '/Users/davidgoss/Projects/my-project/node_modules/@cucumber/cucumber/lib/formatter/helpers.js'

Additional context

The stack trace leads to this require statement in the pretty formatter's compiled code:

const helpers_1 = require("@cucumber/cucumber/lib/formatter/helpers");

We have this mapping in our package.json:

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

The actual file it should be importing is:

node_modules/@cucumber/cucumber/lib/formatter/helpers/index.js

We'll need to either improve this exports mapping or rethink how we're accomodating ESM.

[davidjgoss]

I've spent some time experimenting with this (based on extending an existing test d0a63b2), and I don't think we can do any better than what's there now unfortunately.

• We need the exports there in order to be compatible with ESM projects
• The subpath exports functionality that we are using to try to maintain compatibility with old deep requires is very limited. This issue package.json uses deprecated subpath folder mappings in exports jaydenseric/extract-files#17 (comment) summarises it pretty well, but TLDR: it's not possible to have it cover all the permutations people might have written

So I think the least worst strategy is:

• Leave the exports config as it is
• Document in the release notes that many deep import/requires will likely break
• Fix forward by adding stuff to the main entry point that people need

[mattwynne]

Sounds good to me. I always thought deep requires were a bit of a hack anyway, no?

[davidjgoss]

@mattwynne agreed - they're always a symptom of something else.

[davidjgoss]

Closing the loop on this, I went back over this comment from @jan-molak regarding deep import usages in the wild:

• package.json (via Export version number of cucumber-js #1866)
• TestCaseHookDefinition (via fix(esm): fix esm/commonjs compatibility issues with custom formatters/snippets #1903)
• various issues in @cucumber/pretty-formatter (via fix: remove deep imports to cucumber cucumber-js-pretty-formatter#11)
• IRuntimeOptions
• ITestCaseHookParameter
• EventDataCollector - available on formatterHelpers from main export (along with tons of other stuff)

So in the next RC we should be all good.

[jan-molak]

Thanks @davidjgoss - I'll give that a try soon