Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with transitive (pure) ESM dependency #2248

Closed
NiclasThobaben opened this issue Feb 23, 2023 · 13 comments
Closed

Problems with transitive (pure) ESM dependency #2248

NiclasThobaben opened this issue Feb 23, 2023 · 13 comments

Comments

@NiclasThobaben
Copy link

Not sure whether this is a bug in cucumber-js or some misconfiguration/misunderstanding on our part.

👓 What did you see?

In a library we develop, we use two pure ESM dependencies (react-dnd and react-table-library). The library is written in typescript, so this is not a problem generally. For our unit tests with jest we us babel to transform these libraries to commonJS, which works like a charm.
However, when we import our library in a support file for cucumber we get issues, because we try to require() an ES module from our commonJS bundle. Which seems logical to me, but after many hours of trying to work around this issue, I'm stuck now.

We use babel for transpiling:

// babel.config.js
module.exports = {
  plugins: [
    [
      "@babel/plugin-transform-modules-commonjs",
    ]
  ],
  presets: [
    ["@babel/preset-react", {"runtime": "automatic"}],
    "@babel/preset-typescript",
  ],
  ignore: [
    // ignore node_modules, except our pure ESM dependencies
    "node_modules/(?!@table-library|dnd-core|react-dnd|@react-dnd)",
  ],
};

and register it in our cucumber config:

// cucumber.js

// Removing this results in the .ts/.tsx support files be loaded as CJS?
require("@babel/register")({
    extensions: [".ts", ".tsx", ".js", ".jsx"],
})

module.exports = {
    default: {
        failFast: process.env.NODE_ENV === 'development',
        requireModule: [
            'jsdom-global/register',
            // No difference if removed
            '@babel/register',
        ],
        // Tried importing it as ESM directly using ts-node loader, doesn't work either
        // import: [
        //     './cucumber/worlds/**/*.{ts,tsx}',
        //     './cucumber/step_definitions/**/*.{ts,tsx}',
        // ],
        require: [
            './cucumber/worlds/**/*.{ts,tsx}',
            './cucumber/step_definitions/**/*.{ts,tsx}',
        ],
        worldParameters: {
            deviceSpecification: process.env.DEVICE_SPEC || DEFAULT_DEVICE_SPEC,
            browser: process.env.BROWSER || DEFAULT_BROWSER,
        },
        publish: false,
        publishQuiet: true,
    }
}

AFAIK the @babel/register only registers the on-the-fly require-hook into node, which does not work for ESM, so execution stops at the first require() of such ESM.

I also tried the following (with and without @babel/register):

  • Using the import option with NODE_OPTIONS=--loader ts-node/esm does not work. -> Typescript support files are loaded as CJS
  • Using NODE_OPTIONS=--loader ts-node does not work -> raises TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".tsx"
  • Using any of the above with explicitly setting requireModule: ts-node/register -> seems to be transpiled twice
  • Using only requireModule: ts-node/register -> raises TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".tsx"
  • Using requireModule: 'ts-node/register' does not work

✅ What did you expect to see?

I expected all files to be correctly transformed by babel, before being processed by cucumber-js, just like we achieve it in jest.

📦 Which tool/library version are you using?

@cucumber/cucumber: 8.11.1
@babel/cli: 7.21.0
ts-node: 10.9.1
node: v16.16.0

🔬 How could we reproduce it?

If this is not something completely obvious, I could create an example repository for this issue.

Steps to reproduce the behavior:

  1. Create a library package using an pure ESM dependency
  2. Create a packge using cucumber and the library
  3. Import the library using the ESM dependency in a support file
  4. Use babel and cucumber config from above
  5. Run cucumber

📚 Any additional context?

Like I said, I'm not sure if this is a bug or just some stupidity on our side. I looked into ways to circumvent this issue at the level of our library, but I didn't find some babel or rollup config to convert the imports for only these libs to async import() . Converting all locations manually or (even worse) providing our own CJS ports of the libs seems overkill, considering we only have this issue with cucumber.

I'd appreciate any idea on this case!


This text was originally generated from a template, then edited by hand. You can modify the template here.

@davidjgoss
Copy link
Contributor

Do you have a tsconfig.json? If so, what does that look like?

@NiclasThobaben
Copy link
Author

Yeah sure! (Totally forgot about that, sorry)
We also tried playing around with the target and module options and followed the instructions in the transpiling documentation.

{
  "compilerOptions": {
    "target": "es5",
    "lib": [
      "dom",
      "dom.iterable",
      "esnext"
    ],
    "allowJs": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noFallthroughCasesInSwitch": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react-jsx"
  },
  "include": [
    "src"
  ]
}

@davidjgoss
Copy link
Contributor

davidjgoss commented Feb 28, 2023

As you've found I don't think ts-node will help you here. Babel is what you're using to transpile so we should stick with that.

Calling @babel/register in the way you have from the config file probably makes the most sense. Are you sure it's picking up your babel config file? Also I note the doc for that package suggests you may need to explicitly configure the ignore there too.

If this is not something completely obvious, I could create an example repository for this issue.

That would be good if you have time.

@NiclasThobaben
Copy link
Author

I created a small example repository here.
It seems like the babel config gets picked up, but also with the explicit ignore, it does not work unfortunately. Only thing I've noticed is, when the ignore is not set, it runs a little bit faster, as if it is correctly trying to transpile the external library.

It seems like the issue now comes from babel. As stated here, @babel/register can not transpile ESM on the fly (however, I'm not quite sure if that refers to babel 8 only).

@Izhaki
Copy link
Contributor

Izhaki commented Mar 8, 2023

I've been pretty much on the same path as yours, trying everything you've tried, and spent around 10h on this issue.

It would be impossible for me to impart all I've learnt, but this is generally not a cucumber issue. Rather, it is the current friction between CJS and ESM on node (I haven't had a chance to fully investigate it, but my current hypothesis is that the esm node loader can toggle to the cjs loader, but then breaks the other way around).

The good news is that there is a working solution: https://www.npmjs.com/package/fix-esm; I've used this:

require("fix-esm").register();

And for the brave, a hint of the nature of the issue is in that library code:

if (error.code === "ERR_REQUIRE_ESM") {
	// NOTE: We completely bypass the default internal loader, as we cannot patch the `type: module` check out of that. Please open a PR if you have a better solution!
	let code = fs.readFileSync(filename, "utf8");
	let transpiledCode = transpile(code);
	mod._compile(transpiledCode.code, filename);
} else {
	throw error;
}

@NiclasThobaben
Copy link
Author

Yeah, exactly my thought. This whole topic of CJS and ESM is somewhat confusing, especially when coming from a different background. It would be great, if library vendors would always still include a CJS build, considering that its actually not that hard to do with rollup etc., but oh well..

I will try the fix-esm package later, seems like it's the best and most "generic" way to go about the issue. Thanks a lot for the hint!

@davidjgoss
Copy link
Contributor

I'll close this now as I don't think there's anything for Cucumber to change, it's more a reflection of the weird state of things in Node.js at the moment with regard to module formats and loading.

@davidjgoss davidjgoss closed this as not planned Won't fix, can't repro, duplicate, stale Mar 9, 2023
@Izhaki
Copy link
Contributor

Izhaki commented Mar 9, 2023

@davidjgoss I agree that this issue should be closed.

However, I believe cucumber-js is currently only using require internally, which causes all sorts of issues for people who are configuring their repos as esm ones ("type": "module").

Although many do not support it yet (NextJS for example, as it is using internally bespoke loaders), more and more libraries phase out CJS support, and more and more people migrate to esm repros.

These problems will just generate more and more noise in the future.

I wonder if an initiative such as that by @halvardssm, where we split the distribution to cucumber-js and cucumber-js-esm are a viable plan forward.

This topic is probably worthy a new issues, maybe an RFC.

@davidjgoss
Copy link
Contributor

currently only using require internally

No, we support ESM as well and will use dynamic import where specified:

I wonder if an initiative such as that by @halvardssm, where we split the distribution to cucumber-js and cucumber-js-esm are a viable plan forward.

I don't think that would achieve anything. It was one of the patterns we considered for dual support initially, but it achieves essentially the same result.

@davidjgoss
Copy link
Contributor

More background #2059

@Izhaki
Copy link
Contributor

Izhaki commented Mar 9, 2023

Oooooooooohhhhh.....

It does work! But you have to:

cucumber-js --import 'features/support/**/*.js'

I have read the ESM guide before, but completely missed this sentence:

you'll need to use the import configuration option to specify your files.

This is awesome to know!

@NiclasThobaben
Copy link
Author

@Izhaki This does not seem to work with Typescript. But using fix-esm with require works, although I need to disable the usage of the local babel config file (not really sure why).

What I don't like about the fix-esm solution, is that the project seems to be hibernating. Open issues and PRs for about a year on a private Gitea server is not really a boost in confidence. So I'm thinking about forking it to Github, so the community can work on it.

@michael-lloyd-morris
Copy link
Contributor

I ran into a similar issue to this with NextJS and wrote about my findings here. It might have enough overlap to help:

https://mmorris-sandbox.net/demos/cucumber-next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants