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

esm: add support for loaders #1844

Closed
travi opened this issue Nov 19, 2021 · 31 comments · Fixed by #1993
Closed

esm: add support for loaders #1844

travi opened this issue Nov 19, 2021 · 31 comments · Fixed by #1993
Assignees
Labels
⚡ enhancement Request for new functionality
Milestone

Comments

@travi
Copy link
Member

travi commented Nov 19, 2021

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I'm excited to start migrating projects of mine that are tested with cucumber over to native ESM. However, as mentioned in #1304 (comment), loaders do not appear to work with the recent release candidate. I'm unsure if this is intended to work at this point, but I wasnt able to figure it out on my own.

instead, i meet the following error:

(node:62231) ExperimentalWarning: --experimental-loader is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension "" for /path/to/project/node_modules/@cucumber/cucumber/bin/cucumber-js
    at defaultGetFormat (internal/modules/esm/get_format.js:71:15)
    at getFormat (file:///path/to/project/node_modules/quibble/lib/quibble.mjs:65:12)
    at Loader.getFormat (internal/modules/esm/loader.js:104:42)
    at Loader.getModuleJob (internal/modules/esm/loader.js:242:31)
    at async Loader.import (internal/modules/esm/loader.js:176:17)
    at async Object.loadESM (internal/process/esm_loader.js:68:5) {
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
}

Describe the solution you'd like
A clear and concise description of what you want to happen.

ideally, the cucumber-js binary would accept --loader=<loader-name> directly

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

an alternative that should work is providing the loader through NODE_OPTIONS, like NODE_OPTIONS="--loader=testdouble" cucumber-js ...

Additional context
Add any other context or screenshots about the feature request here.

As requested in #1304 (comment), I've created a fairly minimal reproducible example at https://github.com/travi-test/cucumber-esm/.

The error can be demonstrated by running npm run test:integration (after npm install, of course). If the --loader=testdouble is removed from the test:integration:base script, the error goes away. In this minimal example that results in the tests passing because I'm not actually using the loader.

In a more realistic project, this obviously results in the loader not working, resulting in failures. An example of a more real project can be found in the cucumber-esm branch of https://github.com/pragmatic-divops/cli where I'm attempting to begin the process of converting the project to ESM.

@davidjgoss
Copy link
Contributor

@travi thanks for raising. I think it does make sense to add something, I'll aim to get something done for the next RC (though I note that loaders are still experimental in Node, so any support on our side would be experimental too).

@travi
Copy link
Member Author

travi commented Nov 30, 2021

I think it does make sense to add something, I'll aim to get something done for the next RC

great! i'm happy to try it out and provide feedback

loaders are still experimental in Node, so any support on our side would be experimental too

understood. i think that is important to keep visible, but is in line with how i've seen other projects handle the current state of things, so i don't expect that to present a problem for adoption

@pk
Copy link

pk commented Jan 19, 2022

Hello everyone, I'm hitting a massive wall with the ESM and CommonJS module loading. Here is a bit of background...

I'm testing Angular application using Cucumber & Playwright. In order to do this I want to use Angular Material test harnesses which encapsulate the selectors and interaction into an API. I wrote the Playwright adapter for Angular Material harnesses and all was fine till Angular 13....

In Angular 13 they stopped distributing CommonJS build and only distribute ESM build and now the whole test suite fails because Cucumber-JS uses CommonJS imports but Angular only exports in ESM.

From what I understand, if CucumberJS will load code using ESM all will work again because I can set my testsuite to compile (TS) to ESM, change my Playwright Adapter to ESM and load ESM Angular.

Is there any viable / experimental branch I can give a shot and possibly help? I really need this to work and I'm more than happy to work on it.

@aurelien-reeves
Copy link
Contributor

Did you tried v8.0.0-rc.2?

Using esm code with it should work as-is. you may just have to rename a potential cucumber.js file into cucumber.cjs

@davidjgoss
Copy link
Contributor

+1 to @aurelien-reeves comment above, please do give that a go

I can set my testsuite to compile (TS) to ESM

I think this might be where you hit a snag, if you are compiling TypeScript on the fly, because we don't have support for ESM loaders yet (which is what this issue is, I'll rename it for clarity).

@davidjgoss davidjgoss changed the title using loaders for an ESM project with the v8 rc esm: add support for loaders Jan 19, 2022
@davidjgoss davidjgoss added this to the ESM milestone Jan 19, 2022
@pk
Copy link

pk commented Jan 24, 2022

@davidjgoss & @aurelien-reeves I've now managed to run the whole thing on ESM modules. It has been pretty much rough ride but the Cucumber-JS was not really an issue.

What I did is that at the moment I'm compiling the TypeScript code before running a test suite avoiding the TS-Node for now.

Thanks for your work.

@aurelien-reeves
Copy link
Contributor

Thanks for your feedback @pk

@travi
Copy link
Member Author

travi commented Jan 24, 2022

this comment and following conversation appear to apply very directly to the situation here. linking here in case details of that conversation can help with understanding what a reasonable path forward might be in the short/long term.

@davidjgoss davidjgoss added the ⚡ enhancement Request for new functionality label Mar 2, 2022
@davidjgoss
Copy link
Contributor

Just following up on this now I understand the state of loaders a bit better.

ts-node and others support a --loader option on their own CLIs by doing all the work in a child process with that option passed through via NODE_OPTIONS. This has similarities with how we manage parallel running, but still would be a non-trivial change which I don't think we should commit to until/unless the loaders API becomes stable without a less friction-y way to do this.

However we should definitely aim to work when a loader has been registered with the process outside of Cucumber, and will work on that as a priority (with thanks to @travi for the detailed report and test repo).

@testgitdl

This comment was marked as off-topic.

@davidjgoss

This comment was marked as off-topic.

@testgitdl

This comment was marked as off-topic.

@davidjgoss

This comment was marked as off-topic.

@davidjgoss
Copy link
Contributor

With some testing with the newly-released 8.0.0 I found that:

Despite the inconsistency this seems like the right change - the vast majority of packages also have an extension for their bin files - and will be done via #1993 and released in 8.1.0

@cspotcode
Copy link

ts-node includes a workaround for the extensionless bin entrypoint issue. That might be why you don't see the issue with ts-node:
https://github.com/TypeStrong/ts-node/releases/tag/v10.6.0

@davidjgoss
Copy link
Contributor

@cspotcode ah yep there we go - thanks for the pointer!

@davidjgoss
Copy link
Contributor

This has now been released in v8.1.0 on npm

https://www.npmjs.com/package/@cucumber/cucumber

@travi
Copy link
Member Author

travi commented Apr 22, 2022

i've updated my test repo at https://github.com/travi-test/cucumber-esm/ with the latest version and it does solve the problem that this issue was specifically about. however, i'm now running into a different esm related problem:

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /path/to/project/travi-test/cucumber-esm/test/integration/features/step_definitions/common-steps.js
require() of ES modules is not supported.
require() of /path/to/project/travi-test/cucumber-esm/test/integration/features/step_definitions/common-steps.js from /path/to/project/travi-test/cucumber-esm/node_modules/@cucumber/cucumber/lib/api/support.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename common-steps.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /path/to/project/travi-test/cucumber-esm/package.json.

    at __node_internal_captureLargerStackTrace (internal/errors.js:412:5)
    at __node_internal_addCodeToName (internal/errors.js:168:9)
    at new NodeError (internal/errors.js:322:7)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1102:13)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:101:18)
    at /path/to/project/travi-test/cucumber-esm/node_modules/@cucumber/cucumber/lib/api/support.js:14:32
    at Array.map (<anonymous>)

this surprised me since you mentioned in #1993 that the fix was

tested against the minimal example repo there which confirmed the fix.

i probably assumed too much and thought you saw tests passing, but i imagine that you were mostly checking that the previous error no longer occurred.

@davidjgoss should i open a new issue for this, or is this just something minor that i'm overlooking on my end?

@davidjgoss
Copy link
Contributor

Hey @travi - I think this will be down to a late change we made before 8.0.0 - #1931 - which means you need to explicitly use the import option to point at your ESM support code so it's loaded with await import(). The default loading behaviour (finding .js files under your features dir) uses require for backwards compatibility (for now). Hope this helps!

@travi
Copy link
Member Author

travi commented Apr 23, 2022

Makes sense, thanks! I was wondering if the --esm flag had come back, but that didn't seem to be the case and I overlooked this change. Looks like that was it

@pk
Copy link

pk commented Apr 25, 2022

I'm incredibly happy to report that I've been able to upgrade our project to 8.1.2 and I can now run smoothly ESM, Angular 13, Playwright setup on top of ts-node with all steps/support code in TS.

Thank you everyone.

@testgitdl
Copy link

@pk glad on your results and maybe you can help ..that i'm also glad that this whole esm + ang 13 + playwright solution is working but I still do have 2 problems (not playwright or cucumber related) but maybe you can help
Prob 1: there seems to be a problem with tsconfig-paths & esm - so my paths defined in tconfig.json do not work (i have to use relate paths when importing). I've found this:
TypeStrong/ts-node#1375

Prob 2: when I import although having ts I have to provide the .js extension (in the login.page.ts: import { page } from '../support/hooks.js';) otherwise it does not work. Is it the same for you? I've found this: https://stackoverflow.com/questions/63807613/running-node-with-loader-ts-node-esm-js-requires-imports-to-have-the-js-extensi

Here is my project: https://github.com/testgitdl/cucumber-playwright-ang13
I did try adding --experimental-specifier-resolution=node (so the exec command: node --experimental-loader node --loader ts-node/esm ./node_modules/@cucumber/cucumber/bin/cucumber-js instead of node --loader ts-node/esm ./node_modules/@cucumber/cucumber/bin/cucumber-js bu then i get an invalid module error.

@pk
Copy link

pk commented Apr 26, 2022

@testgitdl

Problem 2:
I've just now tried to remove the *.js extensions from the imports of the support (project) code which is not packaged and it runs the tests without issues. My command to run the test suite is:
"test": "NODE_OPTIONS=\"--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node\" cucumber-js",

I'm using cucumber.mjs with folwing content:

export default {
  format: [
    'message:results/reports/report.ndjson',
    'html:results/reports/report.html',
    'progress',
  ],
  import: [
    "src/**/*.ts",
    "features/**/*.steps.ts"
  ],
  publishQuiet: true,
}

And this is my tsconfig.json:

{
  "compilerOptions": {
    "target": "ESNext",
    "module": "ESNext",
    "moduleResolution": "Node",
    "noEmit": true,
    "resolveJsonModule": true,
    "esModuleInterop": true,
    "lib": ["ESNext", "DOM"]
  },
}

Keep in mind my versions are: Cucumber 8.1.2, ts-node 10.7.0, typescript 4.6.3 & node gives me v18.

@pk
Copy link

pk commented Apr 26, 2022

@testgitdl As for Problem 1, yes that's still valid on my setup as well. I'm using relative paths... at the moment.

@testgitdl
Copy link

@pk thanks for helping out. I've made the setup as you stated and it is still not working but what I did not manage to do is to run the script as you mentioned: "test": "NODE_OPTIONS="--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node" cucumber-js"
So probably here is the problem.. I get this error when I run the script above: > NODE_OPTIONS="--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node" cucumber-js
'NODE_OPTIONS' is not recognized as an internal or external command

If I change to: "test": "SET NODE_OPTIONS="--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node" && cucumber-js",
then I get another error:

SET NODE_OPTIONS="--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node" && cucumber-js
node: --loader ts-node/esm --no-warnings --experimental-specifier-resolution= is not allowed in NODE_OPTIONS

@pk
Copy link

pk commented Apr 26, 2022

@testgitdl Careful with the (") quotes.... You need to escape internal quotes with \ (slash). This is scripts part of my package.json

  "scripts": {
    "build": "rm -rf ./build/* && tsc -p ./tsconfig.json",
    "debug": "PW_DEBUG=1 DEBUG=pw:api npm run test",
    "fixtures": "npm run test fixtures/**/*.feature",
    "fixture": "npm run test",
    "test-only": "npm run test -- --tags @only",
    "test": "NODE_OPTIONS=\"--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node\" cucumber-js",
  },

I run this using: PW_BASE_URL=https://..... npm run test

I've also just run my test suite on the CI with Node v16.13.0 successfully.

@testgitdl
Copy link

testgitdl commented Apr 26, 2022

@pk indeed with the quotes and the slash before (copied from from your comment above "test": "NODE_OPTIONS=\"--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node\" cucumber-js", and running: npm run test I get the same error: 'NODE_OPTIONS' is not recognized as an internal or external command
As I'm running on windows I've also tried with SET NODE_OPTIONS=... as mentioned before but is still does not work
I've tried to also set these values manually in the NODE_OPTIONS environ variab but the same problem as it cannot find the .ts (only .js)

@pk
Copy link

pk commented Apr 26, 2022

@testgitdl I won't be much of help with Windows, sorry. What if you set the "global" env variable and then try to run the test suite.

On Mac I'd do something like this in terminal:

NODE_OPTIONS="--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node"
echo $NODE_OPTIONS # this should print the above
npx cucumber-js

@testgitdl
Copy link

@pk no problem! thanks for your help!
Got it to work!!!! "test": "SET NODE_OPTIONS=--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node && node ./node_modules/@cucumber/cucumber/bin/cucumber.js",

And to debug in VS Code:

{
  // Use IntelliSense to learn about possible attributes.
  // Hover to view descriptions of existing attributes.
  // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
  "version": "0.2.0",
  "configurations": [
    {
      "name": "debug-only",
      "request": "launch",
      "runtimeArgs": [
        "./node_modules/@cucumber/cucumber/bin/cucumber.js",
        // "--tags",
        // "@runMe"
      ],
      "skipFiles": [
        "<node_internals>/**"
      ],
      "type": "node",
      "env": {
        "DEBUG": "pw:api",
        "PWDEBUG": "1",
        "NODE_OPTIONS": "--loader ts-node/esm --no-warnings --experimental-specifier-resolution=node"
      },
      "console": "internalConsole",
      "outputCapture": "std"
    }
  ]
}

@testgitdl
Copy link

@pk managed to remove the relative paths by using a workaround..a custom loader specified here TypeStrong/ts-node#1450 (comment)

On windows now I'm able to use the paths when running Cucumber+Playwright+Angular13 tests via:
"test-bdd-chrome": "SET NODE_OPTIONS=--loader ts-node/esm --loader ./loader.js --no-warnings --experimental-specifier-resolution=node && node ./node_modules/@cucumber/cucumber/bin/cucumber.js",

Where loader.js:
`//this is a workaround until the tsconfig-path ESM problem is fixed - TypeStrong/ts-node#1375
import { resolve as resolveTs } from 'ts-node/esm'
import * as tsConfigPaths from 'tsconfig-paths'
import { pathToFileURL } from 'url'

const { absoluteBaseUrl, paths } = tsConfigPaths.loadConfig()
const matchPath = tsConfigPaths.createMatchPath(absoluteBaseUrl, paths)

export function resolve(specifier, ctx, defaultResolve) {
const match = matchPath(specifier)
return match
? resolveTs(pathToFileURL(${match}).href, ctx, defaultResolve)
: resolveTs(specifier, ctx, defaultResolve)
}

export { load, transformSource } from 'ts-node/esm'`

Hope it helps

@cspotcode
Copy link

Linking to the PR that will add built-in path mapping to ts-node: TypeStrong/ts-node#1664

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

Successfully merging a pull request may close this issue.

6 participants